Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8287912: GTK L&F : Background of tree icons are red #10112

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -1275,8 +1275,10 @@ public void paintTreeCellBackground(SynthContext context, Graphics g,
ENGINE.startPainting(g, x, y, w, h, id, state);
// the string arg should alternate based on row being painted,
// but we currently don't pass that in.
ENGINE.paintFlatBox(g, context, id, gtkState, ShadowType.NONE,
"cell_odd", x, y, w, h, ColorType.TEXT_BACKGROUND);
if (context.getComponent().isOpaque()) {
ENGINE.paintFlatBox(g, context, id, gtkState, ShadowType.NONE,
"cell_odd", x, y, w, h, ColorType.TEXT_BACKGROUND);
}
ENGINE.finishPainting();
}
}
Expand Down
112 changes: 112 additions & 0 deletions test/jdk/javax/swing/JTree/TestTreeBackgroundColor.java
@@ -0,0 +1,112 @@

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this empty line

/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8287912
* @key headful
* @requires (os.family == "linux")
* @summary Verifies if tree background color is red when
* setOpaque(false) method is called for tree component
* @run main TestTreeBackgroundColor
*/

import java.awt.Color;
import java.awt.GridLayout;
import java.awt.Point;
import java.awt.Rectangle;
import java.awt.Robot;
import java.awt.image.BufferedImage;
import java.io.File;
import javax.imageio.ImageIO;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTree;
import javax.swing.SwingUtilities;
import javax.swing.UIManager;

public class TestTreeBackgroundColor {

private static JFrame frame;
private static JPanel panel;
private static JTree tree;

public static void main(String[] args) throws Exception {
UIManager.setLookAndFeel("com.sun.java.swing.plaf.gtk.GTKLookAndFeel");
Robot robot = new Robot();
robot.setAutoDelay(100);
try {
SwingUtilities.invokeAndWait(new Runnable() {
public void run() {
createAndShowUI();
}
});

robot.waitForIdle();
robot.delay(1000);
Point pt = tree.getLocationOnScreen();
BufferedImage img =
robot.createScreenCapture(new Rectangle(pt.x, pt.y,
tree.getWidth(),
tree.getHeight()));

boolean passed = false;
for (int x = 0; x < img.getWidth() - 1; ++x) {
for (int y = 0; y < img.getHeight() - 1; ++y) {
Color c = new Color(img.getRGB(x, y));

if (c.equals(Color.RED)) {
passed = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to check at midpoint tree.width/2, tree.height/2 and also break if it's RED, no need to check all pixels.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possible solution that I used in a previous test was a midpoint as suggested, but also one pixel near each edge. This eliminates the chance of the red still existing but not being detected at the midpoint because the red is at a different location.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test check has been changed to compare the pixels of entire width keeping the height constant to half of tree height. Now it will check pixel color and if it is not red , the test will fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, test is not failing without the fix.
Also, if you test entire width and fail if it is not red, it might encounter black pixel of text font too and might fail, so I think it's better to check width/2 to width -1 and not entire width.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it's might be better to write the image in case of failure so that it gives an idea why it failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, test is not failing without the fix. Also, if you test entire width and fail if it is not red, it might encounter black pixel of text font too and might fail, so I think it's better to check width/2 to width -1 and not entire width.

Test was not failing without the fix because the second argument of getRGB method is height/2, and for that pixel value the returned color is red.
Changing the argument from height/2 to height/4 will fail the test without the fix otherwise it will pass the test.

Also writing the image in case of failure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok .Please remove the empty 1st line in the test.

}
}
if (!passed) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test will pass even in the case of last pixel check is RED. We should check for pixel data and whenever it is not RED we should bail out.

Also if test fails it will leave TreeBackgroundColorTestFail.png, we should use temporary file and delete it on exit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the writing of test image in case of fail operation and test condition changed to check if pixel color is not red then test will fail.

ImageIO.write(img, "png", new File("TreeBackgroundColorTestFail.png"));
throw new RuntimeException("Test Case Failed : Tree Background Color is Not Red");
}
} finally {
SwingUtilities.invokeAndWait(() -> {
if (frame != null) {
frame.dispose();
}
});
}
}

private static void createAndShowUI() {
frame = new JFrame();
panel = new JPanel();
tree = new JTree();
panel.setBackground(Color.red);
panel.setLayout(new GridLayout(1, 1));
tree.setOpaque(false);
panel.add(tree);
frame.getContentPane().add(panel);
frame.pack();
frame.setSize(250, 200);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
}
}