Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-4665233

swing RepaintManager is not optimal for some JTable update scenarios

XMLWordPrintable

    • generic, x86
    • generic, windows_2000

      JTables are generally a grid of non-overlapping uniformly sized rectangles.
      When any one cell is updated a repaint event for that cell is generated.
      WHen two cells are updated a repaint is generated for a rectangle containing
      the two cells, which may cause many intervening cells to be repaint.ed
      When the 2nd cell that is not visible (ie not in the viewport of the embedding
      JScrollPane) is updated, then the rectangle extends all the way to the
      edge of the viewport in the direction in which the invisible cell is to
      be found.

      Thus two individual cell repaints, one of a visible one in the top row
      and one of an invisible cell somewhere off the bottom of the viewport
      can cause the whole JTable to be repainted.

      This will often invoke calls to application-supplied cell renderers
      which are not always written with rendering performance as priority,
      thus degrading user experience.

      The RepaintManager for JTable should ignore invisible cells and
      should probably not union the rectangles until some level of
      coverage of that rectangle (30%?) actually requires repainting.
      Contribution by java.net member leouser:

      A DESCRIPTION OF THE FIX :
      BUGID: 4665233 swing.RepaintManager is not optimal for some JTable update scenarios
      FILES AFFECTED: javax.swing.RepaintManager.
      jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin

      DISCUSION(embeded in test case as well):
      /**
       * BUGID: 4665233 swing.RepaintManager is not optimal for some JTable update scenarios
       * also may point the way to: 4318634 Request improved dirty region tracking in
       * javax.swing.RepaintManager.
       *
       * This bug shows I think how the 'unification' strategy of dirty regions
       * does not fit well at times with the JTable. Unification may result in alot
       * of unnecessary repaints of cells especially if the rectangles are far apart.
       * To deal with this specific situation I have added a class and some methods
       * to the RepaintManager. The new addDirtyRegion specifically targets JTable
       * instances. A new static inner class was introduced to store the dirty
       * Rectangles. A new painting method was created to deal with the new class.
       *
       * UNIFICATION FOR JTABLES NOW HAPPENS:
       * 1. When Rectangles intersect.
       * 2. When Rectangles are 'next' to each other. If they are to the left,
       * right, top or bottom of each other and their distance is 1, then they
       * are unified. This helps in cases where the rectangles are truly contiguous
       * to each other. The underlying assumption here is that doing these calculations
       * are going to be cheaper in the aggregate then doing the big repaint caused
       * by the generic unification scheme. I am presuming that this strategy is
       * a merger of how things happened before a repaint manager and the unfify
       * everything strategy of the modern RepaintManager.
       *
       * SKIPPING collectDirtyComponents AND adjustRoots:
       * Calling these methods seems self defeating. We are trying for a more
       * fragmented approach to repainting JTables, if we repaint the root this
       * implies repainting all of the JTable. Maybe we should do this, Ill
       * have to think about it more.
       *
       * THE COMPLEXIFICATION OF THE CODE:
       * Is complexification a word? :D Well this one special case certainly adds
       * a different path to many methods. Something simple and straightforward
       * now has to do testing each time to determine the right path to take for
       * its operation. We could move all the special cases into their own
       * JTable specific methods but that in turn adds a tremendous amount of
       * methods just for special cases. This is a conundrum. One size does
       * not fit all in this case but how can we make 2 sizes fit everyone without
       * making the code go crazy.
       *
       * AN alternative to this is to provide the ability for components to register
       * special repaint handling mechanisms to the repaint manager, which in turn
       * push the special handling code back onto the widget. This might prove
       * to be a better strategy than this, but it does result in the widget
       * managing its own repainting, which is what the RepaintManager is for. Argggh!
       * In other words, I don't think this implementation/suggestion is the final
       * word on the subject as of now. This thing is definately a good collaboration
       * candidate.
       *
       * DOES ONE SIZE FIT ALL?
       * Well it is conceivable that the whole repaint manager could be retooled to
       * just use this strategy. Im uncertain though if this is good. There may
       * very well be a whole in my thinking here, though I haven't seen it yet.
       * Retooling it would remove the "COMPLEXIFICATION" that I wrote about in the
       * last section.
       *
       * NO BENCHMARKS YET:
       * I haven't proven that this code is faster for JTables than the old. It
       * certainly does result in smaller repaints for the JTable than before, this
       * I have seen. So we won't have the beastly querying of the data model
       * to repaint the table if the cells are in different sections of the table.
       * Perception wise, its hard to discern a noticeable difference in the GUI
       * test. This is probably because of the inexpensive nature of the tests. Hmm...
       * what would make a good expensive test we could implement in this thing?. I
       * would think that grabbing 3 cells vs 100 cells for an operation that takes
       * 100ms for each grab would certainly show off the difference. Suddenly we
       * have a fraction of a second repaint vs a 1 second repaint.
       *
       * TESTING STRATEGY:
       * Stick a JTable up, press the JButton and watch different cells update.
       * It would be good to instrument the RepaintManager code to see how the repaints
       * fragment/merge... this gives you a sense of how things happen here.
       *
       * FILES AFFECTED: javax.swing.RepaintManager.
       * javax.swing.RepaintManager enhanced with java 6 version:
       * jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin
       *
       * test ran succesfully on a SUSE 7.3 Linux distribution
       *
       * Brian Harry
       * ###@###.###
       * JAN 22, 2005
       */

      UNIFIED DIFF:
      --- /home/nstuff/java6/jdk1.6.0/javax/swing/RepaintManager.java Thu Dec 15 02:17:38 2005
      +++ /home/javarefs/javax/swing/RepaintManager.java Mon Jan 23 10:12:16 2006
      @@ -9,6 +9,7 @@
       
       import java.awt.*;
       import java.awt.event.*;
      +import java.awt.geom.Rectangle2D;
       import java.awt.peer.ComponentPeer;
       import java.awt.peer.ContainerPeer;
       import java.awt.image.VolatileImage;
      @@ -70,6 +71,8 @@
           private Map<Container,Rectangle> tmpHWDirtyComponents;
       
           private Map<Component,Rectangle> dirtyComponents;
      + private Map<JTable,DirtyRegionManager> dirtyTables;
      + private Map<JTable,DirtyRegionManager> tmpDirtyTables;
           private Map<Component,Rectangle> tmpDirtyComponents;
           private java.util.List<Component> invalidComponents;
       
      @@ -254,6 +257,8 @@
               synchronized(this) {
                   dirtyComponents = new IdentityHashMap<Component,Rectangle>();
                   tmpDirtyComponents = new IdentityHashMap<Component,Rectangle>();
      + dirtyTables = new IdentityHashMap<JTable,DirtyRegionManager>();
      + tmpDirtyTables = new IdentityHashMap<JTable,DirtyRegionManager>();
                   this.bufferStrategyType = bufferStrategyType;
                   hwDirtyComponents = new IdentityHashMap<Container,Rectangle>();
                   tmpHWDirtyComponents = new IdentityHashMap<Container,Rectangle>();
      @@ -381,28 +386,7 @@
        * that c is completely obscured by an opaque ancestor in
        * the specified rectangle).
        */
      - Component root = null;
      -
      - // Note: We can't synchronize around this, Frame.getExtendedState
      - // is synchronized so that if we were to synchronize around this
      - // it could lead to the possibility of getting locks out
      - // of order and deadlocking.
      - for (Container p = c; p != null; p = p.getParent()) {
      - if (!p.isVisible() || (p.getPeer() == null)) {
      - return;
      - }
      - if ((p instanceof Window) || (p instanceof Applet)) {
      - // Iconified frames are still visible!
      - if (p instanceof Frame &&
      - (((Frame)p).getExtendedState() & Frame.ICONIFIED) ==
      - Frame.ICONIFIED) {
      - return;
      - }
      - root = p;
      - break;
      - }
      - }
      -
      + Component root = getRoot(c);
        if (root == null) return;
       
               synchronized(this) {
      @@ -418,7 +402,7 @@
        * rm.paintDirtyRegions() with SwingUtilities.invokeLater().
        */
        SystemEventQueueUtilities.queueComponentWorkRequest(root);
      - }
      + }
       
           /**
            * Add a component in the list of components that should be refreshed.
      @@ -434,7 +418,23 @@
            */
           public void addDirtyRegion(JComponent c, int x, int y, int w, int h)
           {
      - addDirtyRegion0(c, x, y, w, h);
      + if(c instanceof JTable){
      + JTable jt = (JTable)c;
      + DirtyRegionManager drm;
      + if(dirtyTables.containsKey(jt))
      + drm = dirtyTables.get(jt);
      + else{
      + drm = new DirtyRegionManager();
      + dirtyTables.put(jt, drm);
      + }
      + Rectangle r = new Rectangle(x, y, w, h);
      + drm.addRegion(r);
      + Component root = getRoot(jt);
      + if (root == null) return;
      + SystemEventQueueUtilities.queueComponentWorkRequest(root);
      + }
      + else
      + addDirtyRegion0(c, x, y, w, h);
           }
       
           /**
      @@ -471,6 +471,26 @@
               addDirtyRegion0(applet, x, y, w, h);
           }
       
      + private Component getRoot(Container c){
      + Component root = null;
      + for (Container p = c; p != null; p = p.getParent()) {
      + if (!p.isVisible() || (p.getPeer() == null)) {
      + return null;
      + }
      + if ((p instanceof Window) || (p instanceof Applet)) {
      + // Iconified frames are still visible!
      + if (p instanceof Frame &&
      + (((Frame)p).getExtendedState() & Frame.ICONIFIED) ==
      + Frame.ICONIFIED) {
      + return null;
      + }
      + root = p;
      + break;
      + }
      + }
      + return root;
      + }
      +
           // This is invoked from SystemEventQueueUtilities to flush any pending
           // heavy weight regions into real paints.
           void scheduleHeavyWeightPaints() {
      @@ -564,7 +584,13 @@
           public Rectangle getDirtyRegion(JComponent aComponent) {
        Rectangle r = null;
        synchronized(this) {
      - r = (Rectangle)dirtyComponents.get(aComponent);
      + if(aComponent instanceof JTable){
      + JTable jt = (JTable)aComponent;
      + DirtyRegionManager drm = dirtyTables.get(jt);
      + if(drm != null) r = drm.unified();
      + }
      + else
      + r = (Rectangle)dirtyComponents.get(aComponent);
        }
        if(r == null)
        return new Rectangle(0,0,0,0);
      @@ -572,12 +598,32 @@
        return new Rectangle(r);
           }
       
      + /** Return the currenty dirty regions for a component.
      + * Return an empty rectangle in an array if the component is not
      + * dirty.
      + */
      + public Rectangle[] getDirtyRegions(JComponent aComponent){
      +
      + if(aComponent instanceof JTable){
      + JTable jt = (JTable)aComponent;
      + DirtyRegionManager drm = dirtyTables.get(jt);
      + if(drm == null) return new Rectangle[0];
      + return drm.toArray();
      + }
      + return new Rectangle[]{ getDirtyRegion(aComponent)};
      + }
      +
           /**
            * Mark a component completely dirty. <b>aComponent</b> will be
            * completely painted during the next paintDirtyRegions() call.
            */
           public void markCompletelyDirty(JComponent aComponent) {
      - addDirtyRegion(aComponent,0,0,Integer.MAX_VALUE,Integer.MAX_VALUE);
      + if(aComponent instanceof JTable){
      + JTable jt = (JTable)aComponent;
      + addDirtyRegion(jt,0,0,Integer.MAX_VALUE,Integer.MAX_VALUE);
      + }
      + else
      + addDirtyRegion(aComponent,0,0,Integer.MAX_VALUE,Integer.MAX_VALUE);
           }
       
           /**
      @@ -586,7 +632,11 @@
            */
           public void markCompletelyClean(JComponent aComponent) {
        synchronized(this) {
      - dirtyComponents.remove(aComponent);
      + if(aComponent instanceof JTable){
      + JTable jt = (JTable)aComponent;
      + dirtyTables.remove(jt);
      + }
      + else dirtyComponents.remove(aComponent);
        }
           }
       
      @@ -597,14 +647,27 @@
            * if it return true.
            */
           public boolean isCompletelyDirty(JComponent aComponent) {
      - Rectangle r;
       
      - r = getDirtyRegion(aComponent);
      - if(r.width == Integer.MAX_VALUE &&
      - r.height == Integer.MAX_VALUE)
      - return true;
      - else
      - return false;
      + if(aComponent instanceof JTable){
      + JTable jt = (JTable)aComponent;
      + DirtyRegionManager drm = dirtyTables.get(jt);
      + if(jt == null) return false;
      + Rectangle r = drm.unified();
      + if(r.width == Integer.MAX_VALUE &&
      + r.height == Integer.MAX_VALUE)
      + return true;
      + else
      + return false;
      + }
      + else{
      + Rectangle r;
      + r = getDirtyRegion(aComponent);
      + if(r.width == Integer.MAX_VALUE &&
      + r.height == Integer.MAX_VALUE)
      + return true;
      + else
      + return false;
      + }
           }
       
       
      @@ -636,9 +699,11 @@
            */
           void seqPaintDirtyRegions() {
               Map<Component,Rectangle> dirtyComponents;
      + Map<JTable,DirtyRegionManager> dirtyTables;
               java.util.List<Runnable> runnableList;
               synchronized(this) {
                   dirtyComponents = this.dirtyComponents;
      + dirtyTables = this.dirtyTables;
                   runnableList = this.runnableList;
                   this.runnableList = null;
               }
      @@ -653,6 +718,9 @@
                   // with toplevels.
                   paintDirtyRegions(dirtyComponents);
               }
      + if(dirtyTables.size() > 0) {
      + paintDirtyTables(dirtyTables);
      + }
           }
       
           /**
      @@ -666,8 +734,14 @@
        tmpDirtyComponents = dirtyComponents;
        dirtyComponents = tmp;
        dirtyComponents.clear();
      +
      + Map<JTable,DirtyRegionManager> tmp2 = tmpDirtyTables;
      + tmpDirtyTables = dirtyTables;
      + dirtyTables = tmp2;
      + dirtyTables.clear();
        }
               paintDirtyRegions(tmpDirtyComponents);
      + paintDirtyTables(tmpDirtyTables);
           }
       
           private void paintDirtyRegions(Map<Component,Rectangle>
      @@ -746,6 +820,29 @@
        tmpDirtyComponents.clear();
           }
       
      + private void paintDirtyTables(Map<JTable,DirtyRegionManager>
      + tmpDirtyTables){
      + painting = true;
      + /**we don't do the funky painting of roots in this method
      + primarily because I don't understand what it does yet! */
      + try {
      + for(JTable jt: tmpDirtyTables.keySet()){
      + DirtyRegionManager drm = tmpDirtyTables.get(jt);
      + Rectangle vrect = jt.getVisibleRect();
      + for(Rectangle r: drm){
      + if(r.intersects(vrect)){
      + Rectangle vints = r.intersection(vrect);
      + jt.paintImmediately(vints.x, vints.y, vints.width, vints.height);
      + }
      + }
      +
      +
      + }
      + } finally {
      + painting = false;
      + }
      + tmpDirtyTables.clear();
      + }
       
           /**
            * Removes any components from roots that are children of
      @@ -860,6 +957,10 @@
        StringBuffer sb = new StringBuffer();
        if(dirtyComponents != null)
        sb.append("" + dirtyComponents);
      + if(dirtyTables != null){
      + if(sb.length() != 0) sb.append("\n");
      + sb.append("" + dirtyTables);
      + }
               return sb.toString();
           }
       
      @@ -1416,4 +1517,72 @@
               public Dimension size;
               public boolean needsReset = false;
           }
      +
      + private class DirtyRegionManager implements Iterable<Rectangle>{
      + LinkedList<Rectangle> regions = new LinkedList<Rectangle>();
      +
      + public Iterator<Rectangle> iterator(){
      + return regions.iterator();
      + }
      +
      + public Rectangle[] toArray(){
      + return regions.toArray( new Rectangle[regions.size()]);
      + }
      +
      + public Rectangle unified(){
      + Rectangle r = new Rectangle(0, 0, 0, 0);
      + for(Rectangle r2: regions)
      + r = r.union(r2);
      + return r;
      + }
      +
      + public void addRegion(Rectangle r){
      + while(true){
      + Iterator<Rectangle> ri = regions.iterator();
      + while(ri.hasNext()){
      + Rectangle r2 = ri.next();
      + if(r.intersects(r2)){
      + ri.remove();
      + r = r.union(r2);
      + continue;
      + }
      + else{
      + int ocode = r.outcode(r2.x, r2.y);
      + int distance = 0;
      + switch(ocode){
      + case Rectangle2D.OUT_LEFT:
      + int spot = r2.x + r2.width;
      + distance = spot - r.x;
      + break;
      + case Rectangle2D.OUT_RIGHT:
      + int spot2 = r.x + r.width;
      + distance = r2.x - spot2;
      + break;
      + case Rectangle2D.OUT_TOP:
      + int spot3 = r2.y + r2.height;
      + distance = spot3 - r.y;
      + break;
      + case Rectangle2D.OUT_BOTTOM:
      + int spot4 = r.y + r.height;
      + distance = r2.y - spot4;
      + break;
      + default:
      + continue;
      + }
      + if(distance == 1){
      + ri.remove();
      + r = r.union(r2);
      + continue;
      + }
      + }
      +
      + }
      + regions.addLast(r);
      + break;
      + }
      +
      + }
      +
      + }
      +
       }



      JUnit TESTCASE too large to show here.
      Refer to attached file 633596.txt

            Unassigned Unassigned
            prr Philip Race
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Imported:
              Indexed: