RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed

[Jeanette Winzenburg]

On Mon, 29 Mar 2021 12:35:23 GMT, Marius Hanl <github.com+66004280+Maran23 at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java line 556:
>> 
>>> 554:             }
>>> 555:             getChildren().removeAll(toRemove);
>>> 556:         } else if (resetChildren || cellsEmpty) {
>> 
>> just curious : any idea why fixedCellSize was special-cased here? Not to clean them up sounds (and is) very much wrong, so there must have been a reason?
>
> The '!fixedCellSizeEnabled' check is not needed, as we already check for fixedCellSizeEnabled in the main if condition.
> `if (fixedCellSizeEnabled) {...}`
> 
> So before, it was like: 
> `if (fixedCellSizeEnabled) {...} `
> `else if (!fixedCellSizeEnabled && (resetChildren || cellsEmpty)) {...}` 
> 
> So we only execute the else condition, if fixedCellSizeEnabled is not true -> !fixedCellSizeEnabled. -> The check is not necessary, as fixedCellSizeEnabled must be false, when we come to the else condition.
> The variable fixedCellSizeEnabled is also a primitive boolean, so it can't be null, making this check obsolete.
> 
> Edit: If you mean the special handling for fixed cell size in general, I have no idea. This was added in a commit from Jonathan Giles stating, that he fixed a performance problem with fixed cell size. So maybe instead of resetting all the cells, only the affected are removed/added by this, leading to a performance boost?

yeah, was curious about the stuff in your "edit" paragraph. And thanks for the explanation of the if-block, didn't see the whole .. :)

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

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