[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
Mon Oct 13 23:39:24 UTC 2014
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/
>>>>
>>>> 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/
>>>>>>>
>>>>>>> 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.brygin at oracle.com <mailto:andrew.brygin at 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
>>>>>>>>>>
>>>>>>>>>> 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
>>>>>>>>>>
>>>>>>>>>> voidIntRgbDrawGlyphListLCD(/*…*/){
>>>>>>>>>> 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.Bylokhov at oracle.com
>>>>>>>>>> <mailto:Sergey.Bylokhov at 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.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20141013/1698be2f/attachment.html>
More information about the 2d-dev
mailing list