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