Re: RFR: 8089589: [ListView] ScrollBar content moves toward-backward during scrolling. [v4]
On Fri, 16 Apr 2021 10:45:18 GMT, Johan Vos 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. > > Johan Vos has updated the pull request incrementally with one additional > commit since the last revision: > > Process reviewer comments, uncomment commented test, remove stderr modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java line 926: > 924: // the absolute offset is changed accordingly. > 925: adjustAbsoluteOffset(); > 926: } This seems to violate the invariant `setPosition(...)` ≙ `positionProperty().set(...)`. Your comment indicates that this is by design, but how do you prevent people from invoking `positionProperty().set(...)` and getting a different behavior? - PR: https://git.openjdk.java.net/jfx/pull/398
Re: RFR: 8089589: [ListView] ScrollBar content moves toward-backward during scrolling. [v4]
On Fri, 16 Apr 2021 10:45:18 GMT, Johan Vos 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. > > Johan Vos has updated the pull request incrementally with one additional > commit since the last revision: > > Process reviewer comments, uncomment commented test, remove stderr Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/398
Re: RFR: 8089589: [ListView] ScrollBar content moves toward-backward during scrolling. [v4]
On Fri, 16 Apr 2021 10:45:18 GMT, Johan Vos 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. > > Johan Vos has updated the pull request incrementally with one additional > commit since the last revision: > > Process reviewer comments, uncomment commented test, remove stderr Marked as reviewed by aghaisas (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/398
Re: RFR: 8089589: [ListView] ScrollBar content moves toward-backward during scrolling. [v4]
> 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. Johan Vos has updated the pull request incrementally with one additional commit since the last revision: Process reviewer comments, uncomment commented test, remove stderr - Changes: - all: https://git.openjdk.java.net/jfx/pull/398/files - new: https://git.openjdk.java.net/jfx/pull/398/files/91a82c08..29b682c1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=398&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=398&range=02-03 Stats: 50 lines in 1 file changed: 0 ins; 22 del; 28 mod Patch: https://git.openjdk.java.net/jfx/pull/398.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/398/head:pull/398 PR: https://git.openjdk.java.net/jfx/pull/398
