On Sun, 22 Mar 2026 12:16:48 GMT, chuckyschluz <[email protected]> wrote:

>> Fixes [8202066](https://bugs.openjdk.org/browse/JDK-8202066)
>> 
>> `selectRows(int, int)` calls `clearSelection(int)` in a loop which is slow. 
>> There are two cases to consider: `minRow == maxRow` and `minRow < maxRow`. 
>> 
>> In both cases, all indices such that `selectedIndex < minRow || 
>> selectedIndex > maxRow` are cleared. `clearSelection(int)` clears the 
>> associated bit and clears the focus and selected index if operation causes 
>> the selected indices to be empty. 
>> 
>> **minRow == maxRow**
>> 
>> `select(int)` updates the focus and selected index if the input argument is 
>> not already selected. Otherwise, it is a no-up. Therefore, clearing all the 
>> selected indices with `clearSelection(int)` and calling `select(minRow)` is 
>> equivalent to `clearAndSelect(minRow)`. 
>> 
>> **minRow < maxRow**
>> 
>> `selectRange(int, int)` resolves to `selectIndices(int, int...)`, which 
>> updates the focus and selected index to the last valid index, regardless if 
>> that index was already selected. Therefore, the focus and selected index 
>> prior to `selectRange` is irrelevant. 
>> 
>> 
>> Results are encouraging:
>> 
>> [StringPlayground3.java](https://github.com/user-attachments/files/26034143/StringPlayground3.java)
>> 
>> Mainline:
>> 
>> Starting Benchmark with 5500 elements!
>> Robot finished in: 85ms
>> UI Unfroze after: 33108ms
>> Total Operation: 33193ms
>> 
>> With change:
>> 
>> Starting Benchmark with 5500 elements!
>> Robot finished in: 87ms
>> UI Unfroze after: 1ms
>> Total Operation: 88ms
>
> chuckyschluz has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add unit test

modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewMouseInputTest.java
 line 417:

> 415: 
> 416:     // JDK-8202066: shift-click to contract a range should deselect rows 
> outside the new range
> 417:     @Test public void 
> test_jdk8202066_shiftClickContractsRangeDeselects() {

I would recommend removing the JDK number from the test. It does not provide 
any value, especially since you already wrote a comment above.
As simple as: `testShiftClickRangeDeselect` is sufficient.

I also think you can remove the comment above this test. The JDK number does 
not really help as it is a not a bug but a performance improvement. The test is 
clear in itself IMO (which is good). Same below

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

PR Review Comment: https://git.openjdk.org/jfx/pull/2109#discussion_r2971637814

Reply via email to