<Swing Dev> RFR: [9] [JDK-8081491] The case print incomplete.
Alexandr Scherbatiy
alexandr.scherbatiy at oracle.com
Mon Sep 21 08:50:22 UTC 2015
18.09.2015 10:16, prasanta sadhukhan пишет:
>
>
> On 9/17/2015 8:18 PM, Alexander Scherbatiy wrote:
>> On 9/16/2015 2:04 PM, prasanta sadhukhan wrote:
>>> Hi Alexander, Sergey,
>>>
>>> Waiting for your review on this.
>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.06/
>>
>> Could you describe why the paint artifacts are drawn when a scroll
>> pane is present?
> I see that normally JTable has always been associated with JScrollpane
> and it uses
> // Paint the grid.
> paintGrid(g, rMin, rMax, cMin, cMax);
> // Paint the cells.
> paintCells(g, rMin, rMax, cMin, cMax);
> to paint the cells in the table.
> When we scroll the table, rMin can be say 41 and rMax can be 43 so it
> expects to draw 3 rows with the above code (since the for loop uses
> rows = rMin; rows <= rMax)
> Also, sometimes rMin canbe 44 and rMax can be 44 too in which case 1
> row would be painted as per the above for loop
>
> but since I have modified the code to use (to make same rows to show
> on console and in printed page)
>
> // Paint the grid.
> paintGrid(g, rMin, rMax-1, cMin, cMax);
> // Paint the cells.
> paintCells(g, rMin, rMax-1, cMin, cMax);
>
> it paints only 2 rows (or 0 rows in case rMin=rMax=44 where rMax-1 is
> 43 so for loop will not be executed) and when we go on scrolling, 1
> less row gets painted always than what it expects resulting in artifacts.
> So, I have kept the same code for JTable when it has scrollpane (which
> was till now the case)
- Does it mean if the initialpaintGrid()/Cell() methods are used
there are artifacts when a table is not used with JScrollPane?
- It is not necessary to add isScrollPanePresent varibale if it is
used only once
Thanks,
Alexandr.
>
> Regards
> Prasanta
>>
>> Thanks,
>> Alexandr.
>>>
>>> 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