[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
Fri Oct 24 17:56:28 UTC 2014
Hello,
please take a look to updated version of the fix:
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/
>>>>>
>>>>> 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/20141024/45377150/attachment.html>
More information about the 2d-dev
mailing list