RFR: 8311031: JTable header border vertical lines are not aligned with data grid lines [v10]
Alexey Ivanov
aivanov at openjdk.org
Mon Jul 31 20:43:58 UTC 2023
On Mon, 31 Jul 2023 16:31:21 GMT, Tejesh R <tr at openjdk.org> wrote:
>> The header border uses `g.drawLine` whereas the JTable data grid lines uses `SwingUtilities2.drawVLine` and `SwingUtilities2.drawHLine` to draw horizontal and vertical lines. The SwingUtilities2 uses `Graphics.fillRect` which contributes to the difference between the position of these two lines which happens/visible at higher ui scaling (difference in alignment between vertical lines of these two). The fix propose to use the same methods for metal L&F of JTable header border paint.
>> CI testing shows green.
>>
>> 
>
> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
>
> Review fix
Changes requested by aivanov (Reviewer).
test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 68:
> 66:
> 67: BufferedImage bufferedImage = new BufferedImage(WIDTH, HEIGHT,
> 68: BufferedImage.TYPE_INT_RGB);
Hm, you have to create the image large enough to contain the table after scaling it by `SCALE`, so you still need `w` and `h` or you could inline `WIDTH * SCALE` and `HEIGHT * SCALE` correspondingly.
test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 76:
> 74: g2d.translate(0, header.getHeight());
> 75: table.paint(g2d);
> 76: } finally {
Awesome! I didn't think about it. If it works, it makes verification easier. When you read the code, you clearly see the borders of the header and grid align.
I expected the table would paint both its header and its contents, but it may require validation which can't happen until the table is added to a heavyweight container such as a frame.
test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 83:
> 81: .getColumnModel()
> 82: .getColumn(0)
> 83: .getWidth() * SCALE) - 2;
Suggestion:
int verticalLineCol = (int)(table.getTableHeader()
.getColumnModel()
.getColumn(0)
.getWidth() * SCALE) - 2;
If you add a space after the cast operator, add it everywhere; if you don't, add it nowhere.
I could've given you a suggestion based on my formatting, and I have the space after the cast operator because Java Coding Style recommends adding it. Yet there's no consensus about it in client-libs.
test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 85:
> 83: .getWidth() * SCALE) - 2;
> 84: int expectedRGB = bufferedImage.getRGB(verticalLineCol, 0);
> 85: int maxHeight = (int)(((double)table.getTableHeader().getHeight() * SCALE )
Suggestion:
int maxHeight = (int)(((double)table.getTableHeader().getHeight() * SCALE)
-------------
PR Review: https://git.openjdk.org/jdk/pull/14766#pullrequestreview-1555731207
PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1279855301
PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1279853146
PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1279858473
PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1279850115
More information about the client-libs-dev
mailing list