RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v14]

[Alexey Ivanov]

On Mon, 9 May 2022 22:04:38 GMT, Alisen Chung <achung at openjdk.org> wrote:

>> Changed the drawing area to be increased by 0.5 on the left side to prevent clipping
>
> Alisen Chung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   made suggested changes

Overall, it looks very good. The test catches bad rendering of both horizontal and vertical border. It passes successfully with the fix.

Except for the minor notes I left, I have one more suggestion: run the test for all the scales / images that we have rather than failing the test at a encountered first error. It'll give the an overview of all border rendering.

On the other hand, we don't expect the test to fail in the future…

One more thing: the test now uses `EtchedBorder` only, shall it be moved to `test/jdk/java/awt/EtchedBorder`? And adding the word _Scaled_ to the title — `ScaledEtchedBorderTest` — would clarify the purpose of the test.

What do you think?

src/java.desktop/share/classes/javax/swing/border/EtchedBorder.java line 177:

> 175:                                                     : getShadowColor(c), w, h, stkWidth);
> 176:         paintBorderRect(g, (etchType == LOWERED) ? getShadowColor(c)
> 177:                                                     : getHighlightColor(c), w, h, stkWidth);

I suggest aligning the colon `:` with `?` and wrapping following parameters:
Suggestion:

        paintBorderShadow(g, (etchType == LOWERED) ? getHighlightColor(c)
                                                   : getShadowColor(c),
                          w, h, stkWidth);
        paintBorderRect(g, (etchType == LOWERED) ? getShadowColor(c)
                                                 : getHighlightColor(c),
                        w, h, stkWidth);

I think it's clearer this way. Alternatively, leave the ternary operator on the first line and wrap the following parameters to the second line.

Should `paintBorderRect` be `paintBorderHighlight`?  
The first method is `paintBorderShadow`.

test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 95:

> 93:             BufferedImage img = images.get(i);
> 94:             double scaling = scales[i];
> 95: 

Does it make sense to print the current scale being tested?

            System.out.printf("Scaling: %.2f\n", scaling);


It may be especially useful for error message which don't include the current scale.

test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 112:

> 110:         boolean checkHighlight = false;
> 111:         for (int i = 0; i < img.getWidth(); i++) {
> 112:             int color = img.getRGB(i,y);

Suggestion:

        for (int x = 0; x < img.getWidth(); x++) {
            int color = img.getRGB(x, y);

The loop is for `x` coordinate.

test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 124:

> 122:                     thickness++;
> 123:                 } else if (color == highlight.getRGB()) {
> 124:                     verifyThickness(y, thickness, scaling, "Horizontal");

Here, the IDE warns `y` probably shouldn't be passed as `x` parameter.
In fact, `verifyThickness` doesn't use it at the moment. To make the error message more informative, you may pass both `x` and `y` and also include the real vs expected thickness.

test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 157:

> 155:         boolean checkHighlight = false;
> 156:         for (int i = 0; i < img.getHeight(); i++) {
> 157:             int color = img.getRGB(x,i);

Suggestion:

        for (int y = 0; y < img.getHeight(); y++) {
            int color = img.getRGB(x, y);

Here we're changing `y` coordinate.

test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 248:

> 246:     }
> 247: 
> 248:     private static void setLookAndFeel(UIManager.LookAndFeelInfo laf) {

Please remove this unused method.

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

PR: https://git.openjdk.java.net/jdk/pull/7449