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

Andy Goryachev angorya at openjdk.org
Thu Feb 9 16:55:03 UTC 2023


On Mon, 6 Feb 2023 14:23:38 GMT, Karthik P K <kpk at openjdk.org> wrote:

>> Both `computePrefHeight()` and `computePrefWidth` can be restructured to avoid unnecessary computation (especially since these are very popular objects).
>> 
>> My point is that, for example, textWidth on line 375 (used to compute textHeight:375) is not used if isIgnoreText() == true.  
>> 
>> Similarly, graphicHeight:380 is not used if isIgnoreGraphic() == true, line 386.
>> 
>> Same optimization can be applied to computePrefWidth():315
>> 
>> Just a suggestion, really.
>
> Understood.
> I have optimized the code. Please take a look.
> Did following optimizations in the code:
> 
> - Optimized `if else` conditions by moving the `graphicHeight` and `graphicWidth` value calculation inside the else statement in `computePrefHeight` and `computePrefWidth` methods respectively.
> - Removed unused `graphic` variable.
> - In `computePrefHeight` method, since a `if` condition was already present which was same as the one used for `padding` calculation, moved `padding` calculation code to the same location.
> - In `computePrefWidth` method, replaced multiple return statements with single statement for better readability and did minor optimizations.

looks good, thank you.

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

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


More information about the openjfx-dev mailing list