On Wed, 16 Dec 2020 00:25:15 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Thanks for that.
>>
>> I am wondering; is it ok to potentially address both sub-issues discussed
>> here (Scrollpane snaping vs cache rendering) under the same JBS bug? Or
>> would it be better to address them under separate issues?
>>
>> Speaking of which, the JBS issue title is in fact misleading, as the issue
>> appears to be not specific to Windows; what's the best practice in such
>> cases; rename the issue? Open a new one? Leave it as is?
>
>> I am wondering; is it ok to potentially address both sub-issues discussed
>> here (Scrollpane snaping vs cache rendering) under the same JBS bug? Or
>> would it be better to address them under separate issues?
>
> Since both issues -- the snap-to-pixel bug and the rendering of the cached
> image -- are causing blurriness, it could be OK _in principle_ to address
> both of them as part of the same bug fix.
>
> However, in this instance the snap-to-pixel bug has a simple,
> well-understood, and safe solution, while the cached rendering bug isn't
> nearly as simple; there are a couple ways it could be fixed, each with their
> own implications. Given that, I would prefer to address the snap-to-pixel bug
> here (which can easily make jfx16), and file a follow-up bug for the cached
> rendering.
>
>> the JBS issue title is in fact misleading, as the issue appears to be not
>> specific to Windows; what's the best practice in such cases; rename the
>> issue? Open a new one? Leave it as is?
>
> This is easily fixed by renaming the JBS issue, and then updating the PR
> title to match. I'll update the JBS issue now, after which you can update the
> PR title.
>
> Here are some thoughts about the cached rendering, which should find their
> way into the new issue:
>
> Whatever we do to fix this, the end result should be that rendering a shape
> or control into a cache and then rendering that cached image should match
> rendering that shape or control directly. This should be true the first time
> it is rendered, and should remain true as long as the transform is unchanged
> (or differs only by a translation delta of whole pixel values) from when the
> cache was rendered into. This is clearly broken for rendering text if the
> translation is not on a pixel boundary.
>
> Which leads into a question you asked.
>
>> What is the legitimate result to expect here; should
>> root.setSnapToPixel(true); override setLayoutX(0.5); and align everything
>> for crisp rendering? Or am I misunderstanding the scope of setSnapToPixel
>> and it has no effect when layout is set explicitly?
>
> A Pane will not force layoutX and layoutY to be on an integer boundary, since
> it is documented to not alter the position of any of its children. The
> subclasses of Pane do layout their children, so snap-to-pixel will take
> effect. So the fact that the controls in your most recent example are being
> rendered on a non-pixel boundary is not a bug. The fact that the text is so
> blurry when rendered from a cached image is a bug (as mentioned above).
For completeness, here is the patch for the snap-to-pixel issue:
diff --git
a/modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java
b/modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java
index 565d52b516..00c0f6da61 100644
--- a/modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java
+++ b/modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java
@@ -989,17 +989,11 @@ public class Region extends Parent {
/** Called to update the cached snapped insets */
private void updateSnappedInsets() {
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 = snapSizeY(insets.getTop(), snap);
+ snappedRightInset = snapSizeX(insets.getRight(), snap);
+ snappedBottomInset = snapSizeY(insets.getBottom(), snap);
+ snappedLeftInset = snapSizeX(insets.getLeft(), snap);
}
/**
We will need a test case for this. You can see
[UIRenderSceneTest.java](https://github.com/openjdk/jfx/blob/master/tests/system/src/test/java/test/javafx/scene/UIRenderSceneTest.java)
for an example of a test that forces Hi-DPI scaling.
-------------
PR: https://git.openjdk.java.net/jfx/pull/308
