[OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON
Phil Race
philip.race at oracle.com
Wed May 27 16:31:24 UTC 2015
OK .. it's a step forward, so approved, although there is more to be done.
I do ask that you however restore this comment until you actually remove the constraint :
405 * - and the destination is opaque
-phil.
On 05/21/2015 07:23 AM, Andrew Brygin wrote:
> Hello Phil,
>
> I have changed the default reverse gamma to 180:
>
> http://cr.openjdk.java.net/~bae/8023794/9/webrev.10/
>
> I also did some experiments with the lcd contrast, and found that
> value 160 decreases the discrepancy a bit: 21 instead of 29 with
> the default lcd contrast value (140).
> However, there still is the discrepancy, and at the moment I do not
> see how we can avoid it completely with our rendering model.
> It looks like that there is something more sophisticated than
> just gamma correction, and we are unable to derive 'color independent'
> glyphs from black-on-white glyphs provided by the native system.
>
> I have also played with changing display profiles but it seems to
> have no detectable effect.
>
> Thanks,
> Andrew
>
> On 5/18/2015 11:19 PM, Phil Race wrote:
>> Hi,
>>
>> So 1.79 is essentially 1.8 which is what Mac historically used as
>> gamma. So I'd pick 180 instead of 179
>> Since that value minimises the discrepancy it's looking positive but
>> there's still a discrepancy.
>> Before we 'live with it', I'd be interested to know what happens if
>> you set your display's profile to a generic RGB one.
>>
>> How do the glyph shapes (if you try your best to ignore the colour)
>> match what other apps produce ?
>>
>> -phil.
>>
>>
>> On 04/30/2015 06:21 AM, Andrew Brygin wrote:
>>> Hello Phil,
>>>
>>> please take a look to updated version of the fix:
>>>
>>> http://cr.openjdk.java.net/~bae/8023794/9/webrev.09/
>>>
>>> The main difference is in glyph generation. I have implemented
>>> 'reverse gamma correction'
>>> approach instead of 'glyph blending'. By default we use gamma value
>>> which provides minimum
>>> of discrepancy with gyph images produced by Apple's jdk. However,
>>> it can be adjusted via
>>> environment variable J2D_LCD_REVERSE_GAMMA (CGGlyphImages.m, lines
>>> 198 - 220).
>>>
>>> Following chart illustrates dependency between the 'reverse gamma'
>>> and a difference
>>> in pixel values comparing to jdk6 from Apple:
>>> http://cr.openjdk.java.net/~bae/8023794/best_reverse_gamma.png
>>>
>>> Following set of images has been used for the comparison:
>>> http://cr.openjdk.java.net/~bae/8023794/images/
>>> An index of image corresponds to the value of reverse gamma used
>>> for image
>>> generation.
>>>
>>> Beside this, following minor changes were done:
>>> * RenderingHints.KEY_ANTIALIASING was removed form fontHints,
>>> because it has no direct relation to text rendering, and does
>>> not have
>>> any effect at the moment.
>>> * A comment regarding unevaluated rendering hints was added.
>>>
>>> Please take a look.
>>>
>>> Thanks,
>>> Andrew
>>>
>>> On 4/24/2015 7:33 PM, Andrew Brygin wrote:
>>>> 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