RFR: 5108458: JTable does not properly layout its content [v5]

[Prasanta Sadhukhan]

On Mon, 20 Nov 2023 05:24:51 GMT, Tejesh R <tr at openjdk.org> wrote:

>> Table contents does not follow right-left Orientation when Max width of columns are set. This is due to not considering the offset in `x position` while painting table grid and table cell. The fix handles the offset and adjust the x position for each paint, similar to how header is painted. The fix is applied to both Basic and Synth Look and Feel. 
>> The fix is verified for all Look and Feel manually and test verifies on Metal L&F since automatic test cannot be generalized throughout other Look and Feel. 
>> CI tested is green for regression check and test check.
>
> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review fix

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTableUI.java line 2087:

> 2085:             for (int row = rMin; row <= rMax; row++) {
> 2086:                 cellRect = table.getCellRect(row, cMin, false);
> 2087:                 cellRect.x = getXPosition(cMin);

Why do we need to calculate cellRect.x separately? What is wrong in getting the value already there in `cellRect` ?

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTableUI.java line 2101:

> 2099:             for (int row = rMin; row <= rMax; row++) {
> 2100:                 cellRect = table.getCellRect(row, cMin, false);
> 2101:                 cellRect.x = getXPosition(cMax);

Is this not a problem that `cellRect` is calculated for `cMin` but `cellRect.x` is calculated for `cMax` which means `cellRect.y` would be of `cMin` column but `cellRect.x` would be of `cMax` column. SImilarly for the `cellRect.width`...You probably can get away maybe because usually all cells have same width but I guess it will be a problem if different cells have different width...Please check..

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTableUI.java line 2102:

> 2100:                 cellRect = table.getCellRect(row, cMin, false);
> 2101:                 cellRect.x = getXPosition(cMax);
> 2102:                 for (int column = cMax; column >= cMin; column--) {

Is there any reason for choosing to paint from cMax to cMin ie from left-to-right in RTL orientation
when it was done from right-to-left before your fix? 
I guess painting from right-to-left also should work with cellRect.x -= columnWidth ie with no change in code, no?

One more q,
If there are 5 columns, then is it that for LTR cMin is 1, cMax 5 and for RTL cMin is 5 , cMax 1 or viceversa?

src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTableUI.java line 611:

> 609:                     cellRect.width = columnWidth - columnMargin;
> 610:                     paintCell(context, g, cellRect, row, cMax);
> 611:                 }

I guess you removed this in BasicTableUI...Any particular reason for keeping this for Synth..

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16374#discussion_r1398918527
PR Review Comment: https://git.openjdk.org/jdk/pull/16374#discussion_r1398858032
PR Review Comment: https://git.openjdk.org/jdk/pull/16374#discussion_r1398883285
PR Review Comment: https://git.openjdk.org/jdk/pull/16374#discussion_r1398925522