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

Karthik P K kpk at openjdk.org
Mon Feb 6 14:26:15 UTC 2023


On Fri, 3 Feb 2023 18:04:09 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> 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.
>
> 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.

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

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


More information about the openjfx-dev mailing list