[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
Mon Oct 13 12:41:03 UTC 2014
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.
>
>
> 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.
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/0255a2ec/attachment.html>
More information about the 2d-dev
mailing list