The fix looks good to me.
Thanks,
Alexandr.
On 10/14/2015 2:26 PM, prasanta sadhukhan wrote:
On 10/9/2015 3:00 PM, Alexander Scherbatiy wrote:
On 10/7/2015 3:10 PM, prasanta sadhukhan wrote:
Hi All,
@Sergey,Phil,Alexander Z
http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.10/
Have you checked the corner cases like printing a table which has
only one row (with and without a scroll pane)?
Could you add these scenarios to the test?
Yes, I have checked this cases. JTable with 1 row will not show a
scrollpane so it will be same as a table with 1 row without a scrollpane.
I have added one row without scrollpane subtest
http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.11/
Regards
Prasanta
Thanks,
Alexandr,
Any more comments/feedback? Can I get a +1 for this?
Regards
Prasanta
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