RFR: JDK-8015739: Background of JInternalFrame is located out of JInternalFrame [v5]

Alexey Ivanov aivanov at openjdk.org
Tue Oct 4 19:52:27 UTC 2022


On Tue, 4 Oct 2022 17:48:55 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> JInternalFrame background color seems to overflow into the border region. This issue is more prominently seen on Windows - Metal LAF (with fractional scaling, as shown below). The primary reason is border scaling issue as observed in - [JDK-8279614](https://bugs.openjdk.org/browse/JDK-8279614) 
>> 
>> The fix involves a similar approach as described here https://github.com/openjdk/jdk/pull/7449#issuecomment-1068218648. The test checks the midpoint and corners of borders to check if the internal frame's background color is located out of JInternalFrame.
>> 
>> ![image](https://user-images.githubusercontent.com/95945681/190233555-a7e00f2c-9003-4c11-84fb-207957838c2f.png)
>
> Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review changes, saving a scaled version of image

It looks good to me, except for the comments I left on the coding style.

Yet there's one thing which I noticed in the rendered border. The top left corner has one pixel which is painted over the title bar. It's present in the original painting code. The top right corner has such a pixel too.

The updated code paints the top left corner with that pixel. Yet in the top right corner, it moved to left.
![top right corner of the JInternalFrame](https://user-images.githubusercontent.com/70774172/193912190-8faf432e-0bc2-4906-8811-e3780da86b9b.png)

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java line 29:

> 27: import javax.swing.JLabel;
> 28: import javax.swing.SwingUtilities;
> 29: import javax.swing.UIManager;

I'd rather stick to the same order of imports in sources and tests. However, swing components used are more important.

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java line 185:

> 183:                 errorLog.append("At uiScale: "+ uiScale +
> 184:                 ", Red background color detected at "
> 185:                 + borderDirection + " border\n");

This doesn't look right either. The old code had only the body of `if` incorrectly indented.


            if (Color.RED.equals(robot.getPixelColor(
                    isVertical ? i : (iFrameLoc.x + MIDPOINT),
                    isHorizontal ? i : (iFrameLoc.y + MIDPOINT)))) {
                saveScreenCapture(borderDirection + "_" + uiScale + ".png");
                errorLog.append("uiScale: " + uiScale
                        + " Red background color detected at "
                        + borderDirection + " border\n");


Continuation lines are indented twice as much or to align with a level they're wrapped at. The example for the second case:

                errorLog.append("uiScale: " + uiScale
                                + " Red background color detected at "
                                + borderDirection + " border\n");

Here, the operators on the wrapped lines align to the following column of the opening parenthesis for parameters of `append`.

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java line 217:

> 215:         if (Color.RED.equals(robot.getPixelColor(x, y))) {
> 216:             saveScreenCapture(cornerLocation + "_" + uiScale + ".png", uiScale);
> 217:             errorLog.append("At uiScale: "+ uiScale + ", Red background color" +

Suggestion:

            errorLog.append("At uiScale: " + uiScale + ", Red background color" +

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java line 229:

> 227:         jFrame.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
> 228: 
> 229:         JLabel scale = new JLabel("UI Scale: "+ uiScale);

Suggestion:

        JLabel scale = new JLabel("UI Scale: " + uiScale);

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java line 252:

> 250:         g2d.scale(scale, scale);
> 251:         g2d.drawImage(image, 0, 0, null);
> 252:         g2d.dispose();

This does not make sense, it up-scales a down-scaled image. Either revert to the previous version or use `createMultiResolutionScreenCapture` and save all resolution variants from the `MultiResolutionImage`.

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

Changes requested by aivanov (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10274



More information about the client-libs-dev mailing list