I once more optimized the Swing RepaintManager to limit the amount of
painting work that is carried out by Swing. While observing painting
behaviour I noticed that sometimes (especially with overlapping
components) the painting is carried out in a 2-step manner, which
results in strange artifacts visible for a short time. This was caused
by an unfortunate ordering of the paint jobs. I adjusted this class in
the following way:

- Removed the use of working copies for invalidComponents and
dirtyComponents. Before, when the RepaintWorker job was dequeued from
the EventQueue, it swapped over the working copies with the real one,
itself working on the copies and allowing other threads to queue new
work requests in the empty working copies (which were then processed by
the next RepaintWorker). This is now implemented so that all threads
access the real structures and fine tuned synchronization.
- The ordering of the paint jobs is now done based on the sizes of the
damaged regions. Before it was ordered by the depth of the component in
the component hierarchy, which is similar but has different results when
rendering multiple-layered containers (see above -> 'two step
rendering'). Bigger damaged regions are painted first. This makes it
quite likely that all the smaller regions are painted with this request,
thus increasing overall performance. Imagine the smaller regions would
be painted first, and then the bigger regions that enclose the smaller
ones, this would result in the small regions painted twice or more often.
- The RepaintWorker has been marked 'dead' when it begins its work.
This had the bad effect that there could be more than one RepaintWorker
active on the EventQueue. This is not desired, I moved the
setLive(false) call to after the RepaintWorker finished it's job and
wrapped it up in a try{} finally{} clause to make sure it cleans up when
something in between fails.

2006-02-02  Roman Kennke  <[EMAIL PROTECTED]>

        * javax/swing/RepaintManager.java
        Made fields private.
        (RepaintWorker.run): Enclosed work stuff in try finally block in
        order to clean up correctly if invalidation or painting fails,
        otherwise we would get no more RepaintWorkers onto the EventQueue.
        Also, now the RepaintWorker is marked 'dead' only after it has
        finished its work, avoid more than one RepaintWorker on the queue.
        (ComponentComparator.compareTo): Compare dirty rectangle sizes
        instead of hierarchy depths.
        (workDirtyComponents): Removed unused field.
        (repaintOrder): Removed unused field.
        (workRepaintOrder): Removed unused field.
        (workInvalidComponents): Removed unused field.
        (RepaintManager()): Removed initialization of removed fields.
        (addInvalidComponent): Fine tuned synchronization.
        (removeInvalidComponent): Fine tune synchronization.
        (addDirtyRegion): Short circuit invalid dirty regions. Fine tuned
        synchronization. Don't manager repaintOrder here.
        (insertRepaintOrder): Removed method.
        (markCompletelyClean): Fine tuned synchronization.
        (validateInvalidComponents): Dont use a working copy of the
        invalidComponents list, instead fine tuned synchronization on this
        list. Also, don't search validateRoot, this is already done in
        addInvalidComponent().
        (paintDirtyRegions): Compute repaint order here, based on size of
        damaged regions. Fine tuned synchronization. Avoid use of working
        copies of dirtyComponent.

/Roman
Index: javax/swing/RepaintManager.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/RepaintManager.java,v
retrieving revision 1.22
diff -u -r1.22 RepaintManager.java
--- javax/swing/RepaintManager.java	27 Jan 2006 10:10:00 -0000	1.22
+++ javax/swing/RepaintManager.java	2 Feb 2006 21:13:26 -0000
@@ -62,6 +62,7 @@
  * href="http://java.sun.com/products/jfc/tsc/articles/painting/index.html";>this
  * document</a> for more details.</p>
  *
+ * @author Roman Kennke ([EMAIL PROTECTED])
  * @author Graydon Hoare ([EMAIL PROTECTED])
  */
 public class RepaintManager
@@ -107,12 +108,18 @@
 
     public void run()
     {
-      ThreadGroup threadGroup = Thread.currentThread().getThreadGroup();
-      RepaintManager rm =
-        (RepaintManager) currentRepaintManagers.get(threadGroup);
-      setLive(false);
-      rm.validateInvalidComponents();
-      rm.paintDirtyRegions();
+      try
+        {
+          ThreadGroup threadGroup = Thread.currentThread().getThreadGroup();
+          RepaintManager rm =
+            (RepaintManager) currentRepaintManagers.get(threadGroup);
+          rm.validateInvalidComponents();
+          rm.paintDirtyRegions();
+        }
+      finally
+        {
+          setLive(false);
+        }
     }
 
   }
@@ -135,41 +142,23 @@
      * @param o1 the first component
      * @param o2 the second component
      *
-     * @return a negative integer, if <code>o1</code> is higher in the
-     *         hierarchy than <code>o2</code>, zero, if both are at the same
-     *         level and a positive integer, if <code>o1</code> is deeper in
-     *         the hierarchy than <code>o2</code> 
+     * @return a negative integer, if <code>o1</code> is bigger in than
+     *         <code>o2</code>, zero, if both are at the same size and a
+     *         positive integer, if <code>o1</code> is smaller than
+     *         <code>o2</code> 
      */
     public int compare(Object o1, Object o2)
     {
       if (o1 instanceof JComponent && o2 instanceof JComponent)
         {
           JComponent c1 = (JComponent) o1;
+          Rectangle d1 = (Rectangle) dirtyComponents.get(c1);
           JComponent c2 = (JComponent) o2;
-          return getDepth(c1) - getDepth(c2);
+          Rectangle d2 = (Rectangle) dirtyComponents.get(c2);
+          return d2.width * d2.height - d1.width * d1.height;
         }
-      else
-        throw new ClassCastException("This comparator can only be used with "
-                                     + "JComponents");
-    }
-
-    /**
-     * Computes the depth for a given JComponent.
-     *
-     * @param c the component to compute the depth for
-     *
-     * @return the depth of the component
-     */
-    private int getDepth(JComponent c)
-    {
-      Component comp = c;
-      int depth = 0;
-      while (comp != null)
-        {
-          comp = comp.getParent();
-          depth++;
-        }
-      return depth;
+      throw new ClassCastException("This comparator can only be used with "
+                                   + "JComponents");
     }
   }
 
@@ -179,6 +168,9 @@
    * to exactly one rectangle.  When more regions are marked as dirty on a
    * component, they are union'ed with the existing rectangle.
    *
+   * This is package private to avoid a synthetic accessor method in inner
+   * class.
+   *
    * @see #addDirtyRegion
    * @see #getDirtyRegion
    * @see #isCompletelyDirty
@@ -187,18 +179,10 @@
    */
   HashMap dirtyComponents;
 
-  HashMap workDirtyComponents;
-
-  /**
-   * Stores the order in which the components get repainted.
-   */
-  ArrayList repaintOrder;
-  ArrayList workRepaintOrder;
-
   /**
    * The comparator used for ordered inserting into the repaintOrder list. 
    */
-  Comparator comparator;
+  private transient Comparator comparator;
 
   /**
    * A single, shared instance of the helper class. Any methods which mark
@@ -209,7 +193,7 @@
    * @see #addDirtyRegion
    * @see #addInvalidComponent
    */
-  RepaintWorker repaintWorker;
+  private RepaintWorker repaintWorker;
 
   /** 
    * The set of components which need revalidation, in the "layout" sense.
@@ -221,8 +205,7 @@
    * @see #removeInvalidComponent
    * @see #validateInvalidComponents
    */
-  ArrayList invalidComponents;
-  ArrayList workInvalidComponents;
+  private ArrayList invalidComponents;
 
   /** 
    * Whether or not double buffering is enabled on this repaint
@@ -232,7 +215,7 @@
    * @see #isDoubleBufferingEnabled
    * @see #setDoubleBufferingEnabled
    */
-  boolean doubleBufferingEnabled;
+  private boolean doubleBufferingEnabled;
 
   /** 
    * The current offscreen buffer. This is reused for all requests for
@@ -242,7 +225,7 @@
    * @see #getOffscreenBuffer
    * @see #doubleBufferMaximumSize
    */
-  Image doubleBuffer;
+  private Image doubleBuffer;
 
   /**
    * The maximum width and height to allocate as a double buffer. Requests
@@ -252,7 +235,7 @@
    * @see #getDoubleBufferMaximumSize
    * @see #setDoubleBufferMaximumSize
    */
-  Dimension doubleBufferMaximumSize;
+  private Dimension doubleBufferMaximumSize;
 
 
   /**
@@ -261,11 +244,7 @@
   public RepaintManager()
   {
     dirtyComponents = new HashMap();
-    workDirtyComponents = new HashMap();
-    repaintOrder = new ArrayList();
-    workRepaintOrder = new ArrayList();
     invalidComponents = new ArrayList();
-    workInvalidComponents = new ArrayList();
     repaintWorker = new RepaintWorker();
     doubleBufferMaximumSize = new Dimension(2000,2000);
     doubleBufferingEnabled = true;
@@ -346,7 +325,7 @@
    *
    * @see #removeInvalidComponent
    */
-  public synchronized void addInvalidComponent(JComponent component)
+  public void addInvalidComponent(JComponent component)
   {
     Component ancestor = component.getParent();
 
@@ -363,8 +342,11 @@
     if (invalidComponents.contains(component))
       return;
 
-    invalidComponents.add(component);
-    
+    synchronized (invalidComponents)
+      {
+        invalidComponents.add(component);
+      }
+
     if (! repaintWorker.isLive())
       {
         repaintWorker.setLive(true);
@@ -379,9 +361,12 @@
    *
    * @see #addInvalidComponent
    */
-  public synchronized void removeInvalidComponent(JComponent component)
+  public void removeInvalidComponent(JComponent component)
   {
-    invalidComponents.remove(component);
+    synchronized (invalidComponents)
+      {
+        invalidComponents.remove(component);
+      }
   }
 
   /**
@@ -402,17 +387,21 @@
    * @see #markCompletelyClean
    * @see #markCompletelyDirty
    */
-  public synchronized void addDirtyRegion(JComponent component, int x, int y,
-                                          int w, int h)
+  public void addDirtyRegion(JComponent component, int x, int y,
+                             int w, int h)
   {
-    if (w == 0 || h == 0 || !component.isShowing())
+    if (w <= 0 || h <= 0 || !component.isShowing())
       return;
+
     Rectangle r = new Rectangle(x, y, w, h);
     if (dirtyComponents.containsKey(component))
-      r = r.union((Rectangle)dirtyComponents.get(component));
-    else
-      insertInRepaintOrder(component);
-    dirtyComponents.put(component, r);
+      r = r.union((Rectangle) dirtyComponents.get(component));
+
+    synchronized (dirtyComponents)
+      {
+        dirtyComponents.put(component, r);
+      }
+
     if (! repaintWorker.isLive())
       {
         repaintWorker.setLive(true);
@@ -421,22 +410,6 @@
   }
 
   /**
-   * Inserts a component into the repaintOrder list in an ordered fashion,
-   * using a binary search.
-   *
-   * @param c the component to be inserted
-   */
-  private void insertInRepaintOrder(JComponent c)
-  {
-    if (comparator == null)
-      comparator = new ComponentComparator();
-    int insertIndex = Collections.binarySearch(repaintOrder, c, comparator);
-    if (insertIndex < 0)
-      insertIndex = -(insertIndex + 1);
-    repaintOrder.add(insertIndex, c);
-  }
-
-  /**
    * Get the dirty region associated with a component, or <code>null</code>
    * if the component has no dirty region.
    *
@@ -489,7 +462,7 @@
    */
   public void markCompletelyClean(JComponent component)
   {
-    synchronized (this)
+    synchronized (dirtyComponents)
       {
         dirtyComponents.remove(component);
       }
@@ -523,63 +496,55 @@
    */
   public void validateInvalidComponents()
   {
-    // In order to keep the blocking of application threads minimal, we switch
-    // the invalidComponents field with the workInvalidComponents field and
-    // work with the workInvalidComponents field.
-    synchronized(this)
-    {
-      ArrayList swap = invalidComponents;
-      invalidComponents = workInvalidComponents;
-      workInvalidComponents = swap;
-    }
-    for (Iterator i = workInvalidComponents.iterator(); i.hasNext(); )
+    // We don't use an iterator here because that would fail when there are
+    // components invalidated during the validation of others, which happens
+    // quite frequently. Instead we synchronize the access a little more.
+    while (invalidComponents.size() > 0)
       {
-        Component comp = (Component) i.next();
-        // Find validate root.
-        while ((!(comp instanceof JComponent)
-               || !((JComponent) comp).isValidateRoot())
-               && comp.getParent() != null)
-          comp = comp.getParent();
-
-        // Validate the validate root.
+        Component comp;
+        synchronized (invalidComponents)
+          {
+            comp = (Component) invalidComponents.remove(0);
+          }
+        // Validate the validate component.
         if (! (comp.isVisible() && comp.isShowing()))
           continue;
         comp.validate();
       }
-    workInvalidComponents.clear();
   }
 
   /**
    * Repaint all regions of all components which have been marked dirty in
    * the [EMAIL PROTECTED] #dirtyComponents} table.
    */
-  public synchronized void paintDirtyRegions()
+  public void paintDirtyRegions()
   {
-    // In order to keep the blocking of application threads minimal, we switch
-    // the dirtyComponents field with the workdirtyComponents field and the
-    // repaintOrder field with the workRepaintOrder field and work with the
-    // work* fields.
-    synchronized(this)
-    {
-      ArrayList swap = workRepaintOrder;
-      workRepaintOrder = repaintOrder;
-      repaintOrder = swap;
-      HashMap swap2 = workDirtyComponents;
-      workDirtyComponents = dirtyComponents;
-      dirtyComponents = swap2;
-    }
-    for (Iterator i = workRepaintOrder.iterator(); i.hasNext();)
+    // Short cicuit if there is nothing to paint.
+    if (dirtyComponents.size() == 0)
+      return;
+
+    synchronized (dirtyComponents)
       {
-        JComponent comp = (JComponent) i.next();
-        // If a component is marked completely clean in the meantime, then skip
-        // it.
-        Rectangle damaged = (Rectangle) workDirtyComponents.get(comp);
-        if (damaged == null || damaged.isEmpty())
-          continue;
-        comp.paintImmediately(damaged);
+        // We sort the components by their size here. This way we have a good
+        // chance that painting the bigger components also paints the smaller
+        // components and we don't need to paint them twice.
+        ArrayList repaintOrder = new ArrayList(dirtyComponents.size());
+        repaintOrder.addAll(dirtyComponents.keySet());
+        if (comparator == null)
+          comparator = new ComponentComparator();
+        Collections.sort(repaintOrder, comparator);
+        for (Iterator i = repaintOrder.iterator(); i.hasNext();)
+          {
+            JComponent comp = (JComponent) i.next();
+            // If a component is marked completely clean in the meantime, then skip
+            // it.
+            Rectangle damaged = (Rectangle) dirtyComponents.get(comp);
+            if (damaged == null || damaged.isEmpty())
+              continue;
+            comp.paintImmediately(damaged);
+            dirtyComponents.remove(comp);
+          }
       }
-    workRepaintOrder.clear();
-    workDirtyComponents.clear();
   }
 
   /**

Reply via email to