On Mon, 23 May 2022 20:44:47 GMT, Johan Vos <j...@openjdk.org> wrote:
>> When the size of a ListCell is changed and a scrollTo method is invoked >> without having a layout calculation in between, the old (wrong) size is used >> to calculcate the total estimate. This happens e.g. when the size is changed >> in the `updateItem` method. >> This PR will immediately resize the cell and sets the new value in the cache >> containing the cellsizes. > > Johan Vos has updated the pull request incrementally with one additional > commit since the last revision: > > Try to keep visible cells at their original position when improving the > estimate. > This reduces annoying effects when re-layouting cells without relevant > changes. > > This should not be done when we don't have a valid estimate yet/anymore, > or when the absoluteOffset is set to infinity (which may happen if the > position is set to infinity, which is the case in some calls in Skin > classes). To give some background: the reason for this PR is to fix (real or perceived) regression after https://bugs.openjdk.java.net/browse/JDK-8089589 was fixed. The original problem was this: in case a ListView contains items with different heights, the scrollbar can show weird jumps as it considers all cells to have the same height. The "easy" solution to this would be to pre-calculate the height of all cells, but that would be a potential performance issue. The approach taken in JDK-8089589 is to gradually improve the information about the different sizes of the cells, and it fixes the weird jumps (at the very least, it avoids jumping upwards when scrolling down and vice versa). However, this gradual improvements can cause other visual disturbances. Initially, we improved the parameters whenever a layoutChildren was invoked. However, this leads to undesired effects in case layoutChildren is invoked again after a specific goal is asked (e.g. scroll to a cell with a specific index). A consequence of the gradual improvement is that the algorithm realizes the first cell need to move upwards or downwards a bit, but that will destroy the state where a cell should be positioned exactly at a given position. The latest commit fixes this, by modifying the parameters (absoluteOffset and position) in such a way that they lead to improvements, yet keep the visible cells intact. This broke a number of tests, as there are cases where keeping these cells intact is not what we want. When there is no cached estimate, or when the absoluteOffset has a bogus value, we do not want to maintain the previous situation. I'm moving this PR to draft until I have more confirmation about the (perceived) stability. ------------- PR: https://git.openjdk.java.net/jfx/pull/712
