Bug 28027 - Wrong clip region in Swing repaints - overwrites other components
Summary: Wrong clip region in Swing repaints - overwrites other components
Status: RESOLVED FIXED
Alias: None
Product: classpath
Classification: Unclassified
Component: swing (show other bugs)
Version: 0.92
: P3 normal
Target Milestone: 0.92
Assignee: Roman Kennke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-06-14 09:14 UTC by Norman Hendrich
Modified: 2006-06-16 13:54 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-06-15 13:00:20


Attachments
AVI video showing the clock-generator demo (a little slow, but correct) (846.68 KB, application/octet-stream)
2006-06-14 09:17 UTC, Norman Hendrich
Details
AVI video demonstrating the bug (491.18 KB, application/octet-stream)
2006-06-14 09:25 UTC, Norman Hendrich
Details
testcase to demonstrate the bug. (1.53 KB, text/plain)
2006-06-15 12:40 UTC, Norman Hendrich
Details
testcase with menu and periodic animation repaints (1.85 KB, text/plain)
2006-06-16 08:45 UTC, Norman Hendrich
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Norman Hendrich 2006-06-14 09:14:22 UTC
This is my follow-up bug report to swing/27833, regarding clip region issues
observed with two of my test applications (jfig and Hades, both using the same
FigSwingCanvas component for their main display).

So far I have failed to come up with a simple testcase, but I do have additional
information:

How to reproduce the bug:

1. Download  
   http://tams-www.informatik.uni-hamburg.de/applets/hades/archive/hades.jar
2. Run jamvm -jar hades.jar
3. Select menu > help > demos > clockgen
4. Wait two seconds until the demo begins

5a. Try to open a menu
6a. I personally like menu > layers the best: somehow two different parts of the 
    menu are redrawn when the LED connected to the upper clock generator is 
    updated :-)

5b. Use a mouse-click to ensure that the main canvas has the keyboard focus
6b. Type the 'z' key to initiate a zoom-out operation.
7b. Observe that the application now alternates between the 'old' zoom factor
    and the new zoom factor.

I attach three low-quality videos taken with my digicam; please excuse the
interlacing artifacts. The first video shows the Hades simulation of the 
clock-generator demo without zoom operation or user interaction. This seems 
to work.

In the second video, I initiate a zoom-out operation. My app now only keeps
the zoom-out schematics, but this somehow gets overdrawn periodically by 
a backbuffer that still has the original zoom-factor.

The third video shows the situation after another zoom-out operation. The
schematics should become even smaller (and it does), but Swing somehow still
keeps the original image...



As my app does all drawing via its own backbuffer, which now has the new
zoom factor (otherwise the schematics would never be drawn with the new zoom),
this implies that SWING SOMEHOW KEEPS ITS OWN BACKBUFFER AND WRONGLY OVERWRITES
MY FigSwingCanvas.

A very similar thing happens in jfig, where panning on the main canvas works,
while zooming does not. The only difference is that panning is handled internally by FigSwingCanvas, while zooming also later does a setText() call
on the JTextField used to display the new zoom factor. The repaint generated 
by the setText() call then immediately overwrites the correct new display
with some backbuffer contents managed by Swing. To prove this, I temporarily
removed the setText() call and jfig behaved as it should after zoom operations.

So: some redraw operations in free Swing use broken (too large) clip regions
and overwrite components with the Swing-manager back-buffer. This should never
happen as my component claims isDoubleBuffered=false and isOpaque=true.

Any ideas on how to further debug this are highly appreciated!
Comment 1 Norman Hendrich 2006-06-14 09:17:02 UTC
Created attachment 11665 [details]
AVI video showing the clock-generator demo (a little slow, but correct)
Comment 2 Norman Hendrich 2006-06-14 09:25:13 UTC
Created attachment 11666 [details]
AVI video demonstrating the bug

This video shows the clock-generator animation after two zoom-out operations.
(Bugzilla wouldn't let me upload another video because its size is slightly
over 1MB. arrgghh.)

After the zoom-out, the animation should and does continue but with
the schematics zoomed-out. Instead, the new (zoom-out) schematics is 
periodically overwritten by a schematics in the original size.
I have no idea where Swing might keep a backbuffer or why the repaint of the 
time-panel should overwrite my main canvas.
Comment 3 Norman Hendrich 2006-06-15 12:40:26 UTC
Created attachment 11674 [details]
testcase to demonstrate the bug.

Just compile and run the testcase, <java> RubberbandTest,
then move the mouse around or click the left button to set a new base
point for the rubberband.

On my system, moving the mouse results in horrible flickering, because
the repainted canvas (with rubberband) gets immediately overwritten 
as a result of the setText() call. If you remove the setPositionTextField()
call, no extra text repaints are generated, and the app behaves as it 
should.
Comment 4 Roman Kennke 2006-06-15 13:00:20 UTC
I also see this flickering in the testcase. I'll fix this.
Comment 5 Roman Kennke 2006-06-15 13:16:06 UTC
The problem here is that double buffering is disabled for the RubberbandTest component, so the call to paintImmediately() ends up painting on the screen directly. Seems this interacts badly on GNU Classpath. Dunno why/how it works on Sun, but I guess I have to figure it out.
Comment 6 Roman Kennke 2006-06-15 13:27:34 UTC
This stacktrace (Sun's impl) looks like Sun does double buffering anyway, even though the RubberbandTest component is marked as beeing not double buffered. The only explanation that I can make up of this and which makes sense is that a component can only be not-doublebuffered when all of it's parents are not doublebuffered too. Otherwise the backbuffer and screen would not come into sync and we see that flickering that we see.

I'll fix that in this way.
Comment 7 Roman Kennke 2006-06-15 13:31:52 UTC
Here is that stacktrace that I meant:

java.lang.Exception: Stack trace
        at java.lang.Thread.dumpStack(Thread.java:1158)
        at RubberbandTest.paint(RubberbandTest.java:110)
        at javax.swing.JComponent.paintWithOffscreenBuffer(JComponent.java:4963)        at javax.swing.JComponent.paintDoubleBuffered(JComponent.java:4916)
        at javax.swing.JComponent._paintImmediately(JComponent.java:4859)
        at javax.swing.JComponent.paintImmediately(JComponent.java:4666)
        at RubberbandTest.mouseMoved(RubberbandTest.java:156)
Comment 8 cvs-commit@developer.classpath.org 2006-06-15 13:33:26 UTC
Subject: Bug 28027

CVSROOT:	/cvsroot/classpath
Module name:	classpath
Changes by:	Roman Kennke <rabbit78>	06/06/15 13:32:48

Modified files:
	javax/swing    : JComponent.java 
	.              : ChangeLog 

Log message:
	2006-06-15  Roman Kennke  <kennke@aicas.com>
	
		PR 28027
		* javax/swing/JComponent.java
		(paintImmediately2): Only paint component without double buffering
		when all of it's parents have also double buffering disabled.
		(isPaintingDoubleBuffered): New helper method.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/classpath/javax/swing/JComponent.java?cvsroot=classpath&r1=1.127&r2=1.128
http://cvs.savannah.gnu.org/viewcvs/classpath/ChangeLog?cvsroot=classpath&r1=1.7821&r2=1.7822

Patches:

Index: ChangeLog
===================================================================
RCS file: /cvsroot/classpath/classpath/ChangeLog,v
retrieving revision 1.7821
retrieving revision 1.7822
diff -u -b -r1.7821 -r1.7822
--- ChangeLog	15 Jun 2006 08:42:52 -0000	1.7821
+++ ChangeLog	15 Jun 2006 13:32:46 -0000	1.7822
@@ -1,3 +1,11 @@
+2006-06-15  Roman Kennke  <kennke@aicas.com>
+
+	PR 28027
+	* javax/swing/JComponent.java
+	(paintImmediately2): Only paint component without double buffering
+	when all of it's parents have also double buffering disabled.
+	(isPaintingDoubleBuffered): New helper method.
+
 2006-06-15  David Gilbert  <david.gilbert@object-refinery.com>
 
 	* gnu/java/awt/peer/gtk/CairoGraphics2D.java



Comment 9 Roman Kennke 2006-06-15 13:43:37 UTC
Fixed by only disable double buffering for a JComponent, when all of it's ancestors have double buffering disabled (or aren't JComponents).
Comment 10 Norman Hendrich 2006-06-15 13:45:11 UTC
Subject: Re:  Wrong clip region in Swing repaints - overwrites other components

> The problem here is that double buffering is disabled for the RubberbandTest
> component, so the call to paintImmediately() ends up painting on the screen
> directly. 

Well, drawing the rubberband directly is the whole idea of this component
(which has worked fine on Sun since JDK 1.1)... 

My circuit simulator also uses direct painting during animation. It is
usually much faster to bitblt a small area of (static) backbuffer and
then to draw animated objects on top of this, instead of always updating
the backbuffer (all objects) and then blitting the buffer.


> The only explanation that I can make up of this and which makes sense is that
> a component can only be not-doublebuffered when all of it's parents are not
> doublebuffered too. Otherwise the backbuffer and screen would not come into
> sync and we see that flickering that we see.

Is there some documentation about the current repaint strategy in Swing?
The source code for RepaintManager and the GTK stuff is pretty involved,
and not too helpful in this regard...

I always thought that the isDoubleBuffered() method was respected by 
Swing, but I never cared to check what Sun does internally. 

1. JComponent ------------------------> Swing Backbuffer -> screen    
2. JComponent -> internal backbuffer -> Swing backbuffer -> screen
3. JComponent -> internal backbuffer ---------------------> screen

My understanding was that "normal" components use strategy (1). 
My component definitely needs to manage its own backbuffer, and I thought
that with isDoubleBuffered=false I would get (3) and avoid the extra
useless copying involved with isDoubleBuffered=true and (2).





Comment 11 Roman Kennke 2006-06-15 13:49:41 UTC
I only can tell you what I assume from the mentioned stacktrace. You see that paintImmediately() calls paintDoubleBuffered() which means to me, that the actual painting takes place on the double buffer anyway. So I think you have implemented case#2, which is the least efficient. If you wanna be sure, maybe look at Sun's sources (I am not allowed to).

        at RubberbandTest.paint(RubberbandTest.java:110)
        at
javax.swing.JComponent.paintWithOffscreenBuffer(JComponent.java:4963)        at
javax.swing.JComponent.paintDoubleBuffered(JComponent.java:4916)
        at javax.swing.JComponent._paintImmediately(JComponent.java:4859)
        at javax.swing.JComponent.paintImmediately(JComponent.java:4666)

Comment 12 Norman Hendrich 2006-06-15 14:10:03 UTC
Subject: Re:  Wrong clip region in Swing repaints - overwrites other components

 
> at RubberbandTest.paint(RubberbandTest.java:110)
> at javax.swing.JComponent.paintWithOffscreenBuffer(JComponent.java:4963)        
> at javax.swing.JComponent.paintDoubleBuffered(JComponent.java:4916)
> at javax.swing.JComponent._paintImmediately(JComponent.java:4859)
> at javax.swing.JComponent.paintImmediately(JComponent.java:4666)

> So I think you have
> implemented case#2, which is the least efficient. 


looking at that stacktrace, you may have a point :-)   

Or rather, Sun decided that an extra buffering was the way to go. 

However, rubberbanding is quite smooth on the JDK, and jfig can open and
redraw typical files at over 20 "frames per second". That is fast enough 
for most cases.

I still don't see how classpath ends up overwriting my component, but
I haven't tested your latest patches yet.

Comment 13 Roman Kennke 2006-06-15 14:22:23 UTC
The problem has been that Classpath indeed respected the isDoubleBuffered() flag, and painted the component directly on the screen, but not to the Swing double buffer. Now, when the RepaintManager decides that the GUI has to be repainted, it blits the Swing double buffer on the screen, thus overwriting what your component has drawn on it before. Most likely this is why Sun 'decided to use an extra buffering' even when a component is marked as not beeing double buffered, if any parent of that component is double buffered, the buffer and the screen get out of sync and you see the flickering that you've seen.
Comment 14 Norman Hendrich 2006-06-15 14:38:14 UTC
Subject: Re:  Wrong clip region in Swing repaints - overwrites other components

> The problem has been that Classpath indeed respected the isDoubleBuffered()
> flag, and painted the component directly on the screen, but not to the Swing
> double buffer. Now, when the RepaintManager decides that the GUI has to be
> repainted, it blits the Swing double buffer on the screen, thus overwriting
> what your component has drawn on it before.

OK, so RepaintManager is broken after all - it should not use its own
backbuffer for those components that claim isDoubleBuffered=false...

... but I guess we might try to silently enforce double-buffering for
all components, and see whether and how (fast) this works.


Comment 15 Roman Kennke 2006-06-15 15:07:33 UTC
Well, looking at the specs this behaviour isn't actually wrong. It only says something about what happens (or rather should happen, so it's really only a hint) when this flag returns true. It doesn't say that when it's false that it doesn't paint to a buffer.

See API docs for JComponent.setDoubleBuffered(), JComponent.isDoubleBuffered and Component.isDoubleBuffered().
Comment 16 Norman Hendrich 2006-06-15 16:24:26 UTC
Subject: Re:  Wrong clip region in Swing repaints - overwrites other components

Hello Roman,

thanks for working on the bug so quickly. My testcase does indeed work now
(if slowly), but I don't consider the bug FIXED. Please reopen.

Please run my original testcase to verify that the menus-overwritten issue
is still there. I am pretty certain there is another clipRect buglet in
RepaintManager or our popups.

Comment 17 Roman Kennke 2006-06-15 18:56:04 UTC
Whoops. I didn't even look at the original bug. Bad me ;-)
Comment 18 Roman Kennke 2006-06-15 19:05:14 UTC
Somehow I can't download hades.jar, could you fix the Link?
Comment 19 Norman Hendrich 2006-06-16 07:58:43 UTC
(In reply to comment #18)
> Somehow I can't download hades.jar, could you fix the Link?

Works for me; tried again right now. What exactly is your download problem?

But I have an idea on how to demonstrate the problem with yesterday's testcase,
coming soon...
Comment 20 Roman Kennke 2006-06-16 08:33:54 UTC
You are really beating our Swing painting to perfectness ;-)

Seriously, I see the issue. I think we have something wrong here with determining overlapping components. I'll look at this now.

BTW, the testcase slipped into the wrong PR.
Comment 21 Norman Hendrich 2006-06-16 08:45:04 UTC
Created attachment 11677 [details]
testcase with menu and periodic animation repaints
Comment 22 Norman Hendrich 2006-06-16 08:56:56 UTC
(In reply to comment #20)
> You are really beating our Swing painting to perfectness ;-)

I hope so!  :-)


During the last few days, classpath swing has gotten to the point where testing
reveals as many bugs in my apps as bugs in classpath. So, having an alternative 
implementation to the JDK pays off now, besides all the obvious advantages of 
freedom.

Comment 23 Roman Kennke 2006-06-16 10:30:01 UTC
I fixed a clipping bug that solves the last testcase. Could you please retest and close when appropriate?
Comment 24 cvs-commit@developer.classpath.org 2006-06-16 10:52:47 UTC
Subject: Bug 28027

CVSROOT:	/cvsroot/classpath
Module name:	classpath
Changes by:	Roman Kennke <rabbit78>	06/06/16 10:27:31

Modified files:
	gnu/java/awt/peer/gtk: CairoGraphics2D.java 
	.              : ChangeLog 

Log message:
	2006-06-16  Roman Kennke  <kennke@aicas.com>
	
		PR 28027
		* gnu/java/awt/peer/gtk/CairoGraphics2D.java
		(drawImage): Don't use setClip() but instead clipRect() to
		intersect the current clip with a new one.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/java/awt/peer/gtk/CairoGraphics2D.java?cvsroot=classpath&r1=1.24&r2=1.25
http://cvs.savannah.gnu.org/viewcvs/classpath/ChangeLog?cvsroot=classpath&r1=1.7837&r2=1.7838

Patches:

Index: ChangeLog
===================================================================
RCS file: /cvsroot/classpath/classpath/ChangeLog,v
retrieving revision 1.7837
retrieving revision 1.7838
diff -u -b -r1.7837 -r1.7838
--- ChangeLog	15 Jun 2006 23:08:48 -0000	1.7837
+++ ChangeLog	16 Jun 2006 10:27:30 -0000	1.7838
@@ -1,3 +1,10 @@
+2006-06-16  Roman Kennke  <kennke@aicas.com>
+
+	PR 28027
+	* gnu/java/awt/peer/gtk/CairoGraphics2D.java
+	(drawImage): Don't use setClip() but instead clipRect() to
+	intersect the current clip with a new one.
+
 2006-06-15  Thomas Fitzsimmons  <fitzsim@redhat.com>
 
 	* configure.ac: Rename appletviewer to gappletviewer, jarsigner to



Comment 25 Norman Hendrich 2006-06-16 13:54:22 UTC
(In reply to comment #23)
> I fixed a clipping bug that solves the last testcase. Could you please retest
> and close when appropriate?

Great!

I retested with jfig and Hades, and all clipping issues seem fixed, including
nested drop-down menus and nested popup-menus. 

Rubberbanding is fast enough for simple drawings in jfig+jamvm (but most of 
the Hades animations are a little too slow to be useful.)

I will close the bug now.