On Fri, 9 Apr 2021 22:32:28 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> This PR introduces a refactory for VirtualFlow, fixing a number of issues >> reported about inconsistent scrolling speed (see >> https://bugs.openjdk.java.net/browse/JDK-8089589) >> The problem mentioned in the JBS issue (and in related issues) is that the >> VirtualFlow implementation depends on cell index and cell count, instead of >> on pixel count. The latter is unknown when the VirtualFlow is created, and >> pre-calculating the size of a large set of items would be very expensive. >> Therefore, this PR uses a combination of a gradual calculation of the total >> size in pixels (estimatedSize) and a smoothing part that prevents the >> scrollback to scroll in the reverse direction as the requested change. >> This PR currently breaks a number of tests that hard-coded depend on a >> number of evaluations. This is inherit to the approach of this PR: if we >> want to estimate the total size, we need to do some additional calculations. >> In this PR, I try to balance between consistent behavior and performance. > > modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java > line 2902: > >> 2900: >> 2901: // if we are at or beyond the edge, correct the absolteOffset >> 2902: if (newPosition >= 1.d) { > > Do you need to also check for `newPosition < 0` if you are scrolling > backwards? That can not happen. The only reason we can have `newPosition > 1.d` is when the correction applied to scrolling in the wrong direction would bring the position beyond 1: `newPosition = getPosition()*1.01` The inverse check for this might reduce the position (by `.99`) but it will never render it negative. Note that there is a documentation placeholder in case we want to allow for gravity-alike scrolling ``` // When we're at the top already, we can't move back further, unless we // want to allow for gravity-alike effects. In that case, we might want to use negative values for the position, but that seems a follow-up issue to me (if we want to do that at all, that is) ------------- PR: https://git.openjdk.java.net/jfx/pull/398
