Re: RFR: 8311031: JTable header border vertical lines are not aligned with data grid lines [v9]

[Alexey Ivanov]

On Thu, 27 Jul 2023 04:58:33 GMT, Tejesh R <t...@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.
>> 
>> ![image](https://github.com/openjdk/jdk/assets/94159358/f6d1d822-55ba-4ad3-9914-d3f68b67a6c5)
>
> Tejesh R has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Review fix

test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 45:

> 43:  */
> 44: 
> 45: public class TableHeaderBorderPositionTest {

Suggestion:

 */
public class TableHeaderBorderPositionTest {

test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 91:

> 89:         } finally {
> 90:             g2d.dispose();
> 91:         }

I wonder if it's possible to make the table paint its header into the same 
image, this would simplify the test.

test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 94:

> 92: 
> 93:         int verticalLineCol = (int)(table.getTableHeader().
> 94:                 getColumnModel().getColumn(0).getWidth() * SCALE) - 2;

Is it reasonable to use the same formatting as in #14464, in 
`JTableHeaderLabelRightAlignTest.java`?

test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 108:

> 106: 
> 107:         int maxVerticalLine = (int) Math.ceil(
> 108:                 table.getRowCount() * table.getRowHeight() * SCALE);

Suggestion:

        int maxVerticalLine = (int)Math.ceil(
                table.getRowCount() * table.getRowHeight() * SCALE);

In all the other cases, you didn't add a space after the cast operator.

test/jdk/javax/swing/JTableHeader/TableHeaderBorderPositionTest.java line 112:

> 110:             for (int x = verticalLineCol; x < verticalLineCol + 3; x++) {
> 111:                 if (expectedRGB != imgData.getRGB(x, y)) {
> 112:                     saveImage(imgData,"FailureImageData.png");

Suggestion:

                    saveImage(imgData, "FailureImageData.png");

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1277704878
PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1277707604
PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1277710561
PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1277706469
PR Review Comment: https://git.openjdk.org/jdk/pull/14766#discussion_r1277652177