Hi Andrew, I have noticed an issue with quartz (at list with our way to use it).
This is a simple project that allows to illustrate the issue with a pure Quartz and Cocoa. https://github.com/denis-fokin/OffscreenVsQuartzFontRendering <https://github.com/denis-fokin/OffscreenVsQuartzFontRendering> Below you can see results. I used the same MBP with an external Thunderbolt display. Rendering of a glyph in the view and in the offscreen bitmap is the same for non-retina display. http://imgur.com/Y33C0tF,vnn5Ajt#0 <http://imgur.com/Y33C0tF,vnn5Ajt#0> In case of retina, it seems that the image uses improper number of pixels. http://imgur.com/Y33C0tF,vnn5Ajt#1 <http://imgur.com/Y33C0tF,vnn5Ajt#1> I see the same effect in the java-2d sub pixel rendering implementation. Right now I am not sure, how to handle this properly. Thank you, Denis. > On 24 Oct 2014, at 21:56, Andrew Brygin <andrew.bry...@oracle.com> wrote: > > Hello, > > please take a look to updated version of the fix: > > http://cr.openjdk.java.net/~bae/8023794/9/webrev.03/ > <http://cr.openjdk.java.net/~bae/8023794/9/webrev.03/> > > TODOs were removed: > * CGLSurfaceData.java > the condition for lcd rendering is inherited from OGL surface data, but > here we have to remove the check for the transparency, because we always > create transparent volatile images for swing backbuffers on macosx in > order > to support shaped windows. > > * CGGlyphImages.m and AWTStrike.m > NSException is used to handle invalid (unevaluated) text antialising > hint values. > We actually have already used NSException to handle memory allocation > failure, > so this change just makes usage of get/release of primitive arrays a > bit more safe. > > > Known differences comparing to apple jdk6: > > a) we do not interpret TEXT_ANTIALIASING_ON as lcd text. > > b) we do not render lcd text in a case of non-opaque paint. This behavior is > common for > all cases (software loops, OGL, and D3D), so it seems to deserve a > separate bug if we > want to handle this case. > > Thanks, > Andrew > > On 10/14/2014 3:39 AM, Phil Race wrote: >> On 10/13/2014 05:41 AM, Andrew Brygin wrote: >>> Hello Phil, >>> >>> please see my comments inline. >>> >>> On 10/13/2014 12:05 AM, Phil Race wrote: >>>> 373 fontHints.put(RenderingHints.KEY_ANTIALIASING, >>>> RenderingHints.VALUE_ANTIALIAS_ON); >>>> 374 fontHints.put(RenderingHints.KEY_TEXT_ANTIALIASING, >>>> RenderingHints.VALUE_TEXT_ANTIALIAS_LCD_HBGR); >>>> >>>> >>>> I am not sure why we have the graphics AA hint "ON" set here. >>>> Its not truly a font hint even though if its set it implcitly triggers >>>> font AA to be on too. >>>> If you remove that first line do you notice any problems with Swing apps ? >>> I failed to spot any difference in the SwingSet2 demo after removal the >>> line 373. >> >> Hmm .. Sergey seemed to suggest it was needed. >> I was concerned that it might subvert the text rendering hint >> as it used to cause us to do everything via the graphics AA loops. >> So I guess I am not sure what the impact of removing it will be >> You probably should check if it changes Metal & Nimbus rendering >> in SwingSet as it looks to be read by those but I don't see anywhere >> Aqua is reading these hints which is puzzling as you'd think that's >> where they were wanted. >> >>> >>>> >>>> >>>> The logic in the following code looks odd to me .. >>>> 310 static CGGI_GlyphInfoDescriptor grey = >>>> 311 { 1, &CGGI_CopyImageFromCanvasToAlphaInfo }; >>>> 312 static CGGI_GlyphInfoDescriptor rgb = >>>> 313 { 3, &CGGI_CopyImageFromCanvasToRGBInfo }; >>>> 314 >>>> 315 static inline CGGI_RenderingMode >>>> 316 CGGI_GetRenderingMode(const AWTStrike *strike) >>>> 317 { >>>> 318 CGGI_RenderingMode mode; >>>> 319 mode.cgFontMode = strike->fStyle; >>>> 320 >>>> 321 switch (strike->fAAStyle) { >>>> >>>> 322 case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_OFF: >>>> 323 case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_ON: >>>> >>>> >>>> 324 mode.glyphDescriptor = &grey; >>>> 325 break; >>>> 326 case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_LCD_HRGB: >>>> 327 case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_LCD_HBGR: >>>> 328 case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_LCD_VRGB: >>>> 329 case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_LCD_VBGR: >>>> 330 mode.glyphDescriptor = &rgb; >>>> 331 break; >>>> 332 case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_GASP: >>>> 333 case sun_awt_SunHints_INTVAL_TEXT_ANTIALIAS_DEFAULT: >>>> 334 default: >>>> 335 // TODO: generate an error? >>>> 336 break; >>>> 337 } >>>> >>>> >>>> .. If we get to default does this mean aliased rendering ? >>>> If so why would "OFF" go the grey path - seems it should go default >>>> And "GASP" looks like there is no support to specify the intended meaning >>>> there >>>> Given that OS X text is unhinted I'd think "ON" is a better option here >>>> even though most fonts would do "OFF" at typical screen sizes .. but that's >>>> because they are expecting hinted rendering. So I think you need to get >>>> hinted glyphs to default this to aliased .. >>> >>> If my understanding is correct, we never get TEXT_ANTIALIAS_GASP and >>> TEXT_ANTIALIAS_DEFAULT here, because these hints must be already evaluated >>> and translated to either TEXT_ANTIALIAS_OFF, or TEXT_ANTIALIAS_ON. >>> It should be done in SunGraphisc2D.checkFontInfo(), lines 699 - 775. >> >> this is probably right so its OK .. >> >> -phil. >> >>> >>> At this point we have no enough info to evaluate these hints properly, >>> and a best solution is probably to raise an error. >>> >>> Thanks, >>> Andrew >>> >>>> >>>> -phil. >>>> >>>> >>>> On 10/10/14 2:13 PM, Andrew Brygin wrote: >>>>> Hello Phil, >>>>> >>>>> please see my comments inline. >>>>> >>>>> On 10/11/2014 12:49 AM, Phil Race wrote: >>>>>> On 10/10/14 11:05 AM, Andrew Brygin wrote: >>>>>>> Hello Phil, >>>>>>> >>>>>>> changes in SunGraphics2D and SurfaceData were made in order to >>>>>>> implement an approach 'LCD instead of greyscale AA if possible'. >>>>>>> >>>>>>> Without this part of change, we get results according to the hints: >>>>>>> greyscale for TEXT_ANTIALIAS_ON, and subpixels for lcd hints: >>>>>>> >>>>>>> http://cr.openjdk.java.net/~bae/8023794/9/webrev.01/ >>>>>>> <http://cr.openjdk.java.net/%7Ebae/8023794/9/webrev.01/> >>>>>> >>>>>> So you are changing the meaning of application-specified "ON" across >>>>>> platforms ? >>>>>> That sounds wrong - even for Mac ... >>>>> >>>>> now I do not: the fix revision 01 does not change the meaning of the >>>>> hints, >>>>> and just makes text rendering on macosx to behave in a similar manner as >>>>> on windows. >>>>> >>>>> Please take a look at the webrev.01. >>>>> >>>>> Thanks, >>>>> Andrew >>>>>> >>>>>>> >>>>>>> I am not sure whether do we need to change the 'default' >>>>>>> behavior: at the moment it produces aliased text. >>>>>> >>>>>> default has meant aliased in all the non-mac sun/oracle implementations >>>>>> since JDK 1.2. We have talked about changing it to something >>>>>> more modern but should not do it lightly .. >>>>>> >>>>>> -phil. >>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> Andrew >>>>>>> >>>>>>> On 10/10/2014 8:54 PM, Phil Race wrote: >>>>>>>> I don't have my head around all these changes but a lot of it seems to >>>>>>>> imply we really weren't asking for LCDon Mac when we could/should. >>>>>>>> >>>>>>>> The change in the shared SurfaceData.javais something I want to >>>>>>>> ask about as you have commented out as follows .. >>>>>>>> ; >>>>>>>> 749 >>>>>>>> 750 // TODO: we have to take into account aaHint on macosx >>>>>>>> 751 //case SunHints.INTVAL_TEXT_ANTIALIAS_ON: >>>>>>>> 752 // return aaTextRenderer; >>>>>>>> >>>>>>>> ... >>>>>>>> >>>>>>>> that looks like the case where the application code has *explicitly* >>>>>>>> specified greyscale (ON==greyscale) so I don't see why you need >>>>>>>> to go check the fontinfo in this case ? >>>>>>>> >>>>>>>> -phil. >>>>>>>> >>>>>>>> On 10/10/2014 07:34 AM, Andrew Brygin wrote: >>>>>>>>> Hello Denis, >>>>>>>>> >>>>>>>>> could you please take a look at a preliminary version of the fix? >>>>>>>>> >>>>>>>>> http://cr.openjdk.java.net/~bae/8023794/9/webrev.00/ >>>>>>>>> <http://cr.openjdk.java.net/%7Ebae/8023794/9/webrev.00/> >>>>>>>>> >>>>>>>>> This fix promotes the text antialiasing from grayscale to LCD if >>>>>>>>> destination surface data is able to render LCD, and provides >>>>>>>>> selection of an appropriate text pipeline for both cases. >>>>>>>>> It also separates production of gatyscale and LCD glyph images. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Andrew >>>>>>>>> >>>>>>>>> On 10/9/2014 4:13 PM, Denis Fokin wrote: >>>>>>>>>> Hi Andrew, >>>>>>>>>> >>>>>>>>>> I am happy about you participation in this work! >>>>>>>>>> >>>>>>>>>> Looks like, I have missed this letter, while being sick. Sorry about >>>>>>>>>> this. >>>>>>>>>> >>>>>>>>>> I signed OCA, but I have not gotten access to cr.openjdk.java.net >>>>>>>>>> <http://cr.openjdk.java.net/> yet. This is the reason why I have not >>>>>>>>>> updated the webrev. >>>>>>>>>> >>>>>>>>>> I think that an API that is consistent with other platforms is a >>>>>>>>>> great solution. It just requires more efforts and multi-platform >>>>>>>>>> testing. On the other hand, a property could be a safe start. >>>>>>>>>> >>>>>>>>>> As for the offscreen rendering, I have some kind of a workaround >>>>>>>>>> with the next approach. >>>>>>>>>> >>>>>>>>>> In BufImgSurfaceData.getRenderLoops() I always return >>>>>>>>>> super.getRenderLoops(sg2d), and never solid loops. >>>>>>>>>> >>>>>>>>>> In case if “useQuartz" flag is specified, I return only >>>>>>>>>> lcdTextRenderer from SurfaceData.getTextPipe() >>>>>>>>>> >>>>>>>>>> Of course it is a brute force approach, but it allows producing a >>>>>>>>>> legible text in case of offscreen rendering. >>>>>>>>>> >>>>>>>>>> Thank you, >>>>>>>>>> Denis. >>>>>>>>>> >>>>>>>>>> On 29 Sep 2014, at 19:30, Andrew Brygin <andrew.bry...@oracle.com >>>>>>>>>> <mailto:andrew.bry...@oracle.com>> wrote: >>>>>>>>>> >>>>>>>>>>> Hello Denis, >>>>>>>>>>> >>>>>>>>>>> I am not sure whether we should use 'apple.awt.graphics.UseQuartz' >>>>>>>>>>> property. >>>>>>>>>>> Probably we have to change the text antialiasing defaults for >>>>>>>>>>> macosx instead. >>>>>>>>>>> >>>>>>>>>>> I am working on the issue with software loops. I will update the >>>>>>>>>>> thread >>>>>>>>>>> with my findings. >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> Andrew >>>>>>>>>>> >>>>>>>>>>> On 9/3/2014 3:32 PM, Denis Fokin wrote: >>>>>>>>>>>> Hi Sergey and 2d team, >>>>>>>>>>>> >>>>>>>>>>>> I have rewritten the fix. It works fine for text rendered on >>>>>>>>>>>> window using OpenGL. >>>>>>>>>>>> >>>>>>>>>>>> http://web-dot.ru/openjdk/8023794/webrev.00/index.html >>>>>>>>>>>> <http://web-dot.ru/openjdk/8023794/webrev.00/index.html> >>>>>>>>>>>> >>>>>>>>>>>> It is incomplete though. It does not work for rendering in a >>>>>>>>>>>> buffered image. >>>>>>>>>>>> >>>>>>>>>>>> Additionally, I have not tested the code on other platforms except >>>>>>>>>>>> MacOS X. >>>>>>>>>>>> >>>>>>>>>>>> To enable the antialiasing you should pass >>>>>>>>>>>> >>>>>>>>>>>> -Dapple.awt.graphics.UseQuartz=true >>>>>>>>>>>> >>>>>>>>>>>> to java. >>>>>>>>>>>> >>>>>>>>>>>> The current issue now is the glyph info bytes that are passed from >>>>>>>>>>>> CGGlyphImage to AATextRenderer. >>>>>>>>>>>> >>>>>>>>>>>> To render data we use DEFINE_SOLID_DRAWGLYPHLIST* macros. Basing >>>>>>>>>>>> on the macros a set of functions is generated for the next loops. >>>>>>>>>>>> >>>>>>>>>>>> sun/java2d/loops/ByteGray.c >>>>>>>>>>>> sun/java2d/loops/ByteIndexed.c >>>>>>>>>>>> sun/java2d/loops/FourByteAbgr.c >>>>>>>>>>>> sun/java2d/loops/FourByteAbgrPre.c >>>>>>>>>>>> sun/java2d/loops/Index12Gray.c >>>>>>>>>>>> sun/java2d/loops/Index8Gray.c >>>>>>>>>>>> sun/java2d/loops/IntArgb.c >>>>>>>>>>>> sun/java2d/loops/IntArgbBm.c >>>>>>>>>>>> sun/java2d/loops/IntArgbPre.c >>>>>>>>>>>> sun/java2d/loops/IntBgr.c >>>>>>>>>>>> sun/java2d/loops/IntRgb.c >>>>>>>>>>>> sun/java2d/loops/IntRgbx.c >>>>>>>>>>>> sun/java2d/loops/LoopMacros.h >>>>>>>>>>>> sun/java2d/loops/ThreeByteBgr.c >>>>>>>>>>>> sun/java2d/loops/Ushort555Rgb.c >>>>>>>>>>>> sun/java2d/loops/Ushort555Rgbx.c >>>>>>>>>>>> sun/java2d/loops/Ushort565Rgb.c >>>>>>>>>>>> sun/java2d/loops/UshortGray.c >>>>>>>>>>>> sun/java2d/loops/UshortIndexed.c >>>>>>>>>>>> >>>>>>>>>>>> For instance, C preprocessor generates the next code for IntRgb.c >>>>>>>>>>>> >>>>>>>>>>>> void IntRgbDrawGlyphListLCD(/*…*/){ >>>>>>>>>>>> jint glyphCounter, bpp; >>>>>>>>>>>> jint scan = pRasInfo->scanStride; >>>>>>>>>>>> IntRgbDataType *pPix; >>>>>>>>>>>> fprintf(__stderrp, "NAME_SOLID_DRAWGLYPHLISTLC\n"); >>>>>>>>>>>> jint srcA; >>>>>>>>>>>> jint srcR , srcG, srcB;;;; >>>>>>>>>>>> do { >>>>>>>>>>>> (srcB) = (argbcolor) & 0xff; >>>>>>>>>>>> (srcG) = ((argbcolor) >> 8) & 0xff; >>>>>>>>>>>> (srcR) = ((argbcolor) >> 16) & 0xff; >>>>>>>>>>>> (srcA) = ((argbcolor) >> 24) & 0xff; >>>>>>>>>>>> } while (0);; >>>>>>>>>>>> // and so on… >>>>>>>>>>>> >>>>>>>>>>>> Looks like rgb loop expects to see 4 8-bit color channels per >>>>>>>>>>>> pixel. >>>>>>>>>>>> >>>>>>>>>>>> For now, I do not understand which contract should be honoured to >>>>>>>>>>>> meet DEFINE_SOLID_DRAWGLYPHLIST* expectations, i.e. how should I >>>>>>>>>>>> place bytes in GlyphInfo. >>>>>>>>>>>> >>>>>>>>>>>> May be it should be set somewhere in Java code. >>>>>>>>>>>> >>>>>>>>>>>> Could anyone share this knowledge with me? >>>>>>>>>>>> >>>>>>>>>>>> Thank you, >>>>>>>>>>>> Denis. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On 09 Jul 2014, at 19:22, Sergey Bylokhov >>>>>>>>>>>> <sergey.bylok...@oracle.com <mailto:sergey.bylok...@oracle.com>> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hello, Denis. >>>>>>>>>>>>> Thanks for this research! >>>>>>>>>>>>> On 09.07.2014 15:13, Denis Fokin wrote: >>>>>>>>>>>>>> The current version consist of three parts. >>>>>>>>>>>>>> >>>>>>>>>>>>>> 1. We are rendering glyphs in offscreen images using Quartz >>>>>>>>>>>>>> functions. This does not work without kCGBitmapByteOrder32Host >>>>>>>>>>>>>> mask. >>>>>>>>>>>>> I assume LCD hint does not work? this looks good. >>>>>>>>>>>>>> >>>>>>>>>>>>>> 2. We assume that subpixel antialiasing should not be used on a >>>>>>>>>>>>>> non-opaque surface. As I understand the vImage in >>>>>>>>>>>>>> CGLVolatileSurfaceManager is not related directly to our window. >>>>>>>>>>>>>> For a start, I have hardcoded Transparency.OPAQUE, but it >>>>>>>>>>>>>> requires much better understanding of the architecture to make a >>>>>>>>>>>>>> more proper solution. >>>>>>>>>>>>> It is related to the CGLOffScreenSurfaceData, which is used as a >>>>>>>>>>>>> surface for VolatileImages. I check this code and looks like we >>>>>>>>>>>>> ignore type of the ColorModel and create a transparent native >>>>>>>>>>>>> texture anyway. >>>>>>>>>>>>>> >>>>>>>>>>>>>> 3. When I started using CGGI_CopyImageFromCanvasToRGBInfo as a >>>>>>>>>>>>>> rendering mode, I had found that the little endian mode should >>>>>>>>>>>>>> be undefined. Again, it might be an improper way to do this. >>>>>>>>>>>>> It seems __LITTLE_ENDIAN__usage in this file should be checked. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thank you, >>>>>>>>>>>>>> Denis. >>>>>>>>>>>>>> >>>>>>>>>>>>>> diff -r f87c5be90e01 >>>>>>>>>>>>>> src/macosx/classes/sun/java2d/opengl/CGLVolatileSurfaceManager.java >>>>>>>>>>>>>> >>>>>>>>>>>>>> --- >>>>>>>>>>>>>> a/src/macosx/classes/sun/java2d/opengl/CGLVolatileSurfaceManager.javaFri >>>>>>>>>>>>>> Jun 20 10:15:30 2014 -0700 >>>>>>>>>>>>>> >>>>>>>>>>>>>> +++ >>>>>>>>>>>>>> b/src/macosx/classes/sun/java2d/opengl/CGLVolatileSurfaceManager.javaWed >>>>>>>>>>>>>> Jul 09 14:50:09 2014 +0400 >>>>>>>>>>>>>> >>>>>>>>>>>>>> @@ -108,7 +108,7 @@ >>>>>>>>>>>>>> >>>>>>>>>>>>>> } else { >>>>>>>>>>>>>> >>>>>>>>>>>>>> CGLGraphicsConfig gc = >>>>>>>>>>>>>> >>>>>>>>>>>>>> (CGLGraphicsConfig)vImg.getGraphicsConfig(); >>>>>>>>>>>>>> >>>>>>>>>>>>>> - ColorModel cm = >>>>>>>>>>>>>> gc.getColorModel(vImg.getTransparency()); >>>>>>>>>>>>>> >>>>>>>>>>>>>> + ColorModel cm = >>>>>>>>>>>>>> gc.getColorModel(Transparency.OPAQUE); >>>>>>>>>>>>>> >>>>>>>>>>>>>> int type = vImg.getForcedAccelSurfaceType(); >>>>>>>>>>>>>> >>>>>>>>>>>>>> // if acceleration type is forced (type != >>>>>>>>>>>>>> UNDEFINED) then >>>>>>>>>>>>>> >>>>>>>>>>>>>> // use the forced type, otherwise choose one >>>>>>>>>>>>>> based on caps >>>>>>>>>>>>>> >>>>>>>>>>>>>> diff -r f87c5be90e01 src/macosx/native/sun/font/CGGlyphImages.m >>>>>>>>>>>>>> >>>>>>>>>>>>>> --- a/src/macosx/native/sun/font/CGGlyphImages.mFri Jun 20 >>>>>>>>>>>>>> 10:15:30 2014 -0700 >>>>>>>>>>>>>> >>>>>>>>>>>>>> +++ b/src/macosx/native/sun/font/ .mWed Jul 09 14:50:09 2014 >>>>>>>>>>>>>> +0400 >>>>>>>>>>>>>> >>>>>>>>>>>>>> @@ -196,6 +196,8 @@ >>>>>>>>>>>>>> >>>>>>>>>>>>>> #pragma mark --- Font Rendering Mode Descriptors --- >>>>>>>>>>>>>> >>>>>>>>>>>>>> +#undef __LITTLE_ENDIAN__ >>>>>>>>>>>>>> >>>>>>>>>>>>>> + >>>>>>>>>>>>>> >>>>>>>>>>>>>> static inline void >>>>>>>>>>>>>> >>>>>>>>>>>>>> CGGI_CopyARGBPixelToRGBPixel(const UInt32 p, UInt8 *dst) >>>>>>>>>>>>>> >>>>>>>>>>>>>> { >>>>>>>>>>>>>> >>>>>>>>>>>>>> @@ -366,7 +368,8 @@ >>>>>>>>>>>>>> >>>>>>>>>>>>>> canvas->context = CGBitmapContextCreate(canvas->image->data, >>>>>>>>>>>>>> >>>>>>>>>>>>>> width, height, 8, bytesPerRow, >>>>>>>>>>>>>> >>>>>>>>>>>>>> colorSpace, >>>>>>>>>>>>>> >>>>>>>>>>>>>> - kCGImageAlphaPremultipliedFirst); >>>>>>>>>>>>>> >>>>>>>>>>>>>> + kCGImageAlphaPremultipliedFirst >>>>>>>>>>>>>> >>>>>>>>>>>>>> + | >>>>>>>>>>>>>> kCGBitmapByteOrder32Host); >>>>>>>>>>>>>> >>>>>>>>>>>>>> CGContextSetRGBFillColor(canvas->context, 0.0f, 0.0f, 0.0f, >>>>>>>>>>>>>> 1.0f); >>>>>>>>>>>>>> >>>>>>>>>>>>>> CGContextSetFontSize(canvas->context, 1); >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> -- >>>>>>>>>>>>> Best regards, Sergey. >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >