[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
Sun Oct 12 20:05:47 UTC 2014
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 ?
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 ..
-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/20141012/e88759b8/attachment.html>
More information about the 2d-dev
mailing list