RFR: 8337268: Redundant Math.ceil in StyleSheet.ListPainter#drawShape

[Prasanta Sadhukhan]

On Sun, 28 Jul 2024 22:33:05 GMT, Phil Race <prr at openjdk.org> wrote:

>> src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java line 2394:
>> 
>>> 2392:             int gap = isLeftToRight ? - (bulletgap + size/3) : (aw + bulletgap);
>>> 2393:             int x = ax + gap;
>>> 2394:             int y = Math.max(ay, ay + ah/2);
>> 
>> Rather than redundant, wasn't this code just incorrect? For example int 3/2 is 1 and ceiling that after is still 1, but I assume that the original purpose of the code was to make ceil(3/2)=2? Should this be fixed to divide as a float then ceil it afterwards?
>
> 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...

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20358#discussion_r1694661426