On Mon, 29 Jun 2020 14:02:11 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

>> I went ahead and wrote a bunch of tests that:
>> 1. Setup a scene to display an `ImageView` of a selected dimensions chosen 
>> to trigger tiling in different ways when
>> taking snapshots. 2. Fill up the image with noise.
>> 3. Take a snapshot and do a pixel-wise comparison with the original image.
>> 
>> I've added the new tests to the existing `Snapshot2Test.java`.
>
> I observed that the added tests are failing on mac machine(Mojave 10.14.6), 
> but they do pass on windows10. Can you
> please verify ? Timeout and Unrecognized Image loader errors from the log,
> test.javafx.scene.Snapshot2Test > testSnapshot2x2TilesSameWidthImm[0] FAILED
>     java.lang.IllegalArgumentException: Unrecognized image loader: null
>         at 
> javafx.graphics/javafx.scene.image.WritableImage.loadTkImage(WritableImage.java:278)
>         at 
> javafx.graphics/javafx.scene.image.WritableImage$1.loadTkImage(WritableImage.java:53)
>         at javafx.graphics/javafx.scene.Scene.doSnapshot(Scene.java:1342)
>         at javafx.graphics/javafx.scene.Node.doSnapshot(Node.java:2136)
>         at javafx.graphics/javafx.scene.Node.snapshot(Node.java:2214)
>         at 
> test.javafx.scene.Snapshot2Test.lambda$doTestTiledSnapshotImm$12(Snapshot2Test.java:364)
> 
> test.javafx.scene.Snapshot2Test > testSnapshot2x1TilesSameSizeDefer[0] FAILED
>     junit.framework.AssertionFailedError: Timeout waiting for snapshot 
> callback
>         at 
> test.javafx.scene.SnapshotCommon.runDeferredSnapshotWait(SnapshotCommon.java:157)
>         at 
> test.javafx.scene.SnapshotCommon.runDeferredSnapshotWait(SnapshotCommon.java:183)
>         at 
> test.javafx.scene.Snapshot2Test.doTestTiledSnapshotDefer(Snapshot2Test.java:378)
>         at 
> test.javafx.scene.Snapshot2Test.testSnapshot2x1TilesSameSizeDefer(Snapshot2Test.java:459)
> 
> I would suggest to fill the image with a single or preferably some pattern of 
> multiple recognizable colors instead of
> noise. The noise gives a broken feel. It might confuse the verifications of 
> any future fixes in this area. A well
> defined color would be nice.

Thanks for your review.

I don't have access to a Mac so I can't check that directly.
The tests pass on both my Windows 10 and Ubuntu 20.04 environments and the 
stack isn't very helpful (it simply
indicates the renderImage method returned null).
Running the test with the unpatched version of the 
`QuantumToolkit::renderToImage` method would typically result in
such a stack, but there are many other possible reasons.

With regard to the choice of random noise for the test image, my idea was to be 
able to catch any misalignment caused
by tiling in the final snapshot: using a single color or a simple pattern would 
not necessarily help catching such
errors. Using a complex image could do the trick, and avoid the broken aspect 
you mentioned, but it would need to be
very large (>4096x4096), and I was not sure it would be wise to add such a 
large binary resource to the repo. So I
settled for random noise, since it was the simplest way to generate an image 
which guaranties any misalignment would be
caught by comparing pixels 1 to 1.

-------------

PR: https://git.openjdk.java.net/jfx/pull/112

Reply via email to