Layers should have a setExtent method

[hobu]

To provide symmetry with the mapObj and alleviate some redundant coding, I would
like to see a layer have the ability to have its extent be set.  WMS layers
already have this capability (in the Metadata section), but others like SDE,
GDAL, OGR, and shape files do not (please correct me if I'm wrong).  I can think
of a couple of reasons why this might be helpful.

1.  I have a national map, with some layers that are national in coverage and
some that are states or localities.  A user might query for an area, and
MapScript must test that extent against all of the map layer's getExtent(s) to
see if they intersect before deciding to turn on or off a layer.  getExtent can
be very computationally expensive, and could be easily avoided if we've already
defined one in the mapfile.  

2.  This would allow MapServer to avoid opening a layer if the extent of the
mapObj and the extent of a layerObj aren't coincident.  Layers that are
db-backed and have a high connection cost can be avoided all together if their
extents don't cross.  We can get some cheap speed improvements on some cases.

I would be willing to take a crack at this, but I wouldn't know where to start.
   Ideas, pointers, and code examples would be greatly appreciated.

Howard

[fwarmerdam]

Howard, 

As discussed in IRC I think this would be a great idea.  Extent queries against
OGR datasources are sometimes very expensive.  Extent queries against raster
layers can also be challenging and expensive in some requests. 

Amoung other places, I would encourage you to update msWCSGetCoverageMetadata()
in mapwcs.c to use the EXTENT keyword value if an explicit wcs_extent keyword
isn't available in the metadata.  In fact, the other W*S services should also
use a provided extent if explicit metadata isn't provided and I think we should
be promoting the EXTENT keyword as the "proper" way of providing layer extents.

Other places to look might include anywhere that calls msLayerGetExtent(). 
However, msLayerGetExtent() is normally called after msLayerOpen() which we 
would want to avoid for originally determining which layers fall in the area
of interest.  So some restructuring of the main layer test loop might be 
called for. 

[hobu]

I have commited changes to CVS HEAD that allow a layer to have an extent set.

Here is a short synopsis of what I did (with the help of Sean and Frank on IRC):
- Modified mapfile.c so that it will capture an EXTENT object when it sees it in
a LAYER block
- Created a new function - msRectIsValid - in mapsearch.c (where other
Rect-related functions seem to be) that tests if an extent is valid.  
- Added the msRectIsValid prototype to map.h
- Used msRectIsValid to check all of the extents that are taken in in mapfile.c
(web, map, reference, layer). MapServer now complains if you give it an invalid
extent.
- Made sure that copyLayer copies the rectObj in mapcopy.c
- Updated msMapSetExtent in mapobject.c to use msRectIsValid
- Added a msLayerSetExtent method that mirrors msMapSetExtent (except for the
scale and geotransform stuff).
- The setting of a layer's extent to (-1.0, -1.0, -1.0, -1.0) using
msLayerSetExtent is a special case, and it will not complain of an error.
- Updated layer.i in SWIG to provide a setExtent method to layerObj
- Updated layer.i in SWIG to change the logic of getExtent to use the
mapfile-specified extent if it is there (and not reset to -1,-1,-1,-1).
- Updated test cases in Sean's python unit tests

[sdlime]

No biggie, but there is already already a macro to test extents, does the same
thing as the new function. It's called MS_VALID_EXTENT and is in map.h. Might
make sense to use it or change other parts of the code to use the function...

Steve

[hobu]

I found the macro about 10 minutes after checking in my code.  That is how these
things always go, I guess.

I propose that we use the newly created msRectIsValid function instead of the
macro.  The macro appears to me to be used four times -- one time in each of
mapfile.c, mapscale.c, mapserv.c and mapserv_fcgi.c.  The macro requires that
the caller separate out all of the members of the extent object that is being
tested.  The new method is more convienient (allows you to pass in just the
extent object), more explicitly named (IMO), and more flexible than the macro.

If I don't hear anything, I'll assume this is ok and drop the macro and replace
the calls to use the new function.

[dmorissette]

Or just change the macro to:

#define MS_VALID_RECT(rect) \
           ((rect)->minx < (rect)->maxx && (rect)->miny < (rect)->maxy)

The decision to use a macro or a function should also be based on whether the
function is called often (i.e. in a loop?) and whether or not it's worth saving
the overhead of the function call. I guess in this specific case the test isn't
done in critical areas so it doesn't really matter and we can use a function,
but the old programmer in me thinks that macros have their place too so we
shouldn't systematically get rid of them.

[sdlime]

The macro may be used in a few places where the exent is not stored as a rect 
for some reason...

[hobu]

We've had a couple of releases now where LAYER EXTENT has existed, but we
haven't fully taken advantage of it.  An excellent target for 5.0 would be to
make our usage of things like ows_extent/wms_extent LAYER/EXTENT consistent
and/or interchangeable.  

[sdlime]

Howard, are you going to do something with this one? Otherwise we should mark as 5.2 or FUTURE...

Steve

[hobu]

kicking it forward... Maybe TomK will be interested in doing this unification as part of his owscommon work.

[sdlime]

I'm not seeing any compelling reason to rush this into 5.2 unless someone else is feeling particularly motivated. Any comments?

Steve

[sdlime]

Passing on to 5.4...

Steve