Re: [OpenJDK 2D-Dev] RFR: 8171303: sun/java2d/pipe/InterpolationQualityTest.java fails on Windows & Linux [v2]

2020-09-09 Thread Kevin Rushforth
On Wed, 9 Sep 2020 23:17:21 GMT, Sergey Bylokhov  wrote:

>> This the only test which was created to check different types of 
>> interpolation.
>> It checks the rendering to the VolatileImage and uses BufferedImage as a 
>> gold image.
>> But it does not take into account that rendering to the VolatileImage might 
>> be affected
>> by the HiDPI support.
>> 
>> Old review request:
>> https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/010982.html
>
> Sergey Bylokhov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   @author tag is removed

Marked as reviewed by kcr (no project role).

-

PR: https://git.openjdk.java.net/jdk/pull/86


Re: [OpenJDK 2D-Dev] RFR: 8171303: sun/java2d/pipe/InterpolationQualityTest.java fails on Windows & Linux [v2]

2020-09-09 Thread Sergey Bylokhov
> This the only test which was created to check different types of 
> interpolation.
> It checks the rendering to the VolatileImage and uses BufferedImage as a gold 
> image.
> But it does not take into account that rendering to the VolatileImage might 
> be affected
> by the HiDPI support.
> 
> Old review request:
> https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/010982.html

Sergey Bylokhov has updated the pull request incrementally with one additional 
commit since the last revision:

  @author tag is removed

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/86/files
  - new: https://git.openjdk.java.net/jdk/pull/86/files/2e6a0251..99235829

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=86&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=86&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/86.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/86/head:pull/86

PR: https://git.openjdk.java.net/jdk/pull/86


Re: [OpenJDK 2D-Dev] RFR: 7183828: Invalid Image Variant when using anything other than BufferedImage

2020-09-09 Thread Phil Race
On Tue, 8 Sep 2020 21:16:10 GMT, Sergey Bylokhov  wrote:

> It is intended that our draw machinery works only on specific image formats 
> that we know we support.
> But the user still able to create some image subclasses for their purpose and 
> according to our spec of
> Graphics2D.drawImage() we should not throw any exceptions.
> 
> I suggest handling this situation in a similar way as we handle some errors 
> during rendering when
> the pipeline is in the wrong state, or the ToolkitImage is not loaded yet, 
> just ignore the request and
> return false.
> 
> All our pipelines have a special meaning of InvalidPipeException, if the 
> pipeline found that it cannot complete the draw
> operation throws this exception which is handled by all methods in the 
> SunGraphics2D class.
> 
> So as a fix I suggest changing the IllegalArgumentException to the 
> InvalidPipeException.
> Also, we need to add a try/catch block to the drawHiDPIImage(it uses the 
> SurfaceManager.getManager method directly)
> 
> 
> Old review request:
> https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/010995.html

Ok. Fair enough. Please copy those spec. excerpts into the bug report.

-

Marked as reviewed by prr (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/85


Re: [OpenJDK 2D-Dev] RFR: 7183828: Invalid Image Variant when using anything other than BufferedImage

2020-09-09 Thread Sergey Bylokhov
On Wed, 9 Sep 2020 16:50:22 GMT, Phil Race  wrote:

> I have very mixed feelings about this whole idea.
> InvalidPipeException is being co-opted for a different purpose.

We already use this exception in such cases, and I think it is intended for 
this:
https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/sun/java2d/opengl/OGLMaskFill.java#L75
https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/sun/java2d/NullSurfaceData.java#L92

> The user no longer gets a clear message that their image type isn't supported.
> Is there any specification we can point to ?

The spec for the Image class:
>  * The abstract class {@code Image} is the superclass of all
>  * classes that represent graphical images. The image must be
>  * obtained in a platform-specific manner.
https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/java/awt/Image.java#L40

Or the spec for the VolatileImage
>  * This image should not be subclassed directly but should be created
>  * by using the {@link java.awt.Component#createVolatileImage(int, int)
>  * Component.createVolatileImage} or
>  * {@link java.awt.GraphicsConfiguration#createCompatibleVolatileImage(int, 
> int)
>  * GraphicsConfiguration.createCompatibleVolatileImage(int, int)} methods.
https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/java/awt/image/VolatileImage.java#L74

-

PR: https://git.openjdk.java.net/jdk/pull/85


Re: [OpenJDK 2D-Dev] RFR: 8171303: sun/java2d/pipe/InterpolationQualityTest.java fails on Windows & Linux

2020-09-09 Thread Kevin Rushforth
On Tue, 8 Sep 2020 21:54:43 GMT, Sergey Bylokhov  wrote:

> This the only test which was created to check different types of 
> interpolation.
> It checks the rendering to the VolatileImage and uses BufferedImage as a gold 
> image.
> But it does not take into account that rendering to the VolatileImage might 
> be affected
> by the HiDPI support.
> 
> Old review request:
> https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/010982.html

Marked as reviewed by kcr (no project role).

-

PR: https://git.openjdk.java.net/jdk/pull/86


Re: [OpenJDK 2D-Dev] RFR: 7183828: Invalid Image Variant when using anything other than BufferedImage

2020-09-09 Thread Phil Race
On Tue, 8 Sep 2020 21:16:10 GMT, Sergey Bylokhov  wrote:

> It is intended that our draw machinery works only on specific image formats 
> that we know we support.
> But the user still able to create some image subclasses for their purpose and 
> according to our spec of
> Graphics2D.drawImage() we should not throw any exceptions.
> 
> I suggest handling this situation in a similar way as we handle some errors 
> during rendering when
> the pipeline is in the wrong state, or the ToolkitImage is not loaded yet, 
> just ignore the request and
> return false.
> 
> All our pipelines have a special meaning of InvalidPipeException, if the 
> pipeline found that it cannot complete the draw
> operation throws this exception which is handled by all methods in the 
> SunGraphics2D class.
> 
> So as a fix I suggest changing the IllegalArgumentException to the 
> InvalidPipeException.
> Also, we need to add a try/catch block to the drawHiDPIImage(it uses the 
> SurfaceManager.getManager method directly)
> 
> 
> Old review request:
> https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/010995.html

I have very mixed feelings about this whole idea.
InvalidPipeException is being co-opted for a different purpose.
The user no longer gets a clear message that their image type isn't supported.
Is there any specification we can point to ?

-

PR: https://git.openjdk.java.net/jdk/pull/85


Re: [OpenJDK 2D-Dev] RFR: 8171303: sun/java2d/pipe/InterpolationQualityTest.java fails on Windows & Linux

2020-09-09 Thread Prasanta Sadhukhan
On Tue, 8 Sep 2020 21:54:43 GMT, Sergey Bylokhov  wrote:

> This the only test which was created to check different types of 
> interpolation.
> It checks the rendering to the VolatileImage and uses BufferedImage as a gold 
> image.
> But it does not take into account that rendering to the VolatileImage might 
> be affected
> by the HiDPI support.
> 
> Old review request:
> https://mail.openjdk.java.net/pipermail/2d-dev/2020-August/010982.html

Looks ok albeit with author change in the test.

test/jdk/sun/java2d/pipe/InterpolationQualityTest.java line 31:

> 29:  * image via rendering hints for default, xrender and opengl pipelines.
> 30:  *
> 31:  * @author vadim.pakhnus...@oracle.com

I think we are supposed to remove the author, if present historically, if we 
are making changes to any test.
rest looks ok.

-

Marked as reviewed by psadhukhan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/86