On Thu, 8 Feb 2024 18:55:54 GMT, Marius Hanl <mh...@openjdk.org> wrote:
> `TreeTableRow` does not check the item with `isItemChanged(..)`, unlike all > other implementations of the cell. > > This also means that the `TreeTableRow` always updates the item, although it > should not, resulting in a performance loss as a `TreeTableRow` will always > call `updateItem(..)`. > > It looks like that this was forgotten in > [JDK-8092593](https://bugs.openjdk.org/browse/JDK-8092593). > > Checking the whole history, it looks like the following was happening: > 1. There was a check if the item is the same in all cell implementations > (with `.equals(..)`) > 2. Check was removed as it caused bugs > 3. Check was readded, but instead we first check the index (same index) and > then if we also have the same item (this time with `.isItemChanged(..)`, to > allow developers to subclass it) > 4. Forgotten for `TreeTableRow` inbetween this chaos. > > Added many tests for the case where an item should be changed and where it > should not. > Improved existing tests as well. Using `stageLoader`, as before the tests > only created a stage when doing the assert. > > NOTE: The update logic is now the same as for all other 5 cell > implementations. Especially `TreeCell` (since it is for a tree structure as > well). This pull request has now been integrated. Changeset: 11305843 Author: Marius Hanl <mh...@openjdk.org> URL: https://git.openjdk.org/jfx/commit/11305843300cdbfed9bfa2120da1c7ecd361df39 Stats: 539 lines in 11 files changed: 511 ins; 2 del; 26 mod 8325402: TreeTableRow updateItem() does not check item with isItemChanged(..) unlike all other cell implementations Reviewed-by: angorya, kpk ------------- PR: https://git.openjdk.org/jfx/pull/1360
