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
8279614: The left line of the TitledBorder is not painted on 150 scale factor #7449
Changes from 10 commits
c368173
6e24762
b2e9240
4c8d5bf
f314462
c0b3189
47437d9
88c1b1a
6056faf
7b7732f
06ac18d
3ecaf58
75ec24d
9109b84
5d4854d
4dc1287
6c2efaf
a793582
89e64e9
a50bda0
6767407
e7b2706
b84cf51
e32e6c0
b17cdab
65200a2
61fc0ae
935ec43
cf91eb8
1d6ac07
8f71aba
adca6ef
24d7cee
325d09b
35e7d5a
317fe35
93f0b73
610d8f8
d4ad493
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
* Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved. | ||
* Copyright (c) 1997, 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 | ||
|
@@ -33,6 +33,7 @@ | |
import java.awt.Insets; | ||
import java.awt.Rectangle; | ||
import java.awt.geom.Path2D; | ||
import java.awt.geom.Rectangle2D; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import isn't used. There are no other changes to this file but this added import. In addition to it, you can also remove java.beans.PropertyChangeEvent from imports which is unused as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either revert the changes to |
||
import java.beans.ConstructorProperties; | ||
import java.beans.PropertyChangeEvent; | ||
import java.beans.PropertyChangeListener; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is removing an unused import the only change to TitledBorder ? I think you should revert this to make the change cleaner. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
/* | ||
* 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. Oracle designates this | ||
* particular file as subject to the "Classpath" exception as provided | ||
* by Oracle in the LICENSE file that accompanied this code. | ||
* | ||
* 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. | ||
*/ | ||
import javax.imageio.ImageIO; | ||
import javax.swing.BorderFactory; | ||
import javax.swing.JCheckBox; | ||
import javax.swing.JFrame; | ||
import javax.swing.JPanel; | ||
import javax.swing.SwingUtilities; | ||
import javax.swing.UIManager; | ||
import javax.swing.UIManager.LookAndFeelInfo; | ||
import javax.swing.UnsupportedLookAndFeelException; | ||
import java.awt.BorderLayout; | ||
import java.awt.Graphics2D; | ||
import java.awt.Robot; | ||
import java.awt.image.BufferedImage; | ||
import java.io.File; | ||
import java.io.IOException; | ||
|
||
/* | ||
* @test | ||
* @key headful | ||
* @bug 8279614 | ||
* @summary The left line of the TitledBorder is not painted on 150 scale factor | ||
* @requires (os.family == "windows") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test is not Windows specific, is it? Yet Java supports fractional scales on Windows only at the moment, right? I still think it makes sense to run the test on Linux and macOS for 100%, 200% and 300% which are supported. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well the test is setting the Windows L&F, and evaluating its rendering. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right, the bug was reported on Windows. The Windows L&F uses Yet the fix is in the shared code, in the If the test uses * In fact, Windows 10 renders titled border flat: one grey line instead of etched border that was used in previous versions. Shall we update Swing's Windows L&F? |
||
* @run main TitledBorderTest | ||
*/ | ||
public class TitledBorderTest { | ||
|
||
public static JFrame frame; | ||
public static JPanel parentPanel; | ||
public static JPanel childPanel; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please correct the indentation in the test: each level is indented by four spaces. |
||
|
||
public static void main(String[] args) throws Exception { | ||
LookAndFeelInfo laf = UIManager.getInstalledLookAndFeels()[3]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the test needs to be run in a specific LookAndFeel? If so, you have to set explicitly rather than rely on the order of LAFs. |
||
System.out.println(laf); | ||
SwingUtilities.invokeAndWait(() -> setLookAndFeel(laf)); | ||
SwingUtilities.invokeAndWait(() -> createAndShowGUI()); | ||
|
||
Robot robot = new Robot(); | ||
|
||
BufferedImage buff = new BufferedImage(frame.getWidth()*2, frame.getHeight()*2, | ||
BufferedImage.TYPE_INT_ARGB); | ||
Graphics2D graph = buff.createGraphics(); | ||
graph.scale(1.5, 1.5); | ||
frame.paint(graph); | ||
graph.dispose(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we paint to Shall we run the test with a set of scales: 1.00, 1.25, 1.50, 1.75, 2.00? Without the fix, the left line looks clipped at 2.0 scale too: the shadow is 1-pixel wide on the left whereas the highlight is 2-pixel wide on the four edges. |
||
|
||
robot.waitForIdle(); | ||
boolean testFail = true; | ||
for (int i = 15; i < 25 && testFail == true; i++) { | ||
for (int j = 80; j < 100; j++) { | ||
if (buff.getRGB(i, j) == -6250336) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using hex for colors is better. Getting the highlight or shadow color from the border would make the test more generic. If the color used by border changes, the test will fail but it shouldn't. |
||
System.out.println(i + " " + j + " Color " + buff.getRGB(i, j)); | ||
testFail = false; | ||
break; | ||
} | ||
} | ||
} | ||
|
||
if (testFail) { | ||
saveImage(buff, "test.png"); | ||
throw new RuntimeException("Border was clipped or overdrawn."); | ||
} | ||
|
||
frame.dispose(); | ||
} | ||
|
||
private static void createAndShowGUI() { | ||
frame = new JFrame("Swing Test"); | ||
frame.setSize(new java.awt.Dimension(300, 200)); | ||
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); | ||
|
||
parentPanel = new JPanel(new BorderLayout()); | ||
parentPanel.setBorder(BorderFactory.createEmptyBorder(5, 5, 5, 5)); | ||
|
||
childPanel = new JPanel(new BorderLayout()); | ||
childPanel.setBorder(BorderFactory.createTitledBorder("Title")); | ||
childPanel.add(new JCheckBox(), BorderLayout.CENTER); | ||
|
||
parentPanel.add(childPanel, BorderLayout.CENTER); | ||
|
||
frame.getContentPane().add(parentPanel, BorderLayout.CENTER); | ||
|
||
frame.pack(); | ||
frame.setLocationRelativeTo(null); | ||
frame.setVisible(true); | ||
} | ||
|
||
private static void saveImage(BufferedImage image, String filename) { | ||
try { | ||
ImageIO.write(image, "png", new File(filename)); | ||
} catch (IOException e) { | ||
// Don’t propagate the exception | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend using a regular apostrophe (') instead of typographic one (’). It shouldn't cause issues with compiling because it's in the comment, yet the character may be displayed incorrectly if the default system encoding doesn't match the encoding of the file. |
||
e.printStackTrace(); | ||
} | ||
} | ||
|
||
private static void setLookAndFeel(UIManager.LookAndFeelInfo laf) { | ||
try { | ||
UIManager.setLookAndFeel(laf.getClassName()); | ||
System.out.println(laf.getName()); | ||
} catch (UnsupportedLookAndFeelException ignored){ | ||
System.out.println("Unsupported LookAndFeel: " + laf.getClassName()); | ||
} catch (ClassNotFoundException | InstantiationException | | ||
IllegalAccessException e) { | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the opposite never happen? That the shadow overdraws the highlight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a comment on the actual comment rather than whether the intent is correct.
I'm not in favour of putting bug ids into the comments. Our code will end up utterly cluttered with them
Also it is phrased to describe a fix.
So IF it were to stand as is, I would have phrased it as "what we are doing now and why", ie
// Make sure the shadow line always is drawn second because it needs to over draw the border.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should've posted my question separately rather than as a comment to the comment.