Re: [OpenJDK 2D-Dev] RFR: 8059848 Test java/awt/GraphicsDevice/CloneConfigsTest.java causes JVM crash in OEL 7.0
Hi, Phil. The fix looks good.
Re: [OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON
Hi, Phil.
I guess that on osx defaults were changed to something more modern long time
ago. Including all types of rendering. In jdk6 it was done in the graphics
itself, but during macport the decision not to change SG2D was made, but use
DesktopProperty instead.
It demanded changes in Aqua LaF, which was changed to use
SwingUtilities2.drawString instead of g.drawString.
http://hg.openjdk.java.net/macosx-port/macosx-port/jdk/rev/55b73c5d1c93
Interesting is that in this time HRGB(which did not work) was changed to ON.
http://hg.openjdk.java.net/macosx-port/macosx-port/jdk/rev/e45f5175d849
- philip.r...@oracle.com 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 ?
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
Re: [OpenJDK 2D-Dev] RFR: JDK-8057830: Crash in Java2D Queue Flusher, OGLSD_SetScratchSurface
Hey, I know it's only prio 4 in the database. But since Denis already found this good and it's a tiny fix, can someone with reviewer status please spend the 10 minutes and check this out? I (and real customers) would very much appreciate it! Thank you. -hendrik On Oct 1, 2014, at 17:03, Hendrik Schreiber h...@tagtraum.com wrote: Thanks for the endorsement, Denis. Much appreciated! -hendrik On Oct 1, 2014, at 13:19, Denis Fokin denis.fo...@gmail.com wrote: Hi Hendrick, I am not a reviewer, but the fix looks good to me. Thank you, Denis. On 22 Sep 2014, at 12:19, Hendrik Schreiber h...@tagtraum.com wrote: Hi, Bug: https://bugs.openjdk.java.net/browse/JDK-8057830 On OS X, CGLGraphicsConfigInfo is destroyed by OGLGC_DestroyOGLGraphicsConfig, however the pointer to it still hangs around for a while and is not reset to NULL, until we get rid of it with a Disposer later on. In the meantime it appears to be possible that OGLSD_SetScratchSurface is called with the already destroyed CGLGraphicsConfigInfo as argument. The CGLGraphicsConfigInfo is not NULL, but its structs are in a bad state, most likely freed, leading to the observed crash. The suggested change does not solve the problem, of needing to NULL the pointer to CGLGraphicsConfigInfo right where it's destroyed in OGLRenderQueue.c (not really possible IMO). However, it improves the destruction by NULLing some of it struct members and thus allowing us in OGLSD_SetScratchSurface to test those for NULL values. I also added a trace call for when this happens, to aiding potentially creating a better fix in the future. Unfortunately, I have not been able to come up with a reasonable unit test for this, therefore I cannot be certain that it solves the problem. However, as the changes are minimal and obviously harmless, I would very much appreciate it, if somebody decided to sponsor and commit this patch. I have a live application out there based on 8u20 and this is the number one reason for user-reported crashes. Webrev: https://www.beatunes.com/download/webrev_8057830.zip I did a full clean OS X build to test the change. Before and after I encountered the same 11 jdk_2d failures (which made me wonder whether that is normal...). Cheers, -hendrik PS: I'm new to contributing OpenJDK patches, let me know, if I should have done it some other way. Thanks.
Re: [OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON
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
[OpenJDK 2D-Dev] RFR: 8058969 : Test closed/sun/java2d/cmm/StubCMMShellTest.sh fails
https://bugs.openjdk.java.net/browse/JDK-8058969 http://cr.openjdk.java.net/~prr/8058969/ This is a missing doprivileged in code updated to support jigsaw -phil.
Re: [OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON
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
Re: [OpenJDK 2D-Dev] RFR: 8058969 : Test closed/sun/java2d/cmm/StubCMMShellTest.sh fails
Hello Phil, the fix looks fine to me. Thanks, Andrew On 10/14/2014 3:27 AM, Phil Race wrote: https://bugs.openjdk.java.net/browse/JDK-8058969 http://cr.openjdk.java.net/~prr/8058969/ This is a missing doprivileged in code updated to support jigsaw -phil.
