On Mon, 29 Mar 2021 12:35:23 GMT, Marius Hanl
<github.com+66004280+mara...@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
