On Mon, 19 Aug 2024 11:00:22 GMT, Manukumar V S <[email protected]> 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
