Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8211294: ScrollPane content is blurry with 125% scaling #308

Closed
wants to merge 7 commits into from
@@ -943,20 +943,23 @@ void fireValueChanged() {
private double snappedBottomInset = 0;
private double snappedLeftInset = 0;

/**
* Cached snapScale values, used to determine if snapped cached insets values
* should be invalidated because screen scale has changed.
*/
private double lastUsedSnapScaleY = 0;
private double lastUsedSnapScaleX = 0;

/** Called to update the cached snapped insets */
private void updateSnappedInsets() {
lastUsedSnapScaleX = getSnapScaleX();
lastUsedSnapScaleY = getSnapScaleY();
final Insets insets = getInsets();
if (_snapToPixel) {
snappedTopInset = Math.ceil(insets.getTop());
snappedRightInset = Math.ceil(insets.getRight());
snappedBottomInset = Math.ceil(insets.getBottom());
snappedLeftInset = Math.ceil(insets.getLeft());
} else {
snappedTopInset = insets.getTop();
snappedRightInset = insets.getRight();
snappedBottomInset = insets.getBottom();
snappedLeftInset = insets.getLeft();
}
final boolean snap = isSnapToPixel();
snappedTopInset = snapSpaceY(insets.getTop(), snap);
snappedRightInset = snapSpaceX(insets.getRight(), snap);
snappedBottomInset = snapSpaceY(insets.getBottom(), snap);
snappedLeftInset = snapSpaceX(insets.getLeft(), snap);
}

/**
@@ -1818,6 +1821,11 @@ public double snapPositionY(double value) {
* @return Rounded up insets top
*/
public final double snappedTopInset() {
// invalidate the cached values for snapped inset dimensions
// if the screen scale changed since they were last computed.
This conversation was marked as resolved by fthevenet

This comment has been minimized.

@kevinrushforth

kevinrushforth Dec 22, 2020
Member

Minor: Maybe copy this comment to the other 4 methods for consistency?

if (lastUsedSnapScaleY != getSnapScaleY()) {
updateSnappedInsets();
}
return snappedTopInset;
}

@@ -1829,6 +1837,11 @@ public final double snappedTopInset() {
* @return Rounded up insets bottom
*/
public final double snappedBottomInset() {
// invalidate the cached values for snapped inset dimensions
// if the screen scale changed since they were last computed.
if (lastUsedSnapScaleY != getSnapScaleY()) {
updateSnappedInsets();
}
return snappedBottomInset;
}

@@ -1840,6 +1853,11 @@ public final double snappedBottomInset() {
* @return Rounded up insets left
*/
public final double snappedLeftInset() {
// invalidate the cached values for snapped inset dimensions
// if the screen scale changed since they were last computed.
if (lastUsedSnapScaleX != getSnapScaleX()) {
updateSnappedInsets();
}
return snappedLeftInset;
}

@@ -1851,6 +1869,11 @@ public final double snappedLeftInset() {
* @return Rounded up insets right
*/
public final double snappedRightInset() {
// invalidate the cached values for snapped inset dimensions
// if the screen scale changed since they were last computed.
if (lastUsedSnapScaleX != getSnapScaleX()) {
updateSnappedInsets();
}
return snappedRightInset;
}

@@ -612,10 +612,6 @@ public boolean isSettable(TextFlow node) {
}

/* The methods in this section are copied from Region due to package visibility restriction */
private static double snapSpace(double value, boolean snapToPixel) {
return snapToPixel ? Math.round(value) : value;
}

static double boundedSize(double min, double pref, double max) {
double a = pref >= min ? pref : min;
double b = min >= max ? min : max;
@@ -627,11 +623,10 @@ static double boundedSize(double min, double pref, double max) {
}

double computeChildPrefAreaWidth(Node child, Insets margin, double height) {
final boolean snap = isSnapToPixel();
double top = margin != null? snapSpace(margin.getTop(), snap) : 0;
double bottom = margin != null? snapSpace(margin.getBottom(), snap) : 0;
double left = margin != null? snapSpace(margin.getLeft(), snap) : 0;
double right = margin != null? snapSpace(margin.getRight(), snap) : 0;
double top = margin != null? snapSpaceY(margin.getTop()) : 0;
double bottom = margin != null? snapSpaceY(margin.getBottom()) : 0;
double left = margin != null? snapSpaceX(margin.getLeft()) : 0;
double right = margin != null? snapSpaceX(margin.getRight()) : 0;
double alt = -1;
if (child.getContentBias() == Orientation.VERTICAL) { // width depends on height
alt = snapSizeY(boundedSize(
@@ -646,11 +641,10 @@ static double boundedSize(double min, double pref, double max) {
}

double computeChildPrefAreaHeight(Node child, Insets margin, double width) {
final boolean snap = isSnapToPixel();
double top = margin != null? snapSpace(margin.getTop(), snap) : 0;
double bottom = margin != null? snapSpace(margin.getBottom(), snap) : 0;
double left = margin != null? snapSpace(margin.getLeft(), snap) : 0;
double right = margin != null? snapSpace(margin.getRight(), snap) : 0;
double top = margin != null? snapSpaceY(margin.getTop()) : 0;
double bottom = margin != null? snapSpaceY(margin.getBottom()) : 0;
double left = margin != null? snapSpaceX(margin.getLeft()) : 0;
double right = margin != null? snapSpaceX(margin.getRight()) : 0;
double alt = -1;
if (child.getContentBias() == Orientation.HORIZONTAL) { // height depends on width
alt = snapSizeX(boundedSize(
@@ -0,0 +1,107 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package test.javafx.scene;

import com.sun.javafx.PlatformUtil;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.scene.Node;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.control.ScrollPane;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
import javafx.stage.WindowEvent;
import junit.framework.Assert;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;

public class UIRenderSnapToPixelTest {
private static final double scale = 1.25;
private static CountDownLatch startupLatch;
private static volatile Stage stage;
private static final double epsilon = 0.00001;

@BeforeClass
public static void setupOnce() throws Exception {
System.setProperty("glass.win.uiScale", String.valueOf(scale));
System.setProperty("glass.gtk.uiScale", String.valueOf(scale));
startupLatch = new CountDownLatch(1);
new Thread(() -> Application.launch(TestApp.class, (String[]) null)).start();
assertTrue("Timeout waiting for FX runtime to start", startupLatch.await(15, TimeUnit.SECONDS));
}

@AfterClass
public static void teardown() {
Platform.runLater(stage::hide);
Platform.exit();
}

@Test
public void testScrollPaneSnapChildrenToPixels() {
assumeTrue(PlatformUtil.isLinux() || PlatformUtil.isWindows());

Assert.assertEquals("Wrong render scale", scale, stage.getRenderScaleY(), 0.0001);

for (Node node : stage.getScene().getRoot().getChildrenUnmodifiable()) {
if (node instanceof ScrollPane) {
var sp = (ScrollPane) node;
Assert.assertEquals("Top inset not snapped to pixel", 0, ((sp.snappedTopInset() * scale) + epsilon) % 1, 0.0001);
Assert.assertEquals("Bottom inset not snapped to pixel", 0, ((sp.snappedBottomInset() * scale) + epsilon) % 1, 0.0001);
Assert.assertEquals("Left inset not snapped to pixel", 0, ((sp.snappedLeftInset() * scale) + epsilon) % 1, 0.0001);
Assert.assertEquals("Right inset not snapped to pixel", 0, ((sp.snappedRightInset() * scale) + epsilon) % 1, 0.0001);
}
}
}

public static class TestApp extends Application {
private static void run() {
startupLatch.countDown();
}

@Override
public void start(Stage primaryStage) throws Exception {
final Label label = new Label("This text may appear blurry at some screen scale without the fix for JDK-8211294");
final ScrollPane scrollpane = new ScrollPane(label);
scrollpane.setSnapToPixel(true);
final VBox root = new VBox();
root.getChildren().add(new Label("This text should be sharp at all screen scale"));
root.getChildren().add(scrollpane);
final Scene scene = new Scene(root);
primaryStage.setScene(scene);
stage = primaryStage;
stage.addEventHandler(WindowEvent.WINDOW_SHOWN, e -> Platform.runLater(TestApp::run));
stage.show();
}
}

}
ProTip! Use n and p to navigate between commits in a pull request.