[OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON
Andrew Brygin
andrew.brygin at oracle.com
Fri Apr 24 16:33:10 UTC 2015
Hello Phil,
please see my comments inline.
On 4/23/2015 11:15 PM, Phil Race wrote:
>
> 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.
I agree that this particular hint has no direct relation to the font hints,
and probably should not be here. I guess that KEY_ANTALIASING was put
here in order to force text antialiasing on when default text
antailiasing is evaluating:
http://hg.openjdk.java.net/jdk9/client/jdk/file/0e483e64c1e4/src/java.desktop/share/classes/sun/java2d/SurfaceData.java#l741
I do not think that it has any effect now, because we set text
antialiasing hint explicitly.
I will check whether it can be safely removed.
> 410 sg2d.surfaceData.getTransparency() == Transparency.OPAQUE &&
>
> I thought you were removing this condition ?
I have rolled this change back, because at the moment lcd shader
produces ugly results in the case of translucent destination.
There is a separate bug regarding the translucent destination support:
https://bugs.openjdk.java.net/browse/JDK-8078516
I am going to relax this condition when(if) our lcd shader will be ready.
In fact, this problem is not limited by ogl, because d3d and software loops
has the same limitation.
>
> 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.
>
Note that we are
Please note that we force the lcd glyph generation by requesting
host (i.e. LITTLE_ENDIAN) byte order for the canvas bitmap:
407 uint32_t bmpInfo = kCGImageAlphaPremultipliedFirst;
408 if (mode->glyphDescriptor == &rgb) {
409 bmpInfo |= kCGBitmapByteOrder32Host;
410 }
So, before the fix (and for grayscale case now) the buffer has default
BIG_ENDIAN order. I.e. grayscale canvas stores data in following format:
as bytes: A R G B
as int: 0xBBGGRRAA
The same byte order was used for the rgb canvas before the fix.
But now the canvas is configured to store data in following format:
as bytes: B G R A
as int: 0xAARRGGBB
So, data extraction routines were updated accordingly.
> 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.
will do.
> 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' ?
I have played with the idea about 'reverse gamma correction', it seemed
very attractive to me.
Roughly speaking, gamma > 1 makes black-on-white glyphs a bit narrower,
and white-no-black
glyphs a bit wider (bolder?), and it is the same effect that we need.
Unfortunately, direct comparison of black-on-white and white-on-black
glyphs makes me think
that difference between these glyphs can not be explained only by gamma
correction.
Please take a look at this screenshot:
http://cr.openjdk.java.net/~bae/8023794/bw-wb-comparision.png
<http://cr.openjdk.java.net/%7Ebae/8023794/bw-wb-comparision.png>
row 1: text is rendered by jdk6 as-is.
row 2: simulates our pipeline where black-on-white glyphs are used (max
error on white-on-black)
row 3: simulates our pipeline where white-on-black glyphs are used (max
error on black-on-white)
row 4: blended glyphs are used.
The basic idea of blending is to minimize max error (difference) between
produced glyph image
and original color-aware glyphs.
If better results can be achieved with 'reverse gamma correction', we
can revise this code later.
> And can (should) any of these changes also be applied to D3D ?
1. We can check how gamma correction is done in d3d. If a lookup is also
used there,
we can assess how rough the interpolation is.
2. translucent destination support (as a part of JDK-8078516)?
Thanks,
Andrew
>
> -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
>>>>>
>>>>>
>>>
>>
>
More information about the 2d-dev
mailing list