<Swing Dev> [9] RFR JDK-8159068:The rendering of JTable is broken

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 6/24/2016 12:37 PM, Prasanta Sadhukhan wrote:
>
>
> On 6/24/2016 1:41 PM, Alexandr Scherbatiy wrote:
>> On 6/24/2016 10:04 AM, Prasanta Sadhukhan wrote:
>>> Hi Alexandr,
>>>
>>> Thanks for your suggestion. Have modified as per the comment to 
>>> decide based on selected row. 8081491 
>>> <https://bugs.openjdk.java.net/browse/JDK-8081491> testcase and 
>>> SwingSet2 JTable demo works.
>>> Please review modified webrev:
>>> http://cr.openjdk.java.net/~psadhukhan/8159068/webrev.01/
>>     - createUI() also should be called from EDT because it  uses 
>> Swing components
>>     - It is better to use CountDownLatch for manual tests 
>> synchronization (it has special await(...) method with timeout 
>> parameter).
>>
> Modified testcase to use CountDownLatch
> http://cr.openjdk.java.net/~psadhukhan/8159068/webrev.02/
>
> Regards
> Prasanta
>>   Thanks,
>>   Alexandr.
>>>
>>> Regards
>>> Prasanta
>>> On 6/23/2016 12:41 PM, Alexandr Scherbatiy wrote:
>>>> On 6/23/2016 9:53 AM, Prasanta Sadhukhan wrote:
>>>>>
>>>>>
>>>>> On 6/23/2016 12:10 PM, Alexandr Scherbatiy wrote:
>>>>>> On 6/21/2016 2:57 PM, Prasanta Sadhukhan wrote:
>>>>>>>
>>>>>>> On 6/21/2016 5:16 PM, Alexandr Scherbatiy wrote:
>>>>>>>> On 6/21/2016 1:58 PM, Prasanta Sadhukhan wrote:
>>>>>>>>>
>>>>>>>>> On 6/21/2016 4:14 PM, Alexandr Scherbatiy wrote:
>>>>>>>>>> On 6/20/2016 8:10 AM, prasanta sadhukhan wrote:
>>>>>>>>>>>
>>>>>>>>>>> Gentle reminder for review!!
>>>>>>>>>>>
>>>>>>>>>>> Regards
>>>>>>>>>>> Prasanta
>>>>>>>>>>> On 6/13/2016 4:31 PM, prasanta sadhukhan wrote:
>>>>>>>>>>>> On 6/13/2016 12:51 PM, prasanta sadhukhan wrote:
>>>>>>>>>>>>> Hi All,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Please review a fix for jdk9 where it was seen that if we 
>>>>>>>>>>>>> try to select some rows in a JTable, the text painted in 
>>>>>>>>>>>>> the rows goes missing.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8159068
>>>>>>>>>>>>>
>>>>>>>>>>>>> webrev: 
>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8159068/webrev.00/
>>>>>>>>>>>>>
>>>>>>>>>>>>> The issue was rMax value was decremented wrongly so when 
>>>>>>>>>>>>> paintCells() is called with wrong rMax, some rows were not 
>>>>>>>>>>>>> printed correctly.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Fix is to make sure rmax is decremented properly, only 
>>>>>>>>>>>>> when we are trying to print whole visible portion of 
>>>>>>>>>>>>> JTable and NOT when some rows are being painted.
>>>>>>>>>>
>>>>>>>>>>   Could you give two samples how this algorithm work. One 
>>>>>>>>>> sample where a whole visible portion of a JTable and another 
>>>>>>>>>> where some rows are being printed. What are rMax and  rMin 
>>>>>>>>>> values in both cases and how are they calculated?
>>>>>>>>>>
>>>>>>>>> If a JTable is of 50 rows and only 35 are being visible in 
>>>>>>>>> page 1, then
>>>>>>>>> if whole visible portion of JTable is printed, rMin will be 0 
>>>>>>>>> and rMax was 35
>>>>>>>>> so 36 rows were getting printed so I decrement rMax by 1 to 34 
>>>>>>>>> so only 35 will be printed (same as shown on console).
>>>>>>>>> When we select some row of JTable as in the case of LostText 
>>>>>>>>> testcase, rMin will be say 6 and rMax will be 9 in which case 
>>>>>>>>> also, I was decrementing rMax so rMin=6, rMax=8 so next row 
>>>>>>>>> was not getting painted.
>>>>>>>>    And what are indices of the selected rows?
>>>>>>>>
>>>>>>> It will depend on the last selection. At start, rMin = 0 , rMax 
>>>>>>> = last indice, say 10 for a JTable of 10 rows
>>>>>>> Now, if we select row 5, rMin and rMax both becomes 5 and we 
>>>>>>> decrement rMax so rMax becomes less than rMin and paintCell() 
>>>>>>> due to this check
>>>>>>> (int row = rMin; row <= rMax; row++) it does not do
>>>>>>> paintCell(g, cellRect, row, column)
>>>>>>> and nothing gets painted.
>>>>>>    It is not clear how the selection interval [0..N] is 
>>>>>> distinguishable from the case where is no selection because rMin 
>>>>>> should be 0 in both case. May be it is better to decrement the 
>>>>>> rMax depending on are there selected rows or not.
>>>>> Can you suggest if there any way we can find out if there are any 
>>>>> selected rows or not?
>>>>    There is the method table.getSelectedRowCount(). May be for 
>>>> performance reason it is better to check table.getSelectedRow().
>>>>
>>>>   Thanks,
>>>>   Alexandr.
>>>>
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>>>
>>>>>>   Thanks,
>>>>>>   Alexandr.
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>>>   Thanks,
>>>>>>>>   Alexandr.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> Prasanta
>>>>>>>>>>   Thanks,
>>>>>>>>>>   Alexandr.
>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regarding the regression testcase, I could not make it 
>>>>>>>>>>>>> automated as the failure happens on random iteration.
>>>>>>>>>>>>> and also, getting selection background/foreground was 
>>>>>>>>>>>>> giving same values with and without the missing text.
>>>>>>>>>>>>>
>>>>>>>>>>>> Also, since it is a regression of 8081491 
>>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8081491>, it's 
>>>>>>>>>>>> testcase are working fine with this fix and so did 
>>>>>>>>>>>> SwingSet2 JTable demo.
>>>>>>>>>>>>> Regards
>>>>>>>>>>>>> Prasanat
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160624/686d93a1/attachment.html>