On Mon, 20 Nov 2023 05:24:51 GMT, Tejesh R <t...@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
