Allow MVT filtering using WMS facade #7011

SodOnBass · 2024-01-14T10:35:04Z

Example code to support #7010 discussion about filtering, through styles, MVT tile served by WMS 'facade'.

The main idea is to consider LAYER classgroup and/or resultcache to filter out features while rendering the tile.

geographika · 2024-01-18T08:06:47Z

@SodOnBass - to get the linting passing on this pull request you can run the following (assuming you have Python installed):

pip install pre-commit
# cd to the MapServer source directory
pre-commit run --all-files

We'd also need some tests to check this is working. I've had a look for MVT tests and it doesn't appear we currently have any - I'll take a look at adding a few basic cases.

I've also sent a post to the dev mailing list asking for feedback on this - https://lists.osgeo.org/pipermail/mapserver-dev/2024-January/017094.html

geographika · 2024-01-18T10:54:33Z

Update - there are tests for MVT added after the initial development at https://github.com/MapServer/MapServer/blob/main/msautotest/wxs/wms_mvt.map added in 6ab90ce

Any update would require a new tests for STYLE and FILTER to be added to this.

SodOnBass · 2024-01-18T19:04:41Z

@SodOnBass - to get the linting passing on this pull request you can run the following (assuming you have Python installed):
pip install pre-commit
# cd to the MapServer source directory
pre-commit run --all-files

Done and committed. Thank you.

rouault · 2024-01-21T18:55:11Z

src/mapmvt.c

+  if (!layer->resultcache)
+    return msLayerNextShape(layer, shape);
+
+  return msLayerGetShape(layer, shape,


there should be some validation that *iShape is in range before accessing layer->resultcache->results[]

@rouault : You are 100% correct.
@geographika : Thank you for taking care

geographika · 2024-01-21T20:43:31Z

I get some strange differences when running the new MVT filter test between Windows and Linux (the CI build, and local vagrant output).

The test is:

# RUN_PARMS: wms_mvt_filtered.mvt [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WMS&VERSION=1.1.0&REQUEST=GetMap&SRS=EPSG:3857&BBOX=-7514065.628545966,5009377.085697311,-6261721.357121638,6261721.357121639&WIDTH=256&HEIGHT=256&STYLES=&LAYERS=road&FILTER=<Filter><PropertyIsEqualTo><PropertyName>F_CODE</PropertyName><Literal>67</Literal></PropertyIsEqualTo></Filter>&FORMAT=application/vnd.mapbox-vector-tile" > [RESULT_DEVERSION]

The results on Windows are correct, but the Linux has an extra 2 features that should have been filtered out:

Feature in purple below:

The same filter as PNG output doesn't have this feature.

I can't think why there would be a difference between Windows and Linux MVT outputs - the other tests produce identical binary outputs for MVT.

rouault · 2024-01-21T20:50:29Z

I can't think why there would be a difference between Windows and Linux MVT outputs

GEOS version?

geographika · 2024-01-21T21:03:41Z

GEOS version?

Both the CI version and Windows SDK appear to use libgeos3.10.2. The filter is an attribute filter, and not a spatial one so not sure why this would affect it. The PNG on both platforms with the same querystring have identical results (without the additional features).

geographika · 2024-01-22T08:53:40Z

The logs for running the tests produced identical output for both platforms, but also hinted at an error picked up in the review by @rouault about the need to check the shapeindex is in the cache, otherwise an error is created in msSHPLayerGetShape (see log output below).
The additional feature however is still being returned so this still needs to be resolved. @SodOnBass if you get a chance to debug maybe you can see why the extra 2 features are returned.

[Mon Jan 22 07:48:41 2024].216502 FLTLayerApplyPlainFilterToLayer(): ([F_CODE] = 67), rect=-7511619.64364084,5011823.07060244,-6264167.34202676,6259275.37221651
[Mon Jan 22 07:48:41 2024].317374 msSHPLayerGetShape(): General error message. Invalid feature id.
[Mon Jan 22 07:48:41 2024].318499 mapserv request processing time (msLoadMap not incl.): 0.103s
[Mon Jan 22 07:48:41 2024].318507 msFreeMap(): freeing map at 0x5612795e0eb0.
[Mon Jan 22 07:48:41 2024].318538 freeLayer(): freeing layer at 0x5612798cf140.

SodOnBass · 2024-01-22T21:47:19Z

@geographika
I don't have access to a Windows setup right now. So far, on Linux, I was not able to reproduce the bug you are describing. I got rid of the 'Invalid feature id' error log with your shapeindex fix and produced the mvt file with the mvt_id=36 feature as the first one.

INFO: Open of `wms_mvt_filtered.mvt'
      using driver `MVT' successful.

Layer name: road
Geometry: Line String
Feature Count: 27
Extent: (481.000000, 1249.000000) - (2748.000000, 2663.000000)
Layer SRS WKT:
(unknown)
mvt_id: Integer64 (0.0)
FNODE_: String (0.0)
TNODE_: String (0.0)
LPOLY_: String (0.0)
RPOLY_: String (0.0)
LENGTH: String (0.0)
ROAD_: String (0.0)
ROAD_ID: String (0.0)
F_CODE: String (0.0)
NAME_E: String (0.0)
NAME_F: String (0.0)
OGRFeature(road):0
  mvt_id (Integer64) = 36
  FNODE_ (String) = 594
  TNODE_ (String) = 614
  LPOLY_ (String) = 3
  RPOLY_ (String) = 3
  LENGTH (String) = 21773.150
  ROAD_ (String) = 990

I have got 27 features in total.

[sod@sodol build]$ ogrinfo wms_mvt_filtered.mvt -al | grep '  mvt_id'
  mvt_id (Integer64) = 36
  mvt_id (Integer64) = 37
  mvt_id (Integer64) = 38
  mvt_id (Integer64) = 39
  mvt_id (Integer64) = 40
  mvt_id (Integer64) = 41
  mvt_id (Integer64) = 42
  mvt_id (Integer64) = 43
  mvt_id (Integer64) = 44
  mvt_id (Integer64) = 45
  mvt_id (Integer64) = 46
  mvt_id (Integer64) = 51
  mvt_id (Integer64) = 52
  mvt_id (Integer64) = 53
  mvt_id (Integer64) = 54
  mvt_id (Integer64) = 55
  mvt_id (Integer64) = 56
  mvt_id (Integer64) = 57
  mvt_id (Integer64) = 58
  mvt_id (Integer64) = 59
  mvt_id (Integer64) = 60
  mvt_id (Integer64) = 61
  mvt_id (Integer64) = 62
  mvt_id (Integer64) = 63
  mvt_id (Integer64) = 64
  mvt_id (Integer64) = 67
  mvt_id (Integer64) = 69

I could have miss something and will check again.

geographika · 2024-01-23T08:10:16Z

@SodOnBass - the Windows results look correct - it is the Linux results that appear to be off - both when I run it using vagrant, and also the MapServer CI - see that failing test at https://github.com/MapServer/MapServer/actions/runs/7608524103/job/20717857736#step:3:8542

My Windows builds use MapServer version 8.1-dev PROJ version 9.3 GDAL version 3.8.

The correct results are added to the test suite at https://github.com/SodOnBass/MapServer/blob/eb6c5d2bda46880c6938f07dce491241e8b8ed7d/msautotest/wxs/expected/wms_mvt_filtered.mvt

Your results are different again than mine on Linux and appear to be missing one feature. There should be 28 results returned by the filter, and so are off by one. You should also get the following feature:

OGRFeature(road):27
  mvt_id (Integer64) = 71
  FNODE_ (String) = 754
  TNODE_ (String) = 785
  LPOLY_ (String) = 0
  RPOLY_ (String) = 0
  LENGTH (String) = 17371.988
  ROAD_ (String) = 1523
  ROAD_ID (String) = 1523
  F_CODE (String) = 67
  NAME_E (String) = Trans-Canada Highway
  NAME_F (String) = route Transcanadienne
  LINESTRING (1740 2358,1739 2354,1729 2296,1721 2280)

When I run on vagrant I get 30 features, including 2 where the F_CODE doesn't match the filter of 67:

OGRFeature(road):28
  mvt_id (Integer64) = 0
  FNODE_ (String) = 579
  TNODE_ (String) = 612
  LPOLY_ (String) = 3
  RPOLY_ (String) = 3
  LENGTH (String) = 28781.506
  ROAD_ (String) = 21
  ROAD_ID (String) = 21
  F_CODE (String) = 74
  NAME_E (String) =
  NAME_F (String) =
  LINESTRING (2674 2716,2650 2675,2646 2669,2644 2664,2643 2657,2642 2648,2641 2641,2642 2636,2642 2631,2638 2622,2633 2617,2626 2611,2619 2606,2614 2602)

OGRFeature(road):29
  mvt_id (Integer64) = 0
  FNODE_ (String) = 579
  TNODE_ (String) = 612
  LPOLY_ (String) = 3
  RPOLY_ (String) = 3
  LENGTH (String) = 28781.506
  ROAD_ (String) = 21
  ROAD_ID (String) = 21
  F_CODE (String) = 74
  NAME_E (String) =
  NAME_F (String) =
  LINESTRING (2674 2716,2650 2675,2646 2669,2644 2664,2643 2657,2642 2648,2641 2641,2642 2636,2642 2631,2638 2622,2633 2617,2626 2611,2619 2606,2614 2602)

The vagrant build uses the following dependencies - MapServer version 8.1-dev PROJ version 8.2 GDAL version 3.4 .
It could be a GDAL version difference, although all filtering is done in MapServer (and works fine on both platforms for PNGs with the same WMS request). It could also be a bug in existing msMVTWriteTile code that is only triggered when filtering.
Alternatively should the &(layer->resultcache->results[(*iShape) - 1])); include the -1? However that doesn't explain the different results between platforms.

rouault · 2024-01-23T20:47:58Z

src/mapmvt.c

+    return msLayerNextShape(layer, shape);
+
+  if ((*iShape) >= 0 && (*iShape) < layer->resultcache->numresults) {
+    (*iShape)++;


The increment should be done before the [0, numresults[ range check

SodOnBass · 2024-01-23T20:56:30Z

I managed to get the test OK by manipulating iShape after the sanity check. I did not spend time figuring out why it could work on Windows. Nevertheless, we now have a kind of ugly solution, hence the TODO in my comment. I created msMVTGetNextShape() to avoid complex branching in msMVTWriteTile() while statement. Consequently, I had to find a way to handle the current cache result index while iterating since there is no getNextShape() equivalent.

I guess I have to find a better way to handle 'cache result' versus 'no cache result' processing but this will probably involve more refactoring that I was initially expecting.

SodOnBass · 2024-01-26T21:24:51Z

@rouault @geographika
I revisited what I had done and finally trust the msLayerGetShapeCount comment saying it "should" be equivalent to calling msLayerWhichShapes and counting. I think the approach is overall better. Linux mvt tests passed.

rouault

It feels it is definitely on the right track. A few potential extra tunings possible. cf my 2 comments

rouault · 2024-01-26T23:17:34Z

src/mapmvt.c

@@ -543,12 +543,34 @@ int msMVTWriteTile(mapObj *map, int sendheaders) {
    features_size = FEATURES_INCREMENT_SIZE;

    msInitShape(&shape);
-    while ((status = msLayerNextShape(layer, &shape)) == MS_SUCCESS) {
+    int nshapes;
+    nshapes = layer->resultcache ? layer->resultcache->numresults


There is a potential performance implication in doing msLayerGetShapeCount() (when layer->resultcache == NULL). Couldn't we avoid it in that situation to just use msLayerNextShape() as before ?

Not sure if it is good practice, but we could entirely rely on msLayerGetShape returned value.

rouault · 2024-01-26T23:18:49Z

src/mapmvt.c

+        int nclasses = 0;
+        int *classgroup = NULL;
+
+        if (layer->classgroup && layer->numclasses > 0)


There might be a performance implication in doing msAllocateValidClassGroups / msFree within the loop. Looking at msQueryByFilter(), the call to msAllocateValidClassGroups is done outside the loop

Yes of course. I don't know why I didn't see it...

geographika · 2024-01-29T15:56:54Z

@SodOnBass - thanks for persisting with this and the refactors.
@rouault - if you're happy with the refactoring could you merge? I'm happy with the new functionality, and test cases.

rouault · 2024-01-29T16:08:16Z

yes, excellent work. Just squashed & merged

MapServer#7010 discussion support

309bfbd

SodOnBass mentioned this pull request Jan 14, 2024

Using LAYER CLASS filters for MVT tiles rendering #7010

Closed

geographika changed the title ~~#7010 discussion support~~ Allow MVT filtering using WMS facade - #7010 discussion support Jan 16, 2024

linting checks

2a8ca55

rouault reviewed Jan 21, 2024

View reviewed changes

geographika added 2 commits January 21, 2024 20:55

Add STYLES and FILTER tests for MVT tiles. Update comments.

eb6c5d2

Linting

da991ad

Check index before accessing cache

57bc3aa

fixed cache index handling

7940cfc

rouault reviewed Jan 23, 2024

View reviewed changes

jmckenna added this to the 8.2.0 Release milestone Jan 25, 2024

Convert while into for with shape count

5f9f864

rouault reviewed Jan 26, 2024

View reviewed changes

SodOnBass added 4 commits January 27, 2024 17:23

Performance concerns

0627e0b

‘classgroup’ may be used uninitialized

ac1c714

Relying on msLayerGetShape return was not good practice

57e51ac

Refactoring while loop in msMVTWriteTile

43a89c4

geographika changed the title ~~Allow MVT filtering using WMS facade - #7010 discussion support~~ Allow MVT filtering using WMS facade Jan 29, 2024

rouault merged commit da769fc into MapServer:main Jan 29, 2024
11 checks passed

SodOnBass deleted the Using-LAYER-CLASS-filters-for-MVT-tiles-rendering-#7010 branch February 3, 2024 08:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow MVT filtering using WMS facade #7011

Allow MVT filtering using WMS facade #7011

SodOnBass commented Jan 14, 2024

geographika commented Jan 18, 2024

geographika commented Jan 18, 2024

SodOnBass commented Jan 18, 2024

rouault Jan 21, 2024 •

edited

SodOnBass Jan 22, 2024

geographika commented Jan 21, 2024

rouault commented Jan 21, 2024

geographika commented Jan 21, 2024

geographika commented Jan 22, 2024

SodOnBass commented Jan 22, 2024

geographika commented Jan 23, 2024

rouault Jan 23, 2024

SodOnBass commented Jan 23, 2024

SodOnBass commented Jan 26, 2024

rouault left a comment

rouault Jan 26, 2024

SodOnBass Jan 27, 2024

rouault Jan 26, 2024

SodOnBass Jan 27, 2024

geographika commented Jan 29, 2024

rouault commented Jan 29, 2024

Allow MVT filtering using WMS facade #7011

Allow MVT filtering using WMS facade #7011

Conversation

SodOnBass commented Jan 14, 2024

geographika commented Jan 18, 2024

geographika commented Jan 18, 2024

SodOnBass commented Jan 18, 2024

rouault Jan 21, 2024 • edited

Choose a reason for hiding this comment

SodOnBass Jan 22, 2024

Choose a reason for hiding this comment

geographika commented Jan 21, 2024

rouault commented Jan 21, 2024

geographika commented Jan 21, 2024

geographika commented Jan 22, 2024

SodOnBass commented Jan 22, 2024

geographika commented Jan 23, 2024

rouault Jan 23, 2024

Choose a reason for hiding this comment

SodOnBass commented Jan 23, 2024

SodOnBass commented Jan 26, 2024

rouault left a comment

Choose a reason for hiding this comment

rouault Jan 26, 2024

Choose a reason for hiding this comment

SodOnBass Jan 27, 2024

Choose a reason for hiding this comment

rouault Jan 26, 2024

Choose a reason for hiding this comment

SodOnBass Jan 27, 2024

Choose a reason for hiding this comment

geographika commented Jan 29, 2024

rouault commented Jan 29, 2024

rouault Jan 21, 2024 •

edited