Set MapViewport CRS from first added layer if not already set

Michael Bedward added a comment - 19/Aug/11 5:50 AM Mmm... after saying this was easy I'm no longer so sure. The viewport can be created by a call to MapContent.getViewport() before any layers have been added. In that case, it will have its default CRS and the MapContent would have to "know" to set a new one when the first layer is added. Also, the user might set a viewport explicitly before adding any layers. In this case we probably don't want to change the CRS. What about a case where layers are added, then all removed, then added. Should CRS only be set the first time we go from no layers to one layer ? Multiple MapContents can share a single viewport and we don't want them all vying to change the CRS as layers are added to them. For now, it seems safest to not implement this feature but instead to require the viewport CRS to be set explicitly by the user.

Michael Bedward added a comment - 20/Aug/11 4:11 AM Had another idea... Add an "editable" flag to the viewport so that a client can lock it down or allow it to be changed

Jody Garnett added a comment - 20/Aug/11 4:35 AM Okay I think we got this a bit backwards :-) 1) The first layer added does not mean anything ... 2) let the user add all the layers one after another 3) the first time they ask for getViewport() ... a) check if we have one - i.e. somebody called setViewport( viewport ) -or- b) we review the layers and sort out a good default viewport bounds ... This approach makes adding a layer easy; and also makes getting / creating a viewport easy. ------------------- Now can I ask what is the cause of this bug report? Can you update the description with the scenario you you are trying to address? I figure you are either up to: -- startup -- I am going to assume it comes from JMapPane or JMapFrame; starting up from scratch; and trying to get a result on screen without additional user intervention - other than adding some layers... -- share -- Or you are trying the case where several MapContent instances are defined; and we want them to share a MapViewport; and we have a bit of fun creating one in order to start the sharing process?

Michael Bedward added a comment - 22/Aug/11 4:58 AM Thanks for the input Jody. I appreciate it because I've got lost in something that seems like it should be straightforward... One of the problems is that the getViewport method can, and in gt-swing frequently is, called before layers are added to a MapContent. E.g. user sets up a map pane with an empty MapContent, displays it, then adds layers via menu action or similar. The map pane calls MapContent.getViewport while initializing. So it is not just about inferring bounds / CRS when a new viewport (that code is in there and works well) but also concerns whether to modify an existing (default) viewport. > Now can I ask what is the cause of this bug report? Can you update the description > with the scenario you you are trying to address? It's not a bug report - it is flagged as an improvement. But yes, the description was murky - updated now. Yes, this issue is inspired by "startup" but needs to take into account "share".

Jody Garnett added a comment - 22/Aug/11 6:21 AM Okay cool! So what we have is a new scenario ... I think we have a couple choices... a) document that we ask people to set the bounds before bring it up screen (if they are going to go through the steps to "share" a viewport with multiple malcontent; they need to make the viewport by hand; and set it into each mapcontent right?) b) go for some magic mode - in this case if the JMapPane starts to try and interact with the map before there are any layers there is nothing smart the user can do (they cannot zoom to all; or zoom or change the CRS or anything can they?). In this case you could just default of something "normal" like the entire world; WGS84 etc... - or perhaps you want to detect the situation where the user adds the first layer and try and take defaults from that?

Michael Bedward added a comment - 23/Aug/11 7:12 AM Thanks Jody. I prefer option (b) as the default. It ensures that the user will get something based on their data even if they forget to set bounds explicitly. I guess you could require them to set the bounds by making that field null initially, but that seems unfriendly. It's not always the case that shared viewports will have been hand-made. The DefaultRenderingExecutor class in gt-swing deals with rendering layers into separate graphics by creating temp MapContents which all share the viewport of the original, and that viewport can easily be the default one. Meanwhile, we seem to have circled back to taking the CRS from the first Layer with a non-null CRS (if present). I think it would be good for MapContent to do that (rather than a client like JMapPane for instance). To prevent automagic changes to a viewport's CRS when it is shared I've added MapViewport.setEditable( boolean ) as per the sub-task GEOT-3815 .

Michael Bedward added a comment - 23/Aug/11 8:12 AM MapContent now sets a default CRS based on the first Layer with a non-null CRS in its layer list. If there is already a viewport it will be updated when a Layer with a CRS is added (unless the viewport is marked non-editable or already has an explicitly set CRS). If there is no viewport when layers are added, the viewport created on the first call to getViewport will be given the CRS found by getMaxBounds(). These two paths should both result in the same CRS being set (unit tests are in place to check this).

Michael Bedward added a comment - 18/Oct/11 6:16 AM Reopening to back-port this to 2.7.x branch

Michael Bedward added a comment - 18/Oct/11 6:25 AM closing again - will do back-port under its own issue