On Fri, 9 Apr 2021 22:29:39 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 2892: > >> 2890: >> 2891: // once at 95% of the total estimated size, we want a correct >> size, not >> 2892: // an estimated size anymore. > > I presume that this is a balance between performance and accuracy? It seems > to work well. Have you tried other values? > > Do you also need to check for `newPosition < 0.05` if you are scrolling > backwards? For smaller lists, it makes sense to eagerly calculate the remaining values. However, since we always add the calculations for a next cell on every adjustment, we will have all values before we are at 95%. In this case, the 95% is just a safeguard in case the list has very large cells in the beginning, and very small cells near the end. The sooner we know this, the better, but not at a price of drastically dropping performance. For longer lists, decreasing the value will lead to a major performance issue. There might be specific combinations in cell count and sizes that would lead to less ideal results. Lowering the value would lead to a lower performance, so I suggest not doing that. In case of these specific combinations that lead to issues, we can create a new issue and have the `95` being more dynamically calculated (based on total cell count and variations between the sizes of the already known items). A check when moving backwards should not be required, as we should have all the information about the cells before the current one already -- so there can be no surprises about sudden jumps. ------------- PR: https://git.openjdk.java.net/jfx/pull/398
