On Thu, 26 Mar 2026 11:46:56 GMT, Marius Hanl <[email protected]> wrote:
> We hash the row and the column, so it should be fine, shouldn't it? Actually the performance impact is probably very minimal. I checked: `HashSet` falls back to `equals` when there is a collision. So its always correct, just a small tax. More importantly, I stared at that weird code for a little longer. The `contains` check is not sufficient to determine if those items are removed. We need to check _where_ the items are not the same, and fire an add/remove notification for that range. This addresses your request in the issue: only fire the event if something changed. This also has the side effect of fixing https://bugs.openjdk.org/browse/JDK-8120331 -- now our selected indices are actually being updated! Please excuse the slop. This passes all the `controls:test`, and our selected items preserved. int size = newState.size(); int start = -1; for (int i = 0; i < size; i++) { boolean changed = !Objects.equals(prevState.get(i), newState.get(i)); if (changed && start == -1) { start = i; } else if (!changed && start != -1) { List<TablePosition<S,?>> removed = (List<TablePosition<S, ?>>) (Object) prevState.subList(start, i); ListChangeListener.Change<TablePosition<S, ?>> c = new NonIterableChange.GenericAddRemoveChange<>(start, i, removed, newState); sm.fireCustomSelectedCellsListChangeEvent(c); start = -1; } } if (start != -1) { List<TablePosition<S,?>> removed = (List<TablePosition<S, ?>>) (Object) prevState.subList(start, size); ListChangeListener.Change<TablePosition<S, ?>> c = new NonIterableChange.GenericAddRemoveChange<>(start, size, removed, newState); sm.fireCustomSelectedCellsListChangeEvent(c); } ``` ------------- PR Comment: https://git.openjdk.org/jfx/pull/2100#issuecomment-4134834085
