RFR: 8230833: LabeledSkinBase computes wrong height with ContentDisplay.GRAPHIC_ONLY [v4]

[Karthik P K]

On Tue, 7 Feb 2023 19:47:50 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Karthik P K has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Addressing review comments
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/LabeledSkinBase.java line 328:
> 
>> 326:         }
>> 327: 
>> 328:         double textWidth = emptyText ? 0.0 : Utils.computeTextWidth(font, cleanText, 0);
> 
> textWidth is being computed even if isIgnoreText() is true.  I wonder if we could re-structure the if-else statements below to avoid unnecessary computations?
> 
> There is another suggestion - isIgnoreText() is not a simple getter (does a bit of computation).  We probably should use a boolean early in this method to avoid calling it repeatedly.

Previously both `textWidth` and `graphicWidth` were calculated regardless of the condition. In the updated code calculation of `graphicWidth` is moved to `else` block so that one unnecessary calculation is avoided. Since `textWidth` is used in 2 more conditions in `else` block, calculated it before the start of `if` block.

Since `isIgnoreText()` does more than just checking for `ContentDisplay` value, further reordering `if-else` statements generates different result than expected.
So this further leads to question previously raised by @hjohn, if `isIgnoreText()` and `isIgnoreGraphic()` should only check for ContetDisplay type or check for conditions like null/empty graphic or text as well. If this needs to be separated, again it gives raise to more conditions in the `computePrefWidth()` method.

Added boolean in the beginning as suggested.

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

PR: https://git.openjdk.org/jfx/pull/996