svn - r34267 - branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing

1 message Options
Embed this post
Permalink
svn_geotools

svn - r34267 - branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing

Reply Threaded More More options
Print post
Permalink
Author: mbedward
Date: 2009-10-27 09:42:29 -0400 (Tue, 27 Oct 2009)
New Revision: 34267

Modified:
   branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing/JMapPane.java
   branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing/RenderingExecutor.java
Log:
Fixed thread race between rendering task cancellation and submittal

Modified: branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing/JMapPane.java
===================================================================
--- branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing/JMapPane.java 2009-10-27 13:34:57 UTC (rev 34266)
+++ branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing/JMapPane.java 2009-10-27 13:42:29 UTC (rev 34267)
@@ -77,7 +77,7 @@
  * @see JMapFrame
  * @see MapPaneListener
  * @see CursorTool
- *
+ *
  * @author Michael Bedward
  * @author Ian Turton
  * @since 2.6
@@ -182,11 +182,11 @@
     private boolean redrawBaseImage;
     private boolean needNewBaseImage;
     private boolean baseImageMoved;
-    private boolean displayAreaChanged;
+    private boolean clearLabelCache;
 
 
-    /**
-     * Constructor - creates an instance of JMapPane with no map
+    /**
+     * Constructor - creates an instance of JMapPane with no map
      * context or renderer initially
      */
     public JMapPane() {
@@ -196,7 +196,7 @@
     /**
      * Constructor - creates an instance of JMapPane with the given
      * renderer and map context.
-     *
+     *
      * @param renderer a renderer object
      * @param context an instance of MapContext
      */
@@ -207,7 +207,7 @@
         needNewBaseImage = true;
         redrawBaseImage = true;
         baseImageMoved = false;
-        displayAreaChanged = false;
+        clearLabelCache = false;
 
         /*
          * We use a Timer object to avoid rendering delays and
@@ -269,7 +269,6 @@
                 resizeTimer.restart();
             }
         });
-
     }
 
     /**
@@ -302,15 +301,16 @@
             }
 
             repaint();
-        
+
             MapPaneEvent ev = new MapPaneEvent(this, MapPaneEvent.Type.PANE_RESIZED);
             publishEvent(ev);
         }
+
     }
 
     /**
      * Set the current cursor tool
-     *
+     *
      * @param tool the tool to set; null means no active cursor tool
      */
     public void setCursorTool(CursorTool tool) {
@@ -358,7 +358,7 @@
 
     /**
      * Register an object that wishes to receive {@code MapPaneEvent}s
-     *
+     *
      * @param listener an object that implements {@code MapPaneListener}
      * @see MapPaneListener
      */
@@ -397,7 +397,7 @@
     }
 
     /**
-     * Set the renderer for this map pane.
+     * Set the renderer for this map pane.
      *
      * @param renderer the renderer to use
      */
@@ -441,7 +441,7 @@
      */
     public void setMapContext(MapContext context) {
         if (this.context != context) {
-            
+
             if (this.context != null) {
                 this.context.removeMapLayerListListener(this);
                 for( MapLayer layer : this.context.getLayers() ){
@@ -462,7 +462,7 @@
                     layer.setSelected(true);
                     if( layer instanceof ComponentListener){
                         addComponentListener( (ComponentListener) layer );
-                    }                    
+                    }
                 }
 
                 setFullExtent();
@@ -525,7 +525,7 @@
      * <p>
      * Note: This method does <b>not</b> check that the requested area overlaps
      * the bounds of the current map layers.
-     *
+     *
      * @param envelope the bounds of the map to display
      *
      * @throws IllegalStateException if a map context is not set
@@ -533,7 +533,7 @@
     public void setDisplayArea(Envelope envelope) {
         if (context != null) {
             /*
-             * If the pane has not been displayed yet or is zero size then
+             * If the pane has not been displayed yet or is zero size then
              * just record the requested display area and defer setting transforms
              * etc.
              */
@@ -541,10 +541,10 @@
                 pendingDisplayArea = new ReferencedEnvelope(envelope);
             } else {
                 doSetDisplayArea(envelope);
-                displayAreaChanged = true;
+                clearLabelCache = true;
                 repaint();
             }
-            
+
         } else {
             throw new IllegalStateException("Map context must be set before setting the display area");
         }
@@ -637,7 +637,7 @@
 
     /**
      * Specify whether the map pane should defer its normal
-     * repainting behaviour.
+     * repainting behaviour.
      * <p>
      * Typical use:
      * <pre>{@code
@@ -740,7 +740,7 @@
 
     /**
      * Get a (copy of) the world to screen coordinate transform
-     * being used by this map pane. This method can be
+     * being used by this map pane. This method can be
      * used to determine the current drawing scale...
      * <pre>{@code
      * double scale = mapPane.getWorldToScreenTransform().getScaleX();
@@ -761,7 +761,7 @@
      * allows dragging the map without the overhead of redrawing
      * the features during the drag. For example, it is used by
      * {@link org.geotools.swing.tool.PanTool}.
-     *
+     *
      * @param dx the x offset in pixels
      * @param dy the y offset in pixels.
      */
@@ -779,17 +779,15 @@
      */
     @Override
     protected void paintComponent(Graphics g) {
-        System.out.println("paintComponent");
-
         super.paintComponent(g);
 
         if (acceptRepaintRequests) {
 
-            if (context == null || renderer == null) {
-                return;
-            }
+            if (curPaintArea == null ||
+                context == null ||
+                context.getLayerCount() == 0 ||
+                renderer == null) {
 
-            if (curPaintArea == null ) {
                 return;
             }
 
@@ -801,7 +799,7 @@
                 baseImageGraphics = baseImage.createGraphics();
                 needNewBaseImage = false;
                 redrawBaseImage = true;
-                labelCache.clear();
+                clearLabelCache = true;
             }
 
             final ReferencedEnvelope mapAOI = context.getAreaOfInterest();
@@ -813,7 +811,7 @@
                 if (baseImageMoved) {
                     afterImageMove(mapAOI, curPaintArea);
                     baseImageMoved = false;
-                    labelCache.clear();
+                    clearLabelCache = true;
                 }
 
                 if (renderingExecutor.submit(mapAOI, curPaintArea, baseImageGraphics)) {
@@ -828,7 +826,7 @@
                 Graphics2D g2 = (Graphics2D) g;
                 g2.drawImage(baseImage, imageOrigin.x, imageOrigin.y, null);
             }
-            
+
             redrawBaseImage = true;
         }
     }
@@ -841,11 +839,10 @@
      * @see MapPaneListener#onRenderingStopped(org.geotools.swing.event.MapPaneEvent)
      */
     public void onRenderingCompleted() {
-        System.out.println("onRenderingCompleted");
-
-        if (displayAreaChanged) {
+        if (clearLabelCache) {
             labelCache.clear();
         }
+        clearLabelCache = false;
 
         Graphics2D paneGr = (Graphics2D) this.getGraphics();
         paneGr.drawImage(baseImage, imageOrigin.x, imageOrigin.y, null);
@@ -919,7 +916,7 @@
         if( layer instanceof ComponentListener ){
             addComponentListener( (ComponentListener) layer );
         }
-        
+
         if (context.getLayerCount() == 1) {
             setFullExtent();
             reset();
@@ -1015,7 +1012,7 @@
      * <p>
      * Tne transform is calculated such that {@code envelope} will
      * be centred in the display
-     *
+     *
      * @param envelope the current map extent (world coordinates)
      * @param paintArea the current map pane extent (screen units)
      */
@@ -1033,7 +1030,7 @@
         worldToScreen = new AffineTransform(scale, 0, 0, -scale, -xoff, yoff);
         try {
             screenToWorld = worldToScreen.createInverse();
-            
+
         } catch (NoninvertibleTransformException ex) {
             ex.printStackTrace();
         }
@@ -1055,7 +1052,7 @@
 
     /**
      * Publish a MapPaneEvent to registered listeners
-     *
+     *
      * @param ev the event to publish
      * @see MapPaneListener
      */

Modified: branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing/RenderingExecutor.java
===================================================================
--- branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing/RenderingExecutor.java 2009-10-27 13:34:57 UTC (rev 34266)
+++ branches/2.6.x/modules/unsupported/swing/src/main/java/org/geotools/swing/RenderingExecutor.java 2009-10-27 13:42:29 UTC (rev 34267)
@@ -25,6 +25,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -79,6 +80,12 @@
 
     private long pollingInterval;
 
+    /*
+     * This latch is used to avoid a race between the cancellation of
+     * a current task and the submittal of a new task
+     */
+    private CountDownLatch cancelLatch;
+
     /**
      * Constants to indicate the result of a rendering task
      */
@@ -89,6 +96,8 @@
         FAILED;
     }
 
+    private long numFeatures;
+
     /**
      * A rendering task
      */
@@ -97,6 +106,7 @@
         private final ReferencedEnvelope envelope;
         private final Rectangle paintArea;
         private final Graphics2D graphics;
+
         private boolean cancelled;
         private boolean failed;
 
@@ -111,7 +121,7 @@
             this.envelope = envelope;
             this.paintArea = paintArea;
             this.graphics = graphics;
-            cancelled = false;
+            this.cancelled = false;
             failed = false;
         }
 
@@ -124,19 +134,23 @@
         public TaskResult call() throws Exception {
             if (!cancelled) {
                 GTRenderer renderer = mapPane.getRenderer();
-                renderer.addRenderListener(this);
+                try {
+                    renderer.addRenderListener(this);
 
-                /*
-                 * Clear the paint area
-                 */
-                Composite composite = graphics.getComposite();
-                graphics.setComposite(AlphaComposite.getInstance(AlphaComposite.CLEAR, 0.0f));
-                graphics.fill(paintArea);
-                graphics.setComposite(composite);
+                    /*
+                     * Clear the paint area
+                     */
+                    Composite composite = graphics.getComposite();
+                    graphics.setComposite(AlphaComposite.getInstance(AlphaComposite.CLEAR, 0.0f));
+                    graphics.fill(paintArea);
+                    graphics.setComposite(composite);
 
-                renderer.paint(graphics, mapPane.getVisibleRect(), envelope, mapPane.getWorldToScreenTransform());
+                    numFeatures = 0;
+                    renderer.paint(graphics, mapPane.getVisibleRect(), envelope, mapPane.getWorldToScreenTransform());
 
-                renderer.removeRenderListener(this);
+                } finally {
+                    renderer.removeRenderListener(this);
+                }
             }
 
             if (cancelled) {
@@ -166,7 +180,8 @@
          */
         public void featureRenderer(SimpleFeature feature) {
             // @todo update a progress listener
-            }
+            numFeatures++ ;
+        }
 
         /**
          * Called by the renderer on error
@@ -195,6 +210,7 @@
         taskExecutor = Executors.newSingleThreadExecutor();
         watchExecutor = Executors.newSingleThreadScheduledExecutor();
         pollingInterval = DEFAULT_POLLING_INTERVAL;
+        cancelLatch = new CountDownLatch(0);
     }
 
     /**
@@ -229,7 +245,14 @@
      *         rejected
      */
     public synchronized boolean submit(ReferencedEnvelope envelope, Rectangle paintArea, Graphics2D graphics) {
-        if (!isRunning()) {
+        if (!isRunning() || cancelLatch.getCount() > 0) {
+            try {
+                // wait for any cancelled task to finish its shutdown
+                cancelLatch.await();
+            } catch (InterruptedException ex) {
+                return false;
+            }
+
             task = new Task(envelope, paintArea, graphics);
             taskRunning.set(true);
             taskResult = taskExecutor.submit(task);
@@ -249,9 +272,10 @@
     /**
      * Cancel the current rendering task if one is running
      */
-    public void cancelTask() {
+    public synchronized void cancelTask() {
         if (isRunning()) {
             task.cancel();
+            cancelLatch = new CountDownLatch(1);
         }
     }
 
@@ -273,6 +297,7 @@
 
         switch (result) {
             case CANCELLED:
+                cancelLatch.countDown();
                 mapPane.onRenderingCancelled();
                 break;
 


------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
GeoTools-commits mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geotools-commits