RFR: 8276553: ListView scrollTo() is broken after fix for JDK-8089589 [v2]

Johan Vos jvos at openjdk.java.net
Fri Dec 3 11:19:21 UTC 2021


On Fri, 3 Dec 2021 10:20:43 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Move functionality in the setCellCount() to the invalidated block.
>>   Some hard numbers used in tests (counters for evaluations) were changed because of this.
>>   Instead of relying on hard values, I modified the failing was into relative ones.
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java line 1124:
> 
>> 1122:                 Platform.runLater(() -> {
>> 1123:                     Toolkit.getToolkit().firePulse();
>> 1124:                     assertTrue(rt_35395_counter < 7);
> 
> I see that you have modified assertions to use "lesser than" some expected value. This may hide some incorrect test outcomes.
> Along with "lesser than" assertion, do you think we should add a "greater than" assertion as well? This will have a range bounded expected value.
> This is applicable for all assertion changes in this PR.

The hard values have been changed a number of times, and I believe it is not really a good metric. 
What we want to ensure is that there is no functional regression and no performance penalty. The tests calculate the number of updateItem invocations and require those to be a strict number. With JDK-8089589, we fixed a number of issues that are related to the fact that the total size of the view (in case all cells are rendered with their preferred height) is not known. This is done by using an estimate of the total size. The estimate is 100% correct if we call updateItem for every item, but that would lead to a huge performance penalty in case the list contains a large amount of items with equal height.
Hence, there is a balance between minimizing the updateItem calls but still have a fair estimate of the total dimensions. Rather than requiring that the amount of calls should be a fixed number, I think it makes more sense to ensure that the amount of calls stays within reasonable boundaries, and is not growing exponentially when we add linearly more items, for example.

-------------

PR: https://git.openjdk.java.net/jfx/pull/683


More information about the openjfx-dev mailing list