RFR: 8337268: Redundant Math.ceil in StyleSheet.ListPainter#drawShape
Phil Race
prr at openjdk.org
Mon Jul 29 18:08:33 UTC 2024
On Mon, 29 Jul 2024 06:40:25 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:
>> I was thinking exactly the same when I saw this.
>> Perhaps the coder meant "ah/2.0" which would promote the result to a double on which Math.ceil WOULD do something useful.
>
> This code was added for JDK-8202013 to make bullet size relative to text font size and "`fix takes into account the bullet is shown in middle to text whereas previously, it is at the bottom`" and since all argument and calculations in this method are in integer arithmetic, I guess it will be right to keep this one also as "int" rather to convert to double and back to int
> so it seems, ah/2 may be more appropriate compared to (int)Math.ceil(ah/2.0)
>
> BTW,
> I see for JDK-8202013 testcase, ah is 19 in windows
> so (int)Math.ceil(19/2.0f) is 10
> and (int)Math.ceil(19/2) is 9
> and so does 19/2 ie 9
> so there's a 1 pixel difference of y-coordinate where the oval will be placed as
> y = Math.max(ay, ay+9) or y = Math.max(ay, ay+10)
>
> I guess this probably may not matter much user-experience wise as this bullet placement is relative to font size (size/3 as per the code) but I am open to change to to ah/2.0...
So it was your fix ? https://mail.openjdk.org/pipermail/swing-dev/2018-September/008848.html
In that case, I'm going to suppose you are in a position that the Math.ceil was wrong and it can deleted.
This will also keep the result as it is which seems to have been OK for 6 years now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20358#discussion_r1695657025
More information about the client-libs-dev
mailing list