Improve image preview performance on Linux#453
Merged
cwisniew merged 3 commits intoRPTools:developfrom Apr 19, 2025
Merged
Conversation
fa3fe46 to
559725e
Compare
It is never meant to be rendered in the main panel, but is only meant to be part of the composition of final results. Removing it from the scene is not only simpler (no need to toggle visibility), it avoids any possibility of flickering when we introduce async preview composition.
The new `ImageUtil.Async.snapshot()` is a convenience wrapper that adapts the callback-based `Node#snapshot()` into a future-returning method. This is then used by the other new async methods introduced in this commit: - `ImageUtil.Async.autoCrop()` for autocropping an image. This is optionally async since there is one use that would otherwise have to be completely rewritten for asynchronous operation. Mostly replaces `ImageUtil.autoCropImage()`, but `ImageUtil.autoCropImage()` still exists as a wrapper around the synchronous version. - `ImageUtil.Async.composeClippedPreview()` for creating a preview clipped to the overlay. Replaces the clipped branch of `ImageUtil.composePreview()`. - `ImageUtil.Async.composeUnclippedPreview()` for creating a preview that is not clipped to the overlay. Replaces the unclipped branch of `ImageUtil.composePreview()`. Make autoCropImage optionally async
559725e to
dc7763a
Compare
cwisniew
approved these changes
Apr 19, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #447
The core of this change is switching from
Node#snapshot(SnapshotParameters, WritableImage)toNode#snapshot(Callback, SnapshotParameters, WritableImage)when rendering the preview image. This is represented by the newImageUtil.Async.snapshot()which wraps that method to in aCompletableFuture<>so that callback aren't required.Operations that depend on snapshots had to be rewritten to account for the asynchony:
ImageUtil.autoCropImage()is mostly replaced withImageUtil.Async.autoCrop().ImageUtil.composePreview()is replaced byImageUtil.Async.composeClippedPreview()andImageUtil.Async.composeUnclippedPreview(), depending on the current clip settings.The only problem with the above changes is the
maskImageView. It needs to be made visible in order to snapshot it, only to be hidden again right away. But with asynchronous snapshots, we can't guarantee that it will be hidden again in time for the next frame, which results in it flickering. To fix that, I've removedmaskImageViewfrom the scene since it is never meant to be rendered there, though it remains part ofTokenTool_Controller. Now it is only incorporated it into aGroupwhen rendering the preview. This lets us leave it marked as visible since it will never actually be on screen.Finally, I've removed some of the unused methods in
ImageUtil:scaleImage()getScaleXRatio()getScaleYRatio()imageToBytes()This change is