On Mon, 21 Dec 2020 18:49:11 GMT, Frederic Thevenet <[email protected]>
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
