Looks good to me.
Regards, Rajeev Chamyal From: Alexandr Scherbatiy Sent: 24 June 2016 17:01 To: Prasanta Sadhukhan; Rajeev Chamyal; swing-dev@openjdk.java.net Subject: Re: <Swing Dev> [9] RFR JDK-8159068:The rendering of JTable is broken 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. HYPERLINK "https://bugs.openjdk.java.net/browse/JDK-8081491"8081491 testcase and SwingSet2 JTable demo works. Please review modified webrev: HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/8159068/webrev.01/"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 HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/8159068/webrev.02/"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: HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/8159068/webrev.00/"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 HYPERLINK "https://bugs.openjdk.java.net/browse/JDK-8081491"8081491, it's testcase are working fine with this fix and so did SwingSet2 JTable demo. Regards Prasanat
