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

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: