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

[Tejesh R]

On Mon, 20 Nov 2023 09:45:19 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> 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` ?

The value we get from `table.getCellRect(row, cMin, false);` doesn't consider x position offset, so we need to update with the offset value.

> 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?

Yes, we can either paint from right-to-left or from left-to-right. I followed `JTableHeader ` paint logic.  for 5 columns cMin = 0 and cMax = 4.

> 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..

Nothing particular, can remove and update there too.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16374#discussion_r1398953181
PR Review Comment: https://git.openjdk.org/jdk/pull/16374#discussion_r1398951260
PR Review Comment: https://git.openjdk.org/jdk/pull/16374#discussion_r1398954259