On Fri, 3 Feb 2023 05:56:32 GMT, Karthik P K <k...@openjdk.org> wrote:
>> 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. 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. ------------- PR: https://git.openjdk.org/jfx/pull/996
