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

[Karthik P K]

On Thu, 2 Feb 2023 16:54:28 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:
>> 
>>   Use width while calling prefHeight method. Use graphicHeight instead of multiple calls to prefHeight
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/LabeledSkinBase.java line 402:
> 
>> 400:         }
>> 401: 
>> 402:         return  height + padding;
> 
> My only comment is possibly reorganize the logic to avoid unnecessary computation.  
> For example, there is no need to compute text width (line 329) if isIgnoreText() is true; 
> Similarly, no need to compute graphicWidth on line 333 if isIgnoreGraphic() is true.
> 
> (basically, same comment as @hjohn has made earlier)
> 
> What do you think, @karthikpandelu ?

Not sure if I didn't understand your comments properly but I'm guessing you are referring to `textHeight` and `graphicHeight` calculation in line 375 and 380 because the line numbers you mentioned are in `computePrefWidth` method and changes are made to height calculation in `computePrefHeight` only.
Since `textHeight` and `graphicHeight` each are used in 3 conditions from line no 386 to 394 I have kept the calculation in the beginning itself. 

On the other hand `textWidth` declared in line 368 might change in line 371, so I can't use `width` directly instead of defining `textWidth` as `width` need to be used while calling other graphic related methods.

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

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