[icedtea-web] RFC: Patch to double-buffer applets

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2010-11-04 15:30]:
> On 11/04/2010 02:14 PM, Deepak Bhole wrote:
> >* Omair Majid<omajid at redhat.com>  [2010-11-03 09:32]:
> >>Hi Deepak,
> >>
> >>On 11/02/2010 05:37 PM, Deepak Bhole wrote:
> >>>* Deepak Bhole<dbhole at redhat.com>   [2010-11-02 17:31]:
> >>>>Hi,
> >>>>
> >>>>IcedTea plugin currently does not double-buffer the applet panel. As a
> >>>>result, some applets may flicker depending on the machine.
> >>>>
> >>>>Attached patch buffers the frame offscreen and then draws it, thereby
> >>>>addressing the issue.
> >>>>
> >>
> >>Is there a specific test case that this fixes? The only flickering
> >>applet that I know of is at
> >>http://download.oracle.com/javase/1.4.2/docs/guide/misc/applet.html,
> >>and I dont see any change in that applet with the patch applied.
> >>
> >>>>ChangeLog:
> >>>>2010-11-02  Deepak Bhole<dbhole at redhat.com>
> >>>>
> >>>>    * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java (update):
> >>>>    Override method and implement double-buffering.
> >>>>
> >>>>Cheers,
> >>>>Deepak
> >>>
> >>>
> >>>Doh! There shouldn't be a println in there. New patch attached. I also
> >>>fixed the variable names for the image and its graphics.
> >>>
> >>>
> >>>>diff -r 6c2527d42900 ChangeLog
> >>>>--- a/ChangeLog	Mon Nov 01 11:44:15 2010 -0400
> >>>>+++ b/ChangeLog	Tue Nov 02 17:29:03 2010 -0700
> >>>>@@ -1,3 +1,8 @@
> >>>>+2010-11-02  Deepak Bhole<dbhole at redhat.com>
> >>>>+
> >>>>+	* plugin/icedteanp/java/sun/applet/PluginAppletViewer.java (update):
> >>>>+	Override method and implement double-buffering.
> >>>>+
> >>>>  2010-11-01  Omair Majid<omajid at redhat.com>
> >>>>
> >>>>  	* Makefile.am (clean-IcedTeaPlugin): Only delete launcher directory if it
> >>>>diff -r 6c2527d42900 NEWS
> >>>>--- a/NEWS	Mon Nov 01 11:44:15 2010 -0400
> >>>>+++ b/NEWS	Tue Nov 02 17:29:03 2010 -0700
> >>>>@@ -12,3 +12,4 @@
> >>>>
> >>>>  * Initial release of IcedTea-Web
> >>>>  * PR542: Plugin fails with NPE on http://www.openprocessing.org/visuals/iframe.php?visualID=2615
> >>>>+* Applets are now double-buffered to eliminate flicker in ones that do heavy drawing
> >>>>diff -r 6c2527d42900 plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> >>>>--- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Mon Nov 01 11:44:15 2010 -0400
> >>>>+++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Tue Nov 02 17:29:03 2010 -0700
> >>>>@@ -359,6 +366,9 @@
> >>>>
> >>>>       private static Long requestIdentityCounter = 0L;
> >>>>
> >>>>+     private Image bufferedPanelImage;
> >>>>+     private Graphics bufferedImageGraphics;
> >>>>+
> >>>>       /**
> >>>>        * Null constructor to allow instantiation via newInstance()
> >>>>        */
> >>>>@@ -2163,4 +2178,24 @@
> >>>>          }
> >>>>      }
> >>>>       }
> >>>>+
> >>>>+     /**
> >>>>+      * {@inheritDoc}
> >>>>+      */
> >>>>+     public void update(Graphics g) {
> >>>>+
> >>>>+    	 System.err.println("Buffering and drawing");
> >>>>+    	
> >>>>+    	 // If the image or the graphics don't exist, create new ones
> >>>>+    	 if (bufferedPanelImage == null || bufferedImageGraphics == null) {
> >>>>+    		 bufferedPanelImage = createImage(getWidth(), getHeight());
> >>>>+    		 bufferedImageGraphics = bufferedPanelImage.getGraphics ();
> >>>>+    	 }
> >>>>+
> >>>>+    	 // Paint off-screen
> >>>>+    	 paint(bufferedImageGraphics);
> >>
> >>The overridden method is update, but paint is called here instead.
> >>Is there a reason for that? As far as I know, the semantics of
> >>update() are slightly different from paint() - update() clears the
> >>background and then calls paint(). Perhaps super.update() might be
> >>more appropriate?
> >>
> >
> >super.update() would just call paint on the Graphics instance passed to
> >it. Container's (super in this case) update() methods do a couple of
> >additional checks, but we don't need them for applets, which are known
> >lightweight containers.
> >
> >I can definitely add the call if you feel it would be cleaner though --
> >just that it doesn't make a difference in this case.
> >
> 
> Thanks for the explanation. Its makes more sense now. Please go
> ahead and commit the patch you posted. If you could add this
> explanation as a comment to the patch too, that would be great.
> 

Added and pushed:
http://icedtea.classpath.org/hg/icedtea-web/rev/8e66d9386273

Thanks for reviewing!

Cheers,
Deepak

> Cheers,
> Omair