This looks extremely promising; https://bugzilla-attachments-216655.netbeans.org/bugzilla/attachment.cgi?id=153888
I'd say don't let perfect be the enemy of the good, please get this out into a jdk9 release and let the community provide more feedback. On 22 May 2015 at 00:23, Andrew Brygin <andrew.bry...@oracle.com> 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 >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > -- -Tor
