RFR: 8230833: LabeledSkinBase computes wrong height with ContentDisplay.GRAPHIC_ONLY [v4]
Karthik P K
kpk at openjdk.org
Wed Feb 8 12:59:01 UTC 2023
On Tue, 7 Feb 2023 19:57:00 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:
>>
>> 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
More information about the openjfx-dev
mailing list