Looks like great work Andrea. Kudos. I haven't had a chance to review the patch yet (on the road atm) but will try to provide some feedback.
As for KML... its indeed troubling. KML seems to be going the way of unsupported these days... i wonder if it is time to think about a demotion into a community module in case a maintainer wants to step up. As for maintenance after the big switch again problematic... seems like a tough call. Not much we can do I guess... past backporting the work to stable branch but this by definition seems suited to only trunk. 2c. On Sat, Jun 4, 2011 at 11:30 AM, Andrea Aime <[email protected]>wrote: > Hi all, > for the second time in a week I'm cross posting on geotools and > geoserver list for a topic that is of interest of both communities. > > Some months ago, right before the GeoTools 2.7.0 release, Jody > (and Michael too I guess?) made and implemented this proposal: > http://docs.codehaus.org/display/GEOTOOLS/MapContext+Refactor > > The proposal was not fully executed as the renderer still works against > MapContext and friends. > Now Michael has worked a patch to complete the proposal, on which > he is waiting for in order to get some work done on the swing module: > http://jira.codehaus.org/browse/GEOT-3565 > > The original patch deprecated just MapContext. However, since MapContent > is made of Layer classes (which is actually a hierarchy or type specific > classes, see attached screenshot), it also makes sense to deprecate > MapLayer (which is-not-a Layer). > > This generates a number of deprecated usages for applications that > work a lot with maps. > Most applications that just setup a map and have it rendered > will have it somewhat easy, just change the part that builds the > map, however others, more complex ones will be hit harder. > > GeoServer is one such example, MapContext and MapLayer are not > just built to feed StreamingRenderer, but they are also used later > to analyze the structure of the map and fed into at least five custom > renderer, the kml one, the streaming svg one, rss and atom, and > the html image map one. > Also the testing code sets up and uses a lot those classes too. > > I've spent all the day performing a tentative switch of the code base > and attached the resulting patch here: > http://jira.codehaus.org/browse/GEOS-4597 > As you can see the patch is not small, weighting 121KB, and affects > pretty much all of the wms module, plus the CRS area of validity > preview in the GUI modules, gwc integration, html image map output > format and the python module. > > The patch basically does two things: > - switches WMSMapContext to be a MapContent extension > - switches all MapLayer usage to using Layer subclasses > > The first is mechanical, a lot of getAreaOfInterest/setAreaOfInterest > needed to be changed to getViewport().getBounds/setBounds. > > The latter is more of black magic, and I probably made mistakes > during the conversion, especially in the KML generation subsytem > which was the most heavily hit. > > The latter is kind of a problem because all of the code using layers > in GeoServer assumes that: > - all layers are uniform unless otherwise needed > - every layer has a query > - every layer has a style > - every layer can return a feature source > > The "Layer" hierarchy breaks all of the above assumptions. > Throughout the code I had to make guess on what kind of layer > the code was meant to work with (feature? grid coverage reader? > wms layer?) and perform casts accordingly, of course with > quite the number of "instanceof" checks spread around. > > Now, the patch survives a build and some interactive testing. > I did not, however, check KML generation: there are a million > different code paths and configurations and I'm not well > setup to test them on Linux (GE does not really work well > here). > All the patch needs a review, but the KML part really needs > a detailed one along with tester willing to check out all of the > usage cases (network links, superoverlays, cached regionation > hierarchies, extrusion and time support, gwc integration, and so on). > Need help on this one, I'm not going to do it myself. > > The patch also has maintenance implications for GeoServer: it will > make harder to backport to the stable branch any patch affecting > WMS, as we'll have to write it for Layer on trunk and recode it > for MapLayer on 2.1.x. > In particular it will very likely intersect with my time/elevation > work, making the first patch landing require reworks in the other. > > Anyways, during the switch I took down notes about things that > bite me during the switch and probably need improvement on > the GeoTools side: > - GridReaderLayer constuctors force one to specify the title to > pass also the parameters. Title might not be always needed, > but some readers will not give the best without custom param > settings. Please add a constructor wtih reader, style and params > - getAreaOfInterest() replacement is getViewport().getBounds(), > made me stare for a good five minutes at the API looking for > it though. I guess we need an upgrade guide and this one should > be cited > - getViewport().getBounds() returns null if not explicitly initialized. > That also got me into quite a bit of trouble, the old classes > would initialize the bounds to the layer bounds if no manual > initialization was made. > Please get back this behavior, otherwise upgraders will have to > fight quite a bit of NullPointerExceptions > - GridCoveageLayer and GridReaderLayer really need a GridLayer > base class, right now to recognize if a layer is raster one has > to make two instanceof, two casts, and if the code relying on the > ability to extract a feature source out of the layer is hard to > change, two casts to call the toFeatureCollection() method that > both expose > > Opinions, suggestions, concerns... even flames if you really need to... > welcomed! > > Cheers > Andrea > > -- > ------------------------------------------------------- > Ing. Andrea Aime > GeoSolutions S.A.S. > Tech lead > > Via Poggio alle Viti 1187 > 55054 Massarosa (LU) > Italy > > phone: +39 0584 962313 > fax: +39 0584 962313 > > http://www.geo-solutions.it > http://geo-solutions.blogspot.com/ > http://www.youtube.com/user/GeoSolutionsIT > http://www.linkedin.com/in/andreaaime > http://twitter.com/geowolf > > ------------------------------------------------------- > > > ------------------------------------------------------------------------------ > Simplify data backup and recovery for your virtual environment with > vRanger. > Installation's a snap, and flexible recovery options mean your data is > safe, > secure and there when you need it. Discover what all the cheering's about. > Get your free trial download today. > http://p.sf.net/sfu/quest-dev2dev2 > _______________________________________________ > Geotools-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel > > -- Justin Deoliveira OpenGeo - http://opengeo.org Enterprise support for open source geospatial.
------------------------------------------------------------------------------ Simplify data backup and recovery for your virtual environment with vRanger. Installation's a snap, and flexible recovery options mean your data is safe, secure and there when you need it. Discover what all the cheering's about. Get your free trial download today. http://p.sf.net/sfu/quest-dev2dev2
_______________________________________________ Geoserver-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geoserver-devel
