RFR: 8238954: Improve performance of tiled snapshot rendering [v5]
github.com+7450507+fthevenet at openjdk.java.net
Mon Jun 29 16:27:30 UTC 2020
On Mon, 29 Jun 2020 14:35:00 GMT, Frederic Thevenet <github.com+7450507+fthevenet at openjdk.org> wrote:
>> 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 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 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.
I have changed the test setup to fill up the scene with a gradient instead of random noise; it should be as able to
catch misalignment, while not looking like something's gone horribly wrong.
More information about the openjfx-dev