<Swing Dev> RFR: [9] [JDK-8081491] The case print incomplete.
prasanta sadhukhan
prasanta.sadhukhan at oracle.com
Wed Sep 16 11:04:14 UTC 2015
Hi Alexander, Sergey,
Waiting for your review on this.
http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.06/
Regards
Prasanta
On 9/15/2015 10:55 AM, prasanta sadhukhan wrote:
>
>
> On 9/14/2015 12:48 PM, prasanta sadhukhan wrote:
>>
>>
>> On 9/11/2015 2:20 PM, prasanta sadhukhan wrote:
>>>
>>>
>>> On 9/10/2015 4:48 PM, Sergey Bylokhov wrote:
>>>> On 10.09.15 13:35, prasanta sadhukhan wrote:
>>>>>
>>>>>
>>>>> On 9/10/2015 3:42 PM, Sergey Bylokhov wrote:
>>>>>> On 10.09.15 9:36, prasanta sadhukhan wrote:
>>>>>>> Please review the modified webrev which solves this artifacts.
>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.05/
>>>>>>
>>>>>> What will happen if the table will be added to the jpanel and the
>>>>>> jpanel will be added to JScrollPane? Will this configuration work as
>>>>>> expected?
>>>>>>
>> Please review which takes care of this configuration
>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.06/
> Gentle reminder for review request.
>>>>>>>
>>>>>>> I also added a reg test for this regression but I am not able to
>>>>>>> create
>>>>>>> a automated testcase to deal with the scrolling artifacts, so I
>>>>>>> added a
>>>>>>> manual test.
>>>>>>
>>>>>> I suggest to try to automate it somehow. Probably make some small
>>>>>> unity test? Or using robot?
>>>>> Even with Robot or unity test, how will I check the artifact has
>>>>> happened? THis is a visual problem. I do not know how to test it
>>>>> automatically.
>>>>
>>>> In case of unit test you can check the return value of some methods
>>>> or the state of the objects which are cause the artifacts. For test
>>>> with robot, you can fill all rows of table in some color, then
>>>> scroll it, and check the color of the table using
>>>> robot.getPixelColor().
>>> Thanks Sergey for the suggestion. I am trying to use Robot to test
>>> this artifacts. But when I use Robot to scroll up/down, the
>>> artifacts are not seen even when the scrollbar moved up and down.
>>> But manually if I scroll, I can see the artifacts. Can you please
>>> let me know if the attached testcase is missing something?
>>>
>> Regards
>> Prasanta
>>>>
>>>>>
>>>>> --Prasanta
>>>>>>
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>> On 9/8/2015 4:27 PM, Sergey Bylokhov wrote:
>>>>>>>> Hi, Prasanta.
>>>>>>>> Just before the push of this fix I made small pit, and found a
>>>>>>>> regression. Please run the SwingSet2, open JTable demo, and
>>>>>>>> scroll the
>>>>>>>> table. You will see some artifacts.
>>>>>>>>
>>>>>>>> On 08.09.15 13:13, prasanta sadhukhan wrote:
>>>>>>>>> Thanks Sergey for pointing this.
>>>>>>>>> I have taken care of this plus formatting in for loop.
>>>>>>>>> Please have a look
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.04/
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> Prasanta
>>>>>>>>> On 9/8/2015 3:32 PM, Sergey Bylokhov wrote:
>>>>>>>>>> Hi, Prasanta.
>>>>>>>>>> A few small notes:
>>>>>>>>>> - BasicTableUI: typo "1850 // otherwise 1 extra rows are
>>>>>>>>>> ptinted"
>>>>>>>>>> - ImageableAreaTest: the test instructions have copy pasted
>>>>>>>>>> numbers
>>>>>>>>>> 1/2/2/2 etc.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 08.09.15 12:43, prasanta sadhukhan wrote:
>>>>>>>>>>> Thanks for your review.
>>>>>>>>>>> I need +1 for this. Alexander Z/Sergey, can you please
>>>>>>>>>>> approve
>>>>>>>>>>> this
>>>>>>>>>>> fix?
>>>>>>>>>>>
>>>>>>>>>>> Regards
>>>>>>>>>>> Prasanta
>>>>>>>>>>> On 9/8/2015 3:02 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> The fix looks good to me.
>>>>>>>>>>>>
>>>>>>>>>>>> But you need to properly format spaces in the 'for' loop
>>>>>>>>>>>> on line
>>>>>>>>>>>> TablePrintable:410 before the push.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>
>>>>>>>>>>>> On 9/8/2015 12:26 PM, prasanta sadhukhan wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 9/7/2015 5:50 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>> On 9/7/2015 9:23 AM, prasanta sadhukhan wrote:
>>>>>>>>>>>>>>> I guess it will be same but anyways have modified to use
>>>>>>>>>>>>>>> visibleBounds.getLocation() to be on safeside as we are
>>>>>>>>>>>>>>> dealing
>>>>>>>>>>>>>>> with visible region for this fix.
>>>>>>>>>>>>>>> Please review the updated webrev
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.02/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> TablePrintable:
>>>>>>>>>>>>>> - Could the rMin be equal to -1?
>>>>>>>>>>>>> This should never happen so long bounds intersects the
>>>>>>>>>>>>> clip but as
>>>>>>>>>>>>> done in BasicTableUI, I have added the check just in case
>>>>>>>>>>>>>> - Line: 406 int rowHeight = (rMax-rMin) *
>>>>>>>>>>>>>> table.getRowHeight();
>>>>>>>>>>>>>> Rows can have different height in the table. Could
>>>>>>>>>>>>>> you
>>>>>>>>>>>>>> also
>>>>>>>>>>>>>> add a test for the this case too?
>>>>>>>>>>>>>>
>>>>>>>>>>>>> Added test for this case too.
>>>>>>>>>>>>> Please review this webrev:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.03/
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards
>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>> On 9/4/2015 8:57 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Could the clip.getLocation() be differ from them
>>>>>>>>>>>>>>>> visibleBounds.getLocation()?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 9/4/2015 3:32 PM, prasanta sadhukhan wrote:
>>>>>>>>>>>>>>>>> Any reviewers for this please?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 9/2/2015 5:06 PM, prasanta sadhukhan wrote:
>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Can this fix be reviewed?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>>>>> On 8/28/2015 4:48 PM, prasanta sadhukhan wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 8/26/2015 6:24 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>>>>>> On 8/25/2015 1:51 PM, prasanta sadhukhan wrote:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On 8/25/2015 3:53 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>>>>>>>> On 8/24/2015 2:23 PM, prasanta sadhukhan wrote:
>>>>>>>>>>>>>>>>>>>>>>> Hi All,
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Bug:
>>>>>>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8081491
>>>>>>>>>>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.00/
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> This seems to be a hidden JTable bug in which if
>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>> user
>>>>>>>>>>>>>>>>>>>>>>> does not call pack() or set a ScrollPane() for
>>>>>>>>>>>>>>>>>>>>>>> JTable
>>>>>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>>>> rather use JFrame.setSize() smaller than table size
>>>>>>>>>>>>>>>>>>>>>>> then it
>>>>>>>>>>>>>>>>>>>>>>> was found that some of the rows which cannot be
>>>>>>>>>>>>>>>>>>>>>>> fitted in
>>>>>>>>>>>>>>>>>>>>>>> 1st page cannot get printed on 2nd and
>>>>>>>>>>>>>>>>>>>>>>> subsequent pages
>>>>>>>>>>>>>>>>>>>>>>> resulting in blank cells to be printed after 1st
>>>>>>>>>>>>>>>>>>>>>>> page.
>>>>>>>>>>>>>>>>>>>>>>> It was found that BasicTableUI checks for table
>>>>>>>>>>>>>>>>>>>>>>> bounds to
>>>>>>>>>>>>>>>>>>>>>>> fall within the clip and if they do not
>>>>>>>>>>>>>>>>>>>>>>> intersect, it
>>>>>>>>>>>>>>>>>>>>>>> bails
>>>>>>>>>>>>>>>>>>>>>>> out from painting the table cells.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> What is the reason that the graphics clip
>>>>>>>>>>>>>>>>>>>>>> does not
>>>>>>>>>>>>>>>>>>>>>> intersect the table bounds during printing in the
>>>>>>>>>>>>>>>>>>>>>> provided
>>>>>>>>>>>>>>>>>>>>>> test case?
>>>>>>>>>>>>>>>>>>>>> The testcase does table.setSize(600,800) whereas
>>>>>>>>>>>>>>>>>>>>> frame
>>>>>>>>>>>>>>>>>>>>> setSize is 400,600 .
>>>>>>>>>>>>>>>>>>>>> For 1st page, the clip was 0,0,384,752 and bounds was
>>>>>>>>>>>>>>>>>>>>> 0,0,384,562 so they intersect and there's no
>>>>>>>>>>>>>>>>>>>>> problem in
>>>>>>>>>>>>>>>>>>>>> printing the rows in 1st page.
>>>>>>>>>>>>>>>>>>>>> After the 1st page is printed, the clip is set to
>>>>>>>>>>>>>>>>>>>>> 0,752,384,48 since we have printed the rows that
>>>>>>>>>>>>>>>>>>>>> we can
>>>>>>>>>>>>>>>>>>>>> fit
>>>>>>>>>>>>>>>>>>>>> in 1st page and the next set of rows are to be
>>>>>>>>>>>>>>>>>>>>> printed
>>>>>>>>>>>>>>>>>>>>> while
>>>>>>>>>>>>>>>>>>>>> bounds remains at 0,0,384,562 because JComponent
>>>>>>>>>>>>>>>>>>>>> getBounds is
>>>>>>>>>>>>>>>>>>>>> returning the visible frame bounds which did not
>>>>>>>>>>>>>>>>>>>>> change.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> The !bounds.intersects(clip) check prevents
>>>>>>>>>>>>>>>>>>>> printing of
>>>>>>>>>>>>>>>>>>>> table rows which are not visible on the frame.
>>>>>>>>>>>>>>>>>>>> It seems that the issue is that extra rows
>>>>>>>>>>>>>>>>>>>> which are
>>>>>>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>>>>> shown in the frame are printed on the first page.
>>>>>>>>>>>>>>>>>>>> It means that the printed rows and columns
>>>>>>>>>>>>>>>>>>>> should be
>>>>>>>>>>>>>>>>>>>> calculated for the table bounds and clip intersection.
>>>>>>>>>>>>>>>>>>>> The test can be updated to mention that only
>>>>>>>>>>>>>>>>>>>> visible
>>>>>>>>>>>>>>>>>>>> part
>>>>>>>>>>>>>>>>>>>> of the table should be printed.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Have modified the code to print only the rows that are
>>>>>>>>>>>>>>>>>>> displayed on console. Also updated the test to
>>>>>>>>>>>>>>>>>>> mention the
>>>>>>>>>>>>>>>>>>> same. Please review the updated webrev.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.01/
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Please, also mention in the email title JDK
>>>>>>>>>>>>>>>>>>>>>> version for
>>>>>>>>>>>>>>>>>>>>>> which the fix is provided.
>>>>>>>>>>>>>>>>>>>>> Done
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> I devised a solution whereby it will not bail
>>>>>>>>>>>>>>>>>>>>>>> out till
>>>>>>>>>>>>>>>>>>>>>>> either rows or columns are still left to be
>>>>>>>>>>>>>>>>>>>>>>> printed on
>>>>>>>>>>>>>>>>>>>>>>> subsequent pages . Please review and let me know if
>>>>>>>>>>>>>>>>>>>>>>> it's ok.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
More information about the swing-dev
mailing list