ok. I changed it initially to see if it helps in printing same number of
rows as displayed but forgot to change it back after adding the rMax
decrement in SwingUtilities.getUnwrappedParent(table) check.
Please find the updated webrev:
http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.10/
Regards
Prasanta
On 9/30/2015 4:14 AM, Phil Race wrote:
On 09/28/2015 11:53 PM, prasanta sadhukhan wrote:
Have updated the webrev to cater to this empty lines
http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.09/
The below code was there initially and I have not added it. It was
there because
table.rowAtPoint() can return -1 when y/getRowHeight() >=
getRowCount() so that rMax points to last row
[say, we have 45 rows but page can fit 50 rows so y/getRowHeight()
will be greater than getRowCount so rMax point to 45th row]
You may not have "added" it but you did change it.
rMax = table.getRowCount()-1;
has become
rMax = table.getRowCount();
Now we will try to access one past the end of the row count
-phil.
Regards
Prasanta
On 9/29/2015 4:03 AM, Phil Race wrote:
Sorry to be nit-picky but TablePrintable.java seems to have
gained some new empty lines and lost other lines that make
the 'diff' look much larger than it really is.
Also a couple in BasicTableUI have the same.
I also see this comment seems to be invalidated by the change
in some cases and it also seems like we may then ask for a row or
cell height
for a row or cell that does not exist ! Why is that OK ?
1829 // If the table does not have enough rows to fill the
view we'll get -1.
1830 // (We could also get -1 if our bounds don't intersect
the clip,
1831 // which is why we bail above if that is the case).
1832 // Replace this with the index of the last row.
1833 if (rMax == -1) {
1834 rMax = table.getRowCount();
1835 }
-phil.
On 09/25/2015 04:47 AM, prasanta sadhukhan wrote:
Thanks Alexander. Can I get a +1 for this?
Regards
Prasanta
On 9/25/2015 2:43 PM, Alexander Scherbatiy wrote:
The fix looks good to me.
Thanks,
Alexandr.
On 9/25/2015 9:26 AM, prasanta sadhukhan wrote:
Added null check for SwingUtilities.getUnwrappedParent(table).
Please review the updated webrev
http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.08/
Regards
Prasanta
On 9/24/2015 5:19 PM, Alexander Scherbatiy wrote:
On 9/23/2015 12:26 PM, prasanta sadhukhan wrote:
On 9/23/2015 2:46 PM, Alexander Scherbatiy wrote:
On 9/23/2015 9:42 AM, prasanta sadhukhan wrote:
I have updated the code as per your comment.
Please review this webrev
http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.07/
- Is it possible that
SwingUtilities.getUnwrappedParent(table) returns null?
I have not seen it. It will return at least JRootPane, I guess.
Is it possible just crate a JTable with some rows and print
it, without adding to a frame or some others components?
Thanks,
Alexandr.
- Does the fix work correctly for a case when rMax has
initial zero value but it is decremented on line 1857?
rMax can have -1 from table.rowAtPoint() in which case it will
be changed to total rowCount so it will not be 0 before line 1857.
Regards
Prasanta
Thanks,
Alexandr.
Regards
Prasanta
On 9/22/2015 7:01 PM, Alexander Scherbatiy wrote:
On 9/21/2015 12:05 PM, prasanta sadhukhan wrote:
On 9/21/2015 2:20 PM, Alexandr Scherbatiy wrote:
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?
When table is not used with JScrollPane, there is no change
of table visible rows (since user is not scrolling the
table) so there is no artifacts if table does not have
jscrollpane.
- It is not necessary to add isScrollPanePresent
varibale if it is used only once
I did not understand. It's a variable and not a function.
So, what you are proposing me to do?
It is possible just to use
-----------------------------
+ if (some expression) {
// do something
}
-----------------------------
instead of
-----------------------------
+ boolean isScrollPanePresent = true;
+ if (some expression) {
+ isScrollPanePresent = false;
+ }
+ if (isScrollPanePresent) {
// do something
}
-----------------------------
In your case it is even better just to update the rMax
according to is scroll pane presence.
Thanks,
Alexandr.
Regards
Prasanta
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
