[OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON

[Phil Race]

Hi Andrew,

  373         fontHints.put(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON);

Why do we need this ? Historically in the (non-OSX) code this would slow things down by making
text rendering go via ductus rendering.
If this really is a 'fontsHint' map,  it seems it should not be here.

  410             sg2d.surfaceData.getTransparency() == Transparency.OPAQUE &&

I thought you were removing this condition ?

CGGlyphImages.m

  202 #if __LITTLE_ENDIAN__
  203     *(dst + 2) = 0xFF - (p >> 24 & 0xFF);
  204     *(dst + 1) = 0xFF - (p >> 16 & 0xFF);
  205     *(dst) = 0xFF - (p >> 8 & 0xFF);
  206 #else
  207     *(dst) = 0xFF - (p >> 16 & 0xFF);
  208     *(dst + 1) = 0xFF - (p >> 8 & 0xFF);
  209     *(dst + 2) = 0xFF - (p & 0xFF);
  210 #endif
  211 }

became
  217 {
  218     *(dst + 0) = 0xFF - (p >> 16 & 0xFF);  // red
  219     *(dst + 1) = 0xFF - (p >>  8 & 0xFF);  // green
  220     *(dst + 2) = 0xFF - (p & 0xFF);        // blue
  221 }


I started by assuming you were removing the BIG_ENDIAN code that
was probably legacy PPC code but instead I see that the LITTLE_ENDIAN 
case is removed,
so I don't understand this.

And further down the file now I see that in ConvertBWPixelToByteGray you 
did remove the big endian case.

365     case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_GASP:
  366     case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_DEFAULT:
  367     default:
  368         e = [NSException
  369                 exceptionWithName:@"IllegalArgumentException"
  370                 reason:@"Invalid hint value"
  371                 userInfo:nil];


I think this means that by the time we get here we should not have 
DEFAULT or GASP
but we should have the actual interpretation for this surface, font and 
point size.
So this looks correct but maybe you can add a comment on this.

The heuristic of blending black and white is interesting.
How did you arrive at this ? It suggests that the glyph bitmap we are 
caching is
not 'colour neutral' which is odd. Maybe we are missing code to apply
the 'reverse gamma correction' ?

And can (should) any of these changes also be applied to D3D ?

-phil.

On 03/27/2015 07:50 AM, Andrew Brygin wrote:
> There is a minor update in the fix: it does not worth to blend 
> black-on-white
> and white-on-black glyphs for the case of AA glyphs, because it makes 
> no difference.
> CGGlyphImages.m, lines 294 - 325, 755 - 763, and 787 - 794:
>
> http://cr.openjdk.java.net/~bae/8023794/9/webrev.08
>
> I have also modified the AntialiasDemo a bit in order to render the
> sample text in different colors, and in order to render into a volatile
> image as well:
> http://cr.openjdk.java.net/~bae/8023794/demo/AntialiasDemo.java
>
> It could be used to assess the change in glyph generation.
>
> Thanks,
> Andrew
>
> On 3/26/2015 3:59 PM, Andrew Brygin wrote:
>> Hi Chris,
>>
>>  thank you for the comments and explanation. I have updated the 
>> OGLContext_IsLCDShaderSupportAvailable()
>>  and comments in OGLTextRenderer.c accordingly:
>>
>> http://cr.openjdk.java.net/~bae/8023794/9/webrev.07/
>>
>>  Good to know that you keep an eye on the OGL pipeline :)
>>
>> Thanks,
>> Andrew
>>
>> On 3/25/2015 8:24 PM, Chris Campbell wrote:
>>> Hi Andrew,
>>>
>>> That's a good find re: pow().  In the comment at lines 277-283 I 
>>> mentioned that there was only a scalar variant of pow().  I wonder 
>>> if that was a limitation in some early version of GLSL I was using 
>>> or perhaps some driver bug/restriction.  According to all the docs I 
>>> can find the vector variants have been there all along:
>>> https://www.opengl.org/sdk/docs/man4/index.php
>>>
>>> So now I'm wondering if the slowness was actually due to me trying 3 
>>> scalar pow() calls versus one vector pow() call.
>>>
>>> Oh, aha!  I think this explains part of it:
>>>
>>>  269  * This is the GLSL fragment shader source code for rendering 
>>> LCD-optimized
>>>  270  * text.  Do not be frightened; it is much easier to understand 
>>> than the
>>>  271  * equivalent ASM-like fragment program!
>>>
>>> Looks like I wrote the original version of this shader using the 
>>> GL_ARB_fragment_program language, which indeed only had a scalar POW 
>>> instruction (see section 3.11.5.20):
>>> https://www.opengl.org/registry/specs/ARB/fragment_program.txt
>>>
>>> And then I'm guessing that when I rewrote it in more modern GLSL, I 
>>> didn't notice that pow() supported vectors.  Sigh. That 3D texture 
>>> LUT was a lot of work for a whole lot of nothing.
>>>
>>> In any case, you might want to rewrite the comment at lines 277-283 
>>> to suit the new code.  Same goes for the comment at line 315:
>>>
>>>     // gamma adjust the dest color using the invgamma LUT
>>>
>>> Also, I noticed that the restrictions in 
>>> OGLContext_IsLCDShaderSupportAvailable() could be loosened since you 
>>> only need 2 texture units now.  Just in the extremely unlikely case 
>>> that anyone's running this stuff on ancient hardware :)
>>>
>>> Thanks,
>>> Chris
>>>
>>>
>>> Andrew Brygin wrote:
>>>> Hello Phil,
>>>>
>>>> could you please take a look to updated version of the fix?
>>>>
>>>> http://cr.openjdk.java.net/~bae/8023794/9/webrev.06/
>>>>
>>>> * OGLTextRenderer.c
>>>> It was discovered, that in some cases lcd glyphs had visible 'dark
>>>> border' around.
>>>> This border appeared due to the gamma correction in lcd shader, which
>>>> uses lookup
>>>> on 3D texture instead of using 'pow' routine. The texture is populated
>>>> with significant
>>>> granularity (16 points per edge), what is a root cause of rough
>>>> interpolation of color values.
>>>> Suggested change is to eliminate the interpolation and use 'pow' 
>>>> routine
>>>> directly.
>>>> It gives more accurate color values, and does not introduce 
>>>> significant
>>>> performance hit
>>>> (see benchmark summary below).
>>>> However, this part of the fix affects not only macosx, but any 
>>>> platform
>>>> where OGL text
>>>> shader can be used, so it probably worth to split into a separate fix.
>>>>
>>>> Summary:
>>>> lcd-ogl-3Dlookup:
>>>> Number of tests: 4
>>>> Overall average: 42.68027553311743
>>>> Best spread: 3.49% variance
>>>> Worst spread: 29.59% variance
>>>> (Basis for results comparison)
>>>>
>>>> lcd-ogl-pow:
>>>> Number of tests: 4
>>>> Overall average: 42.468941501367084
>>>> Best spread: 2.51% variance
>>>> Worst spread: 29.44% variance
>>>> Comparison to basis:
>>>> Best result: 103.28% of basis
>>>> Worst result: 97.36% of basis
>>>> Number of wins: 1
>>>> Number of ties: 2
>>>> Number of losses: 1
>>>>
>>>> Thanks,
>>>> Andrew
>>>>
>>>>
>>
>