On Fri, 27 Jun 2025 18:11:46 GMT, Rajat Mahajan <rmaha...@openjdk.org> wrote:
> Modified the code to account for border thickness correctly and updated the
> related tests to make sure we are testing the new change.
>
> Testing done.
Looks good except for a few nits.
src/java.desktop/share/classes/javax/swing/border/LineBorder.java line 166:
> 164: Shape inner;
> 165:
> 166: int offs = clipRound(this.thickness*scaleFactor);
Suggestion:
int offs = clipRound(this.thickness * scaleFactor);
src/java.desktop/share/classes/javax/swing/border/LineBorder.java line 167:
> 165:
> 166: int offs = clipRound(this.thickness*scaleFactor);
> 167:
I'd leave out the added blank line, both `offs` and `size` are still tightly
coupled, only the formula for `offs` is amended.
test/jdk/javax/swing/border/LineBorder/ScaledLineBorderTest.java line 117:
> 115:
> 116: for (Point p : test.panelLocations) {
> 117: int y = (int) (p.y * test.scale) + (int)
> (SIZE.height * test.scale / 2);
This expression can be simplified:
Suggestion:
int y = (int) ((p.y + (SIZE.height / 2)) * test.scale);
This way may look clearer: find the vertical centre of the panel, taking into
account the scale applied.
test/jdk/javax/swing/border/LineBorder/ScaledLineBorderTest.java line 302:
> 300: private static String getImageFileName(final double scaling,
> 301: int thickness) {
> 302: return String.format("test%02d@%.2f.png", thickness, scaling);
Suggestion:
private static String getImageFileName(final double scaling,
final int thickness) {
return String.format("test%02d@%.2f.png", thickness, scaling);
Declare both parameters as `final` to be consistent.
-------------
Changes requested by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26025#pullrequestreview-2967643714
PR Review Comment: https://git.openjdk.org/jdk/pull/26025#discussion_r2172687248
PR Review Comment: https://git.openjdk.org/jdk/pull/26025#discussion_r2172705614
PR Review Comment: https://git.openjdk.org/jdk/pull/26025#discussion_r2172695201
PR Review Comment: https://git.openjdk.org/jdk/pull/26025#discussion_r2172700022
