Re: [OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON

2014-11-06 Thread Denis S. Fokin
Hi Andrew,

I have added an OpenGL view in the project  just to illustrate that OpenGL 
provides exactly the same results.

I also wrote a simple test the results are below

http://i.imgur.com/cEt6HYo.png 

The letters are written with the same font and the same size

From left to right

1. Panel Graphics
2. BufferedImage  Graphics -> Panel Graphics
3. NSView
4. CGImageRef -> NSView
5. CGImageRef -> OpenGL

1,3,5 give the same results

2,4 are give the same results but a little bit brighter than 1,3,5

I would prefer more consistent results. But again I am not sure how to handle 
this. Might be we should use some kind of “compatible context” and proper 
colorspace.

Thank you,
Denis.

Looks like the same 

> On 05 Nov 2014, at 16:57, Denis S. Fokin  wrote:
> 
> Hi Andrew,
> 
> I have noticed an issue with quartz (at list with our way to use it). 
> 
> This is a simple project that allows to illustrate the issue with a pure 
> Quartz and Cocoa.
> 
> https://github.com/denis-fokin/OffscreenVsQuartzFontRendering 
> 
> 
> Below you can see results.
> 
> I used the same MBP with an external Thunderbolt display.
> 
> Rendering of a glyph in the view and in the offscreen bitmap is the same for 
> non-retina display.
> 
> http://imgur.com/Y33C0tF,vnn5Ajt#0 
> 
> In case of retina, it seems that the image uses improper number of pixels.
> 
> http://imgur.com/Y33C0tF,vnn5Ajt#1 
> 
> I see the same effect in the java-2d sub pixel rendering implementation.
> 
> Right now I am not sure, how to handle this properly.
> 
> Thank you,
>Denis.
> 
> 
>> On 24 Oct 2014, at 21:56, Andrew Brygin > > wrote:
>> 
>> 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_ANTIA

Re: [OpenJDK 2D-Dev] RFR: 8062163: java/awt/geom/AffineTransform/TestInvertMethods.java test fails

2014-11-06 Thread Jennifer Godinez

Looks good.

Jennifer
On 11/06/2014 10:50 AM, Phil Race wrote:

Any takers ?

-phil.

On 10/30/2014 11:14 AM, Phil Race wrote:

https://bugs.openjdk.java.net/browse/JDK-8062163
http://cr.openjdk.java.net/~prr/8062163/

Fixes a test that currently fails because of a JDK9 b33 change
in the implementation of Math.toRadians(double)

-phil.






Re: [OpenJDK 2D-Dev] RFR: 8062163: java/awt/geom/AffineTransform/TestInvertMethods.java test fails

2014-11-06 Thread Phil Race

Any takers ?

-phil.

On 10/30/2014 11:14 AM, Phil Race wrote:

https://bugs.openjdk.java.net/browse/JDK-8062163
http://cr.openjdk.java.net/~prr/8062163/

Fixes a test that currently fails because of a JDK9 b33 change
in the implementation of Math.toRadians(double)

-phil.




Re: [OpenJDK 2D-Dev] [9] Review Request: 8061832 J2DBench can be improved

2014-11-06 Thread Phil Race

  82 if (ImageTests.hasVolatileImage) {
  83 if(ImageTests.hasTransparentVolatileImage) {
  84 volimgdestroot = new Group.EnableSet(destroot, 
"volimg",
  85  "Output to Volatile 
Image");
  86
  87 volimgdestroot.setHorizontal();
  88 new VolatileImg();
  89 new VolatileImg(Transparency.OPAQUE);
  90 new VolatileImg(Transparency.BITMASK);
  91 new VolatileImg(Transparency.TRANSLUCENT);
  92 } else if (ImageTests.hasVolatileImage){
  93 volimgdestroot = new Group.EnableSet(destroot, 
"volimg",
  94  "Output to Volatile 
Image");
  95
  96 volimgdestroot.setHorizontal();
  97 new VolatileImg();
  98 }
  99 }

The test at line 93 is redundant .. it just needs to be an else { ..}
since we are only in this block if it passed at line 82.
Also line 83 has "if(" - should be "if ("

Am I right in understanding that the huge 4000x4000 option is disabled by 
default ?
I think disabling by default is the right thing for that.

-phil

On 11/5/2014 11:17 AM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 9.

Change description:
- Test groups were wrapped by JScrollPane
- Translucent volatile images now can be used as source and destination
- Additional source/destination were added(TYPE_4BYTE_**)
- Main window will be created on EDT at the center of the screen
- Support of the huge shapes were added(4000x4000)
- Support of the interpolation was added, by default nearest neighbor 
is used
- Two cmm tests depended on the default enum.toString() 
implementation, and was broken when usage of enums were removed. I 
reused abbreviation as a name of the test
- "Image Rendering Tests" tab was renamed to "image rendering 
benchmark", because now it has an options as well
- "Touch src image before every operation" was moved from the "image 
rendering source" tab to the "image rendering benchmarks" and placed 
in the same group as interpolation
- The new image rendering test was added: drawimage(img, x, y, w*0.5, 
h*0.5, obs); will simplify testing of some blits on the retina display



Bug: https://bugs.openjdk.java.net/browse/JDK-8061832
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8061832/webrev.02