GeoTools

MapContext Refractor

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.7-M1
  • Fix Version/s: 2.7-M1
  • Component/s: render
  • Labels:
    None

Description

MapContext and MapLayer have really grown over time - and in some cases have diverged from their actual use.

The proposal for simplification is here:

The focus is on cleaning up MapLayer:

  • MapLayer replaced by Layer class
  • Specific Layer subclasses for different kinds of content; this is an open ended set allowing additional kinds of layers to be added over time for TileServers, Google Maps and so forth
  • DefaultMapLayer re-factored to use an internal Layer delegate (so existing code will not be broken; and importantly will not be duplicated)

Over the course of the proposal MapContext and DefaultMapContext were also re-factored:

  • Turn MapContext into a class allowing DefaultmapContext to be deprecated (same solution as Query)
  • Super class called MapContent introduced
    • MapContent uses layers() method to provide direct access to the layer list
    • MapContent improved "viewport" methods
  • Responsibilities for maintaining "area of interest" isolated into a separate MapViewport class
    • MapContent can use a Mapviewport internally to maintain screen and where to draw information
    • MapViewport can be used directly when drawing the same MapContent into several tiles

This proposal does not break any existing API; it provides a safe migration path forward (and like the Query proposal) makes use of classes directly for a simplified experience.

Issue Links

is depended upon by

Improvement - An improvement or enhancement to an existing feature or task. GEOT-3150 Update GTRender to use MapContent, MapViewport and Layer

  • Major - Major loss of function.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jody Garnett added a comment -
This patch is complete with DefaultMapLayer delegating to an internal layer; the renderer and other code are untouched.
Show
Jody Garnett added a comment - This patch is complete with DefaultMapLayer delegating to an internal layer; the renderer and other code are untouched.
Hide
Permalink
Jody Garnett added a comment -
Recreated patch from root of trunk (the previous one was created from the modules directory). This patch includes some additional testing code for shapefile map renderer that I used to debug a problem when Query was null.
Show
Jody Garnett added a comment - Recreated patch from root of trunk (the previous one was created from the modules directory). This patch includes some additional testing code for shapefile map renderer that I used to debug a problem when Query was null.
Hide
Permalink
Jody Garnett added a comment -
Today's patch includes Map implemented; and a method to convert DefaultMapContext to a Map. I have a choice of having DefaultMapContext use an internal Map (probably smart) or having it extend Map (sounds too complicated).

Tomorrow I will change GTRender to have a setMap method in addition to the current setMapContext method.
Show
Jody Garnett added a comment - Today's patch includes Map implemented; and a method to convert DefaultMapContext to a Map. I have a choice of having DefaultMapContext use an internal Map (probably smart) or having it extend Map (sounds too complicated). Tomorrow I will change GTRender to have a setMap method in addition to the current setMapContext method.
Hide
Permalink
Jody Garnett added a comment -
Okay DefaultMapContext now extends Map and implements MapContext. Also sorted out DirectLayer to use Map and MapViewport as parameters (we will see if anyone likes it!).

This represents a complete working patch that does not break API.

So two ideas that are disruptive in nature:
- Copy "Map" to replace "MapContext"
- Update the internals of the GTRender implementations to use Layer, FeatureLayer and so forth
Show
Jody Garnett added a comment - Okay DefaultMapContext now extends Map and implements MapContext. Also sorted out DirectLayer to use Map and MapViewport as parameters (we will see if anyone likes it!). This represents a complete working patch that does not break API. So two ideas that are disruptive in nature: - Copy "Map" to replace "MapContext" - Update the internals of the GTRender implementations to use Layer, FeatureLayer and so forth
Hide
Permalink
Jody Garnett added a comment -
There is a directlayer sample implementation.
Shapefile renderer tests should pass now; they were failing due to a difference of implementation between DefaultMapLayer.getLayerBounds() and Map.getMaxBounds().

I think the new getMapBounds() implementation is correct as it always respects the CRS provided by the Map; the other implementation looks like it will make use of the first layer bounds if the dataCRS is not provided.

I went back to 2.6.x and confirmed; going to add a warning in this case but maintain functionality so test cases pass. This is a corner case of a corner case - that people may run into often when they are just trying to get something quickly on the screen (sigh). The case is trying to display a shapefile where no projection is known; the code ends up ignoring the map projection and just grabbing the bounds of the shapefile numerically. I would kind of prefer to ignore the layer if it does not have a valid area to contribute.
Show
Jody Garnett added a comment - There is a directlayer sample implementation. Shapefile renderer tests should pass now; they were failing due to a difference of implementation between DefaultMapLayer.getLayerBounds() and Map.getMaxBounds(). I think the new getMapBounds() implementation is correct as it always respects the CRS provided by the Map; the other implementation looks like it will make use of the first layer bounds if the dataCRS is not provided. I went back to 2.6.x and confirmed; going to add a warning in this case but maintain functionality so test cases pass. This is a corner case of a corner case - that people may run into often when they are just trying to get something quickly on the screen (sigh). The case is trying to display a shapefile where no projection is known; the code ends up ignoring the map projection and just grabbing the bounds of the shapefile numerically. I would kind of prefer to ignore the layer if it does not have a valid area to contribute.
Hide
Permalink
Jody Garnett added a comment -
The good news is that I have been able to complete the work without breaking any API; I accomplished this by turning MapContext into a class - that extends MapContent. Yes MapContent was the safe name I found to replace "Map". In a similar manner MapLayer is now a class; and extra care has been taken to make it a pure wrapper that does not hold any state.

There is a difference between DefaultMapContext and MapContext - DefaultMapContext is willing to create default styles; and generate a default bounds for the map as layers are added.

Created final patch; unable to commit onto a branch as I am having the following svn error:

%> svn cp . http://svn.osgeo.org/geotools/branches/map-context -m "final patch, does not break api"
svn: Repository moved permanently to 'http://svn.osgeo.org/geotools/'; please relocate
Show
Jody Garnett added a comment - The good news is that I have been able to complete the work without breaking any API; I accomplished this by turning MapContext into a class - that extends MapContent. Yes MapContent was the safe name I found to replace "Map". In a similar manner MapLayer is now a class; and extra care has been taken to make it a pure wrapper that does not hold any state. There is a difference between DefaultMapContext and MapContext - DefaultMapContext is willing to create default styles; and generate a default bounds for the map as layers are added. Created final patch; unable to commit onto a branch as I am having the following svn error: %> svn cp . http://svn.osgeo.org/geotools/branches/map-context -m "final patch, does not break api" svn: Repository moved permanently to ' http://svn.osgeo.org/geotools/'; please relocate
Hide
Permalink
Jody Garnett added a comment -
MapContext and Layer are ready to go
Show
Jody Garnett added a comment - MapContext and Layer are ready to go

People

  • Assignee:
    Jody Garnett
    Reporter:
    Jody Garnett
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: