On Tue, 7 Feb 2023 19:57:00 GMT, Andy Goryachev <ango...@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 382: > >> 380: labeled.getLineSpacing(), text.getBoundsType()); >> 381: >> 382: double height; > > same comment here - textHeight is computed regardless of whether its needed > (isIgnoreText() == true), > > and use a boolean to avoid multiple calls to isIgnoreText() in the same > method. > > isIgnoreGraphic, on the other hand, is very light weight, so it's up to you > whether to use a local boolean variable in this and the other method or not. In the case of `computePrefHeight` as well calculation of `graphicHeight` is moved to `else` block to avoid one unnecessary calculation. In this method, by checking for ignore text condition before checking for ignore graphic, we can move `cleanText`, `textWidth` and `textHeight` calculation to `else` block instead of only `graphicHeight`. But this again leads to unexpected result because of the checks present in `isIgnoreText()` method. Hence did not re-order `if-else` statements now. Added boolean for both `isIgnoreGraphic` and `isIgnoreText`. ------------- PR: https://git.openjdk.org/jfx/pull/996
