Hi,
today I had a look at WMSLab to see what was the issue reported
by a user on the users list.

After playing a bit with it, I'm wondering if it actually ever worked.
I have a patch that solves some of the issues, it's attached for
review.

Issues I've found:
- the raster symbolizer is created that looks into an attribute named
  "raster" for the grid. However the wrapped coverages have been called
  "grid" for a long time (I went back in the history quite a bit
  and did not find a time in which they were called "raster" instead).
  Long story short, with this difference it could not have worked,
  not now, neither two years ago
- the WMSMapLayer seems an unfinished work in progress.
  It assumes it can turn loose a runnable that will fetch them map,
  and that it will return in time for the getFeatureSource() call.
  Purely assumes, I mean, there is no synchronization at all.
  If it eve worked, it did by accident.
  Also the class was filled with System.out
  I modified the class to use Future in order to wait for the
  return value and turned the System.out into LOGGER calls
- the WMSMapLayer has to play tricks in order to issue the
  GetMap request, in particular it requires the mapBoundsChanged
  to be called, but that happens only if whoever adds the
  layer also registers it as a bounds listener to the map.
  I modified the WMSLab to do so.

With all the modifications above the example actually turns
out images and you can pan and zoom.

There are however a number of issues still unsolved, which
I hope someone else might tackle:
- the map is always distorted. This happens because the
  WMSMapLayer always assumes a painting area of 200x400,
  and someone has to change those values in the various
  setter of the WMSMapLayer dynamically... ugh!
- the feature info request does not work because the
  GUI assumes the layer is a raster. Well, it's a fake
  one, the feature info tool needs to be modified
  to be aware of that
- the WMSMapLayer uses a thread pool which is not bounded,
  meaning it will do any amount of requests in parallel.
  This is very bad behaviour, all HTTP clients are supposed
  to avoid making more than 4-6 requests in parallel against
  the same server to avoid overwhelm it. Years ago browsers
  never did more than 2 in parallel, nowadays Firefox does
  6, but never more (unless you install some accelerator)
- the reprojection simply does not work, but I did not
  investigate on the why

Thinking about it, I believe it would be much better if
WMSMapLayer was implemented in a different way.
My best bet would be on a layer that wraps a custom
coverage reader, the reader being in fact something
that delegates the request to the WMS server _when_
it's time to render that layer.
By doing so the reader would get all the necessary
hints, the actual geographic area to be painted and
also the on screen rectangle to be generated.
It would also not need any kind of synchronization, because
it would do the request on demand.
The downside is that it would not be able to make
requests in parallel as it does now (since now the request
starts when the area of interest of the map changes,
not when the getFeatureSource() method is called).

If people are interested in the above approach I
can try to implement it. I'll do eventually anyways
when I start implementing WMS cascading for GeoServer,
but that will happen only in a couple of months
time

Cheers
Andrea


--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.
Index: modules/library/main/src/main/java/org/geotools/styling/RasterSymbolizerImpl.java
===================================================================
--- modules/library/main/src/main/java/org/geotools/styling/RasterSymbolizerImpl.java	(revisione 34976)
+++ modules/library/main/src/main/java/org/geotools/styling/RasterSymbolizerImpl.java	(copia locale)
@@ -60,7 +60,7 @@
     }
     
     public RasterSymbolizerImpl(FilterFactory factory, Description desc, String name, Unit<Length> uom, OverlapBehavior behavior) {
-        super(name, desc, "raster", uom);
+        super(name, desc, "grid", uom);
         this.filterFactory = factory;
         this.opacity = filterFactory.literal(1.0);
         this.overlap = filterFactory.literal(OverlapBehavior.RANDOM);
Index: modules/extension/wms/src/main/java/org/geotools/map/WMSMapLayer.java
===================================================================
--- modules/extension/wms/src/main/java/org/geotools/map/WMSMapLayer.java	(revisione 34976)
+++ modules/extension/wms/src/main/java/org/geotools/map/WMSMapLayer.java	(copia locale)
@@ -14,7 +14,6 @@
 import java.awt.event.ComponentListener;
 import java.awt.image.BufferedImage;
 import java.io.BufferedReader;
-import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.net.URL;
@@ -23,6 +22,7 @@
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -38,13 +38,13 @@
 import org.geotools.data.wms.WebMapServer;
 import org.geotools.data.wms.request.GetMapRequest;
 import org.geotools.factory.CommonFactoryFinder;
-import org.geotools.factory.FactoryRegistryException;
 import org.geotools.geometry.GeneralEnvelope;
 import org.geotools.geometry.jts.ReferencedEnvelope;
 import org.geotools.map.event.MapBoundsEvent;
 import org.geotools.map.event.MapBoundsListener;
 import org.geotools.map.event.MapLayerEvent;
 import org.geotools.referencing.CRS;
+import org.geotools.renderer.GTRenderer;
 import org.geotools.resources.coverage.FeatureUtilities;
 import org.geotools.styling.FeatureTypeStyle;
 import org.geotools.styling.RasterSymbolizer;
@@ -54,9 +54,14 @@
 import org.geotools.styling.Symbolizer;
 import org.opengis.feature.simple.SimpleFeature;
 import org.opengis.feature.simple.SimpleFeatureType;
-import org.opengis.referencing.FactoryException;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
 
+/**
+ * Integrates a WMS layer into a {...@link MapContext}. For this layer to work it is
+ * crucial that the layer is added as a {...@link MapBoundsListener} to the MapContext and
+ * that the MapContext area of interest is kept in synch with whatever is provided to
+ * the {...@link GTRenderer} paint calls 
+ */
 public class WMSMapLayer extends DefaultMapLayer implements MapLayer, MapBoundsListener,
         ComponentListener {
     /** The logger for the map module. */
@@ -88,6 +93,8 @@
     private String exceptions = "application/vnd.ogc.se_inimage";
 
     private static ExecutorService executor = Executors.newCachedThreadPool();
+    
+    private static Future<?> future;
 
     Runnable getmap = new Runnable() {
         public void run() {
@@ -104,8 +111,27 @@
         this.layer = layer;
         this.wms = wms;
         setDefaultStyle();
-        executor.execute(getmap);
     }
+    
+    @Override
+    public FeatureSource<SimpleFeatureType, SimpleFeature> getFeatureSource() {
+        if(featureSource == null) {
+            if(future == null) {
+                future = executor.submit(getmap);
+            }
+        }
+        
+        if(future != null) {
+            try {
+                future.get();
+                future = null;
+            } catch(Exception e) {
+                throw new RuntimeException("Failed to perform the GetMap request");
+            }
+        }
+        
+        return featureSource;
+    }
 
     public void setDefaultStyle() {
         RasterSymbolizer symbolizer = factory.createRasterSymbolizer();
@@ -154,7 +180,6 @@
         GetMapRequest mapRequest = wms.createGetMapRequest();
         mapRequest.addLayer(layer);
 
-        // System.out.println(width + " " + height);
         mapRequest.setDimensions(getWidth(), getHeight());
         mapRequest.setFormat("image/png");
 
@@ -184,14 +209,13 @@
             requestBBox = getBounds();
         }
 
-        // System.out.println(bbox.toString());
         mapRequest.setSRS(srsName);
         mapRequest.setBBox(requestBBox);
         mapRequest.setTransparent(transparent);
 
         URL request = mapRequest.getFinalURL();
 
-        System.out.println(request.toString());
+        LOGGER.fine(request.toString());
         InputStream is = null;
 
         try {
@@ -201,26 +225,24 @@
 
             if (type.equalsIgnoreCase("image/png")) {
                 BufferedImage image = ImageIO.read(is);
-                System.out.println("get map completed");
+                LOGGER.fine("GetMap completed");
                 grid = gcf.create(layer.getTitle(), image, requestBBox);
-                // System.out.println("fetched new grid");
-                // if (featureSource == null)
                 featureSource = DataUtilities.source(FeatureUtilities.wrapGridCoverage(grid));
                 if( viewport == null ){
-                    System.out.println("get map complete but viewport not yet established");
+                    LOGGER.fine("Get map complete but viewport not yet established");
                 }
                 else {
                     fireMapLayerListenerLayerChanged(new MapLayerEvent(this, MapLayerEvent.DATA_CHANGED));                    
                 }
             } else {
-                System.out.println("error content type is " + type);
+                LOGGER.severe("GetMap returned an error, content type is " + type);
 
                 if (StringUtils.contains(type, "text") || StringUtils.contains(type, "xml")) {
                     String line = "";
                     BufferedReader br = new BufferedReader(new InputStreamReader(is));
 
                     while ((line = br.readLine()) != null) {
-                        System.out.println(line);
+                        LOGGER.severe(line);
                     }
                 }
             }        
@@ -234,12 +256,10 @@
     public void mapBoundsChanged(MapBoundsEvent event) {
         viewport = ((MapContext) event.getSource()).getAreaOfInterest();
 
-        // System.out.println("old:" + bbox + "\n" + "new:"
-        // + event.getOldAreaOfInterest());
         if (!viewport.equals(event.getOldAreaOfInterest())) {
-            System.out.println("bbox changed - fetching new grid");
+            LOGGER.fine("bbox changed - fetching new grid");
 
-            executor.execute(getmap);
+            future = executor.submit(getmap);
         }
     }
 
Index: demo/example/src/main/java/org/geotools/demo/WMSLab.java
===================================================================
--- demo/example/src/main/java/org/geotools/demo/WMSLab.java	(revisione 34976)
+++ demo/example/src/main/java/org/geotools/demo/WMSLab.java	(copia locale)
@@ -44,6 +44,7 @@
         for( Layer wmsLayer : wmsLayers ){
             WMSMapLayer displayLayer = new WMSMapLayer(wms, wmsLayer );
             mapcontext.addLayer( displayLayer );
+            mapcontext.addMapBoundsListener(displayLayer);
         }
         // Now display the map
         JMapFrame.showMap(mapcontext);
------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Geotools-devel mailing list
Geotools-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to