RFR: 8338103: Stabilize and open source a Swing OGL ButtonResizeTest [v2]

Damon Nguyen dnguyen at openjdk.org
Wed Aug 21 00:20:06 UTC 2024


On Mon, 19 Aug 2024 11:00:22 GMT, Manukumar V S <mvs at openjdk.org> wrote:

>> This PR creates a new test by stabilising and open sourcing a closed test.
>> This test verifies that the OpenGL pipeline does not create artifacts with swing components after window is zoomed to maximum size and then resized back to normal.
>> This test is run twice, with and without the flags "-Dsun.java2d.opengl=True -Dsun.java2d.opengl.fbobject=false" .
>> 
>> This is tested(15 times per platform) in all the available mach5 headful platforms and found to be stable.
>
> Manukumar V S has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments fixed : Moved the button up to prevent mix up, saved the diff image in failure cases

test/jdk/javax/swing/JButton/SwingButtonResizeTestWithOpenGL.java line 126:

> 124: 
> 125:                 frame.setExtendedState(JFrame.MAXIMIZED_BOTH);  //maximize
> 126:                 // frame from normal size

Suggestion:

            // some platforms may not support maximize frame
            if (frame.getToolkit().isFrameStateSupported(JFrame.MAXIMIZED_BOTH)) {

                robot.waitForIdle();

                frame.setExtendedState(JFrame.MAXIMIZED_BOTH);  //maximize
                // frame from normal size


I see some of your comments with and without a space after the `//`. Maybe standardize this to have the space.

test/jdk/javax/swing/JButton/SwingButtonResizeTestWithOpenGL.java line 148:

> 146:                     int numDiffPixels = di.getNumDiffPixels();
> 147:                     if (numDiffPixels != 0) {
> 148:                         throw new RuntimeException("Button renderings are " + "different after window resize," + " num of Diff Pixels=" + numDiffPixels);

Why are these prints composed of multiple strings with `+` joining them instead of one continuous string sentence? Seems a little odd and is seen multiple times throughout the test.

Suggestion:

                    System.out.println("Getting image of JButton after resize..image2");
                    bimage2 = getButtonImage();
                    File file2 = new File("image2.png");
                    saveButtonImage(bimage2, file2);

                    // compare button images from before and after frame resize
                    DiffImage di = new DiffImage(bimage1.getWidth(), bimage1.getHeight());
                    di.compare(bimage1, bimage2);
                    System.out.println("Taking the diff of two images, image1 and image2");
                    // results of image comparison
                    int numDiffPixels = di.getNumDiffPixels();
                    if (numDiffPixels != 0) {
                        throw new RuntimeException("Button renderings are different after window resize, num of Diff Pixels = " + numDiffPixels);

test/jdk/javax/swing/JButton/SwingButtonResizeTestWithOpenGL.java line 260:

> 258:             nDiff = 0;
> 259:             for (x = minx1; x < (minx1 + w1); x++) {
> 260:                 for (y = miny1; y < (miny1 + h1); y++) {

What's wrong with declaring the variable inside the for loop parameters? Condenses the code a bit too.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20532#discussion_r1724122331
PR Review Comment: https://git.openjdk.org/jdk/pull/20532#discussion_r1724123521
PR Review Comment: https://git.openjdk.org/jdk/pull/20532#discussion_r1724126553


More information about the client-libs-dev mailing list