RFR: 8211294: ScrollPane content is blurry with 125% scaling [v5]

[Kevin Rushforth]

On Mon, 21 Dec 2020 18:49:11 GMT, Frederic Thevenet <fthevenet at openjdk.org> wrote:

>> This PR aims to fix the blurriness to sometimes affects some controls (such as TextArea) in a scene when rendered with a scaling factor that is not an integer (typically when viewed on a HiDPI screen with a 125%, 150% or 175% output scaling).
>> 
>> Please note that regardless of what the JBS issue (and therefore the title of this PR) states, this does not appear to be a Windows only issue as I have observed the same issue on Linux (Ubuntu 20.04).
>> 
>> The following conditions are necessary for the blurriness to appear, but do not guarantee that it will:
>> 
>> - The node, or one of its parents, must have set `Node::cacheProperty` to `true`
>> 
>> - The scaling factor (as detected automatically or explicitly set via glass.win/gtk.uiScale) must be a non integer number (e.g. 1.25, 1.5, 175)
>> 
>> - The effective layout X or Y coordinates for the cached node must be != 0;
>> 
>> Under these conditions, the translate coordinates for the transform used when the cached image for a node is rendered to the screen may be non integer numbers, which is the cause for the blurriness.
>> 
>> Based on these observations, this PR fixes the issue by simply rounding the translate coordinates (using `Math.round`) before the transform is applied in `renderCacheToScreen()` and as far as I can tell, it fixes the blurriness in all the previously affected applications (both trivial test cases or with complex scenegraphs) and does not appear to introduce other noticeable visual artifacts.
>> 
>> Still, there might be a better place somewhere else higher up in the call chain where this should be addressed as it could maybe be the root cause for other rendering glitches, though I'm not yet familiar enough with the code to see if it is really the case.
>
> Frederic Thevenet has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Added a test

The test and the fix look good. I left a couple inline comments.

modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 947:

> 945: 
> 946:     /**
> 947:      * Cached snapScale values, used for to determine if snapped cached insets values

Minor typo: `for to` --> `to`

modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 1825:

> 1823:     public final double snappedTopInset() {
> 1824:         // invalidate the cached values for snapped inset dimensions
> 1825:         // if the screen scale changed since they were last computed.

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

tests/system/src/test/java/test/javafx/scene/UIRenderSnapToPixelTest.java line 77:

> 75:             if (node instanceof ScrollPane) {
> 76:                 var sp = (ScrollPane) node;
> 77:                 Assert.assertEquals("Top inset not snapped to pixel", 0, (sp.snappedTopInset() * scale) % 1, 0.0001);

This could fail if snapped inset * scale is just less than an integer pixel. For example, if `sp.snappedTopInset() * scale` is 1.999999, then `(sp.snappedTopInset() * scale) % 1` would be 0.999999 which would fail. I suggest adding an epsilon value (perhaps 0.00001 so it won't affect the delta comparison) before doing the modulo.

-------------

PR: https://git.openjdk.java.net/jfx/pull/308