On Mon, 14 Dec 2020 12:06:37 GMT, Frederic Thevenet <ftheve...@openjdk.org>
wrote:
>> One more comment: given the quality problems that necessarily arise when the
>> translation of a cached image is not on an integer boundary, part of the
>> solution might be to snap the cached image to a pixel boundary as is done in
>> this PR, but we would need to ensure that this doesn't impact smooth
>> scrolling of a TextArea.
>
>>
>>
>> One more comment: given the quality problems that necessarily arise when the
>> translation of a cached image is not on an integer boundary, part of the
>> solution might be to snap the cached image to a pixel boundary as is done in
>> this PR, but we would need to ensure that this doesn't impact smooth
>> scrolling of a TextArea.
>
> I've done some quick tests with the minimal sample below, and could not
> notice any performance impact with this patch compared to 15.0.1.
>
> java
> public class TextScroll extends Application {
>
> @Override
> public void start(Stage stage) throws Exception {
> var textArea = new TextArea();
> var txt = new
> File(getClass().getResource("/Scrollpane.java").getFile());
> textArea.setText(Files.readString(txt.toPath(),
> StandardCharsets.UTF_8));
> var root = new ScrollPane(textArea);
> root.setFitToHeight(true);
> root.setFitToWidth(true);
> Scene scene;
> scene = new Scene(root, 800, 600);
> stage.setTitle("Scroll test");
> stage.setScene(scene);
> stage.show();
> }
>
> public static void main(String[] args) {
> launch(args);
> }
> }
Actually, when I mentioned smooth scrolling, it wasn't performance I was
thinking of, it was the ability to scroll by fractional pixel amounts, but with
the default snap-to-pixel enabled, that won't happen anyway.
I'll look into the other questions you raised tomorrow, specifically what we
might want to do about rendering the cache, and also your question about
snapToPixel.
As for the specific problem with ScrollPane, I found the cause of that one.
There is a (long-standing from the look of it) bug in
[`Region::updateSnappedInsets`](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java#L990).
It attempts to account for snap-to-pixel, but does so incorrectly, using
`Math.ceil` without taking screen scaling into account (that is, without
calling the `snapXxxx` methods). The correct code should be something like:
final boolean snap = isSnapToPixel();
snappedTopInset = snapSizeY(insets.getTop(), snap);
snappedRightInset = snapSizeX(insets.getRight(), snap);
snappedBottomInset = snapSizeY(insets.getBottom(), snap);
snappedLeftInset = snapSizeX(insets.getLeft(), snap);
This fixes the ScrollPane problem without needing to modify the cached
rendering code.
-------------
PR: https://git.openjdk.java.net/jfx/pull/308
