In getFeatureInfo, add an easy way to search all pixels in requested map

[mraross]

In the common getFeatureInfo scenario, a client app requests attributes of the feature under or near the cursor (see attached searchPixelsnearCursor.jpg). This is conveniently specified using &x, &y, and &radius parameters. In another, less common but important scenario, an client app requests attributes of all features within the current &width and &height of the requested map (see attached searchAllPixels.jpg). It isn't possible to specify this in terms of &x, &y, and &radius from some clients such as Google Earth since it requires a the runtime computation of &width/2,&height/2, etc. We propose an adding support for an additional boolean parameter called &searchAllPixels that, if set to true, means that &width and &height should be used to define the search extent instead of &x, &y, and &radius; if false, search extent is determined by &x, Y, and &radius. By default, &searchAllPixels=false.

[assefa]

I understand the issue but I am not sure how this will be received by others. We are going outside the specs of getfeatureinfo. I am thinking of maybe using a specific word like allmap for radius that could indicate to set query tolerance to the map->widtth/height.

[assefa]

Would this be acceptable if others agree?

[chodgson]

Support for radius=allmap

[chodgson]

This simple patch checks for radius=allmap and sets the query point to the center of the map and the radius to the smaller of half the width or half the height. It is important that the radius define a circle which is inscribed within the map rectangle because if features are selected that are not within the map, the [shplabel] template tag will not work (mraross is requesting this feature specifically for use with custom template-driven kml output).

[chodgson]

Note that it is not possible to support actually querying the whole map using a bbox rather than a radius query (while also supporting the feature_count limit), without either putting in a big inefficient hack, or implementing the fix to #2424.

[assefa]

will review/commit before 6.0

[chodgson]

New version of patch with rectangular query

[chodgson]

mraross decided that the circular query (as radius would normally be) is not acceptable and he wants allmap functionality to instead do a rectangular bbox query, while still maintaining the feature_count limit.

Because msQueryByRect doesn't support the query.maxresults limit, I implemented a new version which does (msQueryByRectWithLimit), and when radius=allmap, this function is called instead of msQueryByPoint. It is possible that msQueryByRectWithLimit could just replace the existing msQuerByRect, but I'm not sure if the query.maxresults limit may be set in other circumstances, causing unintended side effects, so this is a bit of an ugly copy/past hack. It uses the same inefficient two-stage query and then filter/limit technique as is used in msQueryByPoint, because I don't currently have the time or budget to look at fixing #2424 properly.

This feature isn't required for raster layers, so I haven't implemented a msQueryRasterByRectWithLimit - querying a raster layer with radius=allmap will ignore the feature_limit.

[assefa]

This is beyond the initial bug description. Adding other in cc to have their opinion.

[sdlime]

I guess I have the most heartburn with adding another query method. I suppose it would be ok to leave it in as a place holder now with the intent of removing it when proper maxresults support is added to ALL query functions. Now (6.0's) the time in some respects to do stuff like implement maxresults... I'd like Dan to weigh in as one of the WMS gurus.

For what it's worth I would just do radius=bbox. As I understand it that's what you're after, a bbox query. So it would make sense to let the radius reference the bbox parameter name.

Steve

[assefa]

any news on this?

[chodgson]

I think Steve was looking for some input or approval from Dan? I'm happy to change the name of the magic parameter from allmap to bbox.

The reason I added the additional query method was because I wasn't sure what side effects might occur if the existing queryByRect was changed to obey the maxfeatures limit which it ignored before... based on the fact that queryByRect is used to get all the features in the bbox for rendering, etc, it seemed likely that something could go wrong by just adding in the limiting code.

[dmorissette]

I'm fine with adding radius=bbox from a WMS perspective. Makes sense as a vendor-specific extension, and I think Assefa agrees with that too.

[sdlime]

I'd like to avoid a custom query function though. Can we rely on similar max results support that the WFS server has? That is, set layer->maxresults and rely on the drivers to respect that.

Steve

[dmorissette]

I agree that we should try to handle the max results the same way as WFS GetFeature does.

Who is taking the lead on this? It's getting quite late for 6.0 (technically we're past feature freeze and this doesn't qualify as a bug fix). Can we push this to 6.2?

[chodgson]

Dan, I'm doing this work.

In my testing, the WFS GetFeature max_features parameter doesn't actually work, if you also supply a bbox filter. Otherwise we could probably just use that. So there you go, now it's a bug ;) - however I wasn't actually addressing fixing this at this time. The "right" fix, as I mentioned before, is implementing more generic handling of all filtering, ideally passing it down to the drivers as per #2424. Unfortunately my client didn't really have the budget for that and also would really like it in 6.0.

Tell me what I can do to make that happen... I don't think #2424 is going to fly before 6.0. I'd like to see #2424 done though... seems like it could clean up a lot of things.

[chodgson]

OK so re-reading the latest comments on #2424 and following the link to #3739 it seems like these are heavily intertwined. Once all the drivers support maxfeatures then we can do this using the same query method as WFS GetFeatures does, getting rid of the ugly extra query method hack.

IIRC, when I started looking at this 2 months ago, I tested using WFS getfeature with max features and a bbox filter and one of the two was ignored, because msQueryBy... method that was called simply ignored that part of the filter. This was testing against a shapefile.

[sdlime]

If Chris readies a patch I can review/apply. While it is late in the game this functionality is isolated and doesn't impact anything existing.

Chris, we're looking for a patch against mapwms.c that handles the radius=bbox parameter. The patch should take take value for layer->maxfeatures from metadata as in WFS if available. I guess I'd look for wms_, ows_ or wfs_ maxfeatures in that order.

The query functions will rely on the drivers to support layer->maxfeatures. Shapefile, PostGIS and Oracle all do and those are the biggies. So I don't think there's a need for a specialized bbox query file.

We'll address #2424/#3739 across all drivers and certain output formats in 6.2.

I think this get you what your client needs if they work from drivers that support layer->maxfeatures.

Steve

[chodgson]

Patch implementing radius=bbox without additional query function

[chodgson]

The attached patch just checks for radius=bbox, and if it is set, calls msQueryByRect with rect=bbox. Also bypasses checking for the query point x/y parameters that are not required when using the bbox query.

Setting the feature_count will work for standard output formats, as long as the driver respects the layer->maxresults value. For my client mraross, they actually want to use a custom output template, and are using SDE which does not respect layer->maxresults. I found that setting the limit option in the [feature] template tag will provide them with the functionality they require.

Since WMS GetFeatureInfo doesn't currently check metadata feature limits (ows_maxfeatures or the like) I'm not sure that it makes sense for this specific form of GetFeatureInfo to do this. If you like, I can submit a separate patch which adds support for server-side limitation of the feature limit using ows_maxfeatures... however in WMS GetFeatureInfo the parameter is actually called feature_count... should we also check wms_max_feature_count or something like that? Or just leave this as is for now (6.0 release...).

[sdlime]

Chris: Patch looks reasonable and I applied in r11550. WMS tests run the same before and after.

Limiting features is all over the place isn't it. Let's leave it alone and we'll work up a comprehensive solution in an RFC for 6.2.

Steve