RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v3]
Alexey Ivanov
aivanov at openjdk.java.net
Mon Mar 14 21:04:43 UTC 2022
On Mon, 14 Mar 2022 16:15:47 GMT, Alisen Chung <achung at openjdk.org> wrote:
>> Changed the drawing area to be increased by 0.5 on the left side to prevent clipping
>
> Alisen Chung has updated the pull request incrementally with one additional commit since the last revision:
>
> added functions for drawing border, fixed translate
The test uses 2 spaces for indentation. It should use 4 spaces for each level of indentation.
src/java.desktop/share/classes/javax/swing/border/EtchedBorder.java line 154:
> 152:
> 153: // 8279614: The dark line was being overdrawn by the light line. The fix was to
> 154: // make sure that the dark line is always drawn second.
Suggestion:
// 8279614: The dark line was overdrawn by the light line.
// The fix makes sure that the dark line is always drawn second.
Does the opposite never happen? That the shadow overdraws the highlight.
src/java.desktop/share/classes/javax/swing/border/TitledBorder.java line 36:
> 34: import java.awt.Rectangle;
> 35: import java.awt.geom.Path2D;
> 36: import java.awt.geom.Rectangle2D;
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.
test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 56:
> 54:
> 55: public static void main(String[] args) throws Exception {
> 56: LookAndFeelInfo laf = UIManager.getInstalledLookAndFeels()[3];
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.
test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 68:
> 66: graph.scale(1.5, 1.5);
> 67: frame.paint(graph);
> 68: graph.dispose();
If we paint to `BufferedImage`, it makes sense to bypass creating and showing the frame, a panel with `TitledBorder` will be enough. In this case, the test can be headless.
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.
test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 74:
> 72: for (int i = 15; i < 25 && testFail == true; i++) {
> 73: for (int j = 80; j < 100; j++) {
> 74: if (buff.getRGB(i, j) == -6250336) {
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.
test/jdk/java/awt/TitledBorder/TitledBorderTest.java line 115:
> 113: ImageIO.write(image, "png", new File(filename));
> 114: } catch (IOException e) {
> 115: // Don’t propagate the exception
I recommend using a regular apostrophe (') instead of typographic one (’). It should 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.
-------------
Changes requested by aivanov (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7449
More information about the client-libs-dev
mailing list