Add Feature Properties Transform #5343

neodescis · 2025-01-13T19:55:38Z

This is a furtherance of @wipfli's #4199, with the aim to solve #4198.

Things I've done since the original:

Tweaked transform function signature so it returns transformed properties instead of modifying them in situ.
Added Re-encoding of "rawTileData" using vtpbf after feature transforms to ship it back to the main thread. By doing this, queryRenderedFeatures() now works as expected.
Updated example to add a click event handler to demonstrate the functional queryRenderedFeatures() call, and also to transform a feature from a GeoJSON source.
Investigated transforming geometries as well. Unfortunately this is very complex given that a GeoJSON geometry would need to be converted back to an MVT geometry representation.

Launch Checklist

Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
Briefly describe the changes in this PR.
Link to related issues.
Include before/after visuals or gifs if this PR includes visual changes.
Write tests for all new functionality.
Document any changes to public APIs.
Post benchmark scores.
Add an entry to CHANGELOG.md under the ## main section.

* Alternative option * Improve agronomics, add test * Remove unwanted change

* Alternative option * Improve agronomics, add test * Remove unwanted change * Update browser.test.ts - fix test * Update land.json

src/source/vector_tile_worker_source.ts

wipfli · 2025-01-13T20:01:03Z

Thank you for taking this up @neodescis! I don't know if we need a getFeaturePropertiesTransform actually since the user always sets it. Regarding raw tile data I am not sure, but I would do the least amount of work for normal operations, and do some extra work if needed if someone calls queryRenderedFeatures.

codecov · 2025-01-13T20:04:02Z

Codecov Report

Attention: Patch coverage is 83.67347% with 8 lines in your changes missing coverage. Please review.

Project coverage is 91.83%. Comparing base (f1d58f2) to head (d16dbe5).
Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
src/source/worker.ts	20.00%	4 Missing ⚠️
src/source/worker_tile.ts	92.50%	3 Missing ⚠️
src/util/test/util.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5343      +/-   ##
==========================================
- Coverage   91.84%   91.83%   -0.02%     
==========================================
  Files         282      283       +1     
  Lines       38908    38955      +47     
  Branches     6827     6836       +9     
==========================================
+ Hits        35735    35773      +38     
- Misses       3046     3054       +8     
- Partials      127      128       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

neodescis · 2025-01-13T20:43:08Z

Thank you for taking this up @neodescis! I don't know if we need a getFeaturePropertiesTransform actually since the user always sets it. Regarding raw tile data I am not sure, but I would do the least amount of work for normal operations, and do some extra work if needed if someone calls queryRenderedFeatures.

I left the getter in for symmetry, but I can take it out if that's what's wanted. As for queryRenderedFeatures, I believe this updating of rawTileData needs to be done up-front, since the data needs to make its way back to the main thread (ultimately into the FeatureIndex), and queryRenderedFeatures is synchronous.

neodescis · 2025-01-13T21:02:19Z

src/source/worker_tile.ts

+        // If any features were transformed, re-encode to rawTileData to override the original
+        let transformedRawData: ArrayBuffer = null;
+        if (Object.keys(transformedFeatures).length) {
+            const transformedTile: VectorTile = {


This is a bit funky, but the VectorTile classes don't allow you to modify some of their data directly. Instead, this is overriding the layer's function that retrieves its features to get the transformed ones instead, so vtpbf can process using the updated data.

neodescis · 2025-01-14T06:04:00Z

I've spent some time investigating geometry overrides, and I don't think the effort is worth it:

The transformed geometries would still be limited to a given tile, which makes overriding them awkward.
For the API to be usable, an override would need to be in the form of a GeoJSON Geometry, but converting from that to the vector tile geometry format is complex and I haven't been able to find anything that can be leveraged to do it directly. geojson-vt is close, but its inner workings are not exposed.

neodescis · 2025-01-14T06:05:56Z

On the other hand, I've also looked at how this works with type: 'geojson' sources, and it seems to pretty much work the same as vector tiles, but the source layer is always set to "_geojsonTileLayer". So, I think that just leaves tests at this point.

neodescis · 2025-01-14T15:35:16Z

src/source/worker_tile.ts

@@ -99,6 +109,24 @@ export class WorkerTile {
            for (let index = 0; index < sourceLayer.length; index++) {
                const feature = sourceLayer.feature(index);
                const id = featureIndex.getId(feature, sourceLayerId);
+                if (WorkerTile.featurePropertiesTransform) {
+                    const {geometry} = feature.toGeoJSON(this.tileID.canonical.x, this.tileID.canonical.y, this.tileID.canonical.z);


I believe the z here should be canonical and not the overscaled value, since this is used to convert the MVT geometry to GeoJSON. Is that correct?

neodescis · 2025-01-14T17:52:41Z

@HarelM, I have tests working now. The example also works, but is maybe a little complex since it encompasses a GeoJSON source and vector tiles? Also, I'm not sure exactly how to document this API, since it's only available in the worker thread.

HarelM · 2025-01-14T18:33:04Z

I'll review this in the coming days thoroughly.
Can you elaborate why there's a need to re-encode the tiles?
More so, if this is the case, does it have better performance than addProtocol (which can be made to run in a worker thread as well)?

neodescis · 2025-01-14T19:50:09Z

Currently the "rawTileData" is shipped back to the main thread, and it is passed into the FeatureIndex there to be used by queryRenderedFeatures. The raw data is read every time queryRenderedFeatures is called. You can see this in FeatureIndex.query() which calls loadVTLayers().

So, if we want to transform features AND have queryRenderedFeatures return them as they have been transformed, the raw data needs to be updated.

Now, I fully recognize that, from a design perspective, we may not actually want to handle things this way. If we think that addProtocol is the appropriate path for achieving something like this, I'm fine with that. If that is the case, I would be happy to work up an example that does so in a separate pull request. I believe performance should be similar either way, since the vector tiles will need to be decoded, modified, and re-encoded in either approach. I have not yet tried addProtocol for my particular use case, so I haven't been able to compare them yet.

neodescis · 2025-01-15T00:05:09Z

Well, I went down the addProtocol route to do this instead, and I think I like it better. I guess I didn't realize that requests were dispatched back to the main thread to take advantage of a protocol added there. This is really nice, because then I don't need to import a script into the workers! I still have to parse and re-encode the vector tile, but it's much more centralized.

So, my recommendation would be to create an example that does this using addProtocol and abandon this (and @wipfli's) pull requests. I can create such a pull request in the coming days. The down side of this is that it doesn't really handle a geojson source, but the use case for that doesn't seem nearly as important anyway given that you can just update the geojson directly.

HarelM · 2025-01-15T05:53:24Z

I still think it would be great to have an example of addProtocol in a worker if we don't have such an example yet, but yes, addProtocol is very powerful.

neodescis · 2025-01-17T21:50:05Z

Closing this in favor of #5370

wipfli and others added 19 commits May 29, 2024 15:00

Add Feature Properties Transform

dbde2b5

Remove index.ts export, add featureID

cb352f2

Add style.json

2729f17

async

9d7122e

Remove style.json

f74bc8c

Merge main

b74b194

Move to separate file

f5ea1dc

Add example

69a1693

Vector tile transform changes (maplibre#341)

b90e195

* Alternative option * Improve agronomics, add test * Remove unwanted change

Merge branch 'main' into vector-tile-transform

5e0cbc3

Fix test (maplibre#342)

5435ffb

* Alternative option * Improve agronomics, add test * Remove unwanted change * Update browser.test.ts - fix test * Update land.json

Merge remote-tracking branch 'origin/main' into vector-tile-transform

a1f1d6c

Fix lint

1f1a1bb

Add same property test

5aceaed

Add example preview image

2b2d199

Merge branch 'main' into vector-tile-transform

2b07b88

Merge branch 'main' into vector-tile-transform

4964583

Switch example to use data URL

04e41bb

Get queryRenderedFeatures working

a6de152

neodescis requested review from birkskyum and HarelM January 13, 2025 19:55

neodescis commented Jan 13, 2025

View reviewed changes

src/source/vector_tile_worker_source.ts Outdated Show resolved Hide resolved

Remove getFeaturePropertiesTransform

c764bb3

neodescis commented Jan 13, 2025

View reviewed changes

neodescis added 2 commits January 13, 2025 22:53

Pass full geometry to transform function

a5264c6

Expand example with GeoJSON transform

33cf780

Example comments

d47f2ca

neodescis commented Jan 14, 2025

View reviewed changes

neodescis added 5 commits January 14, 2025 09:37

Update browser integration tests

5b6567d

Fix rawTileData logic

8dc5a7d

Use color names in example for clarity

6e37fc0

Add WorkerTile test

1deef2f

Update browser test to use blob URL

d16dbe5

neodescis marked this pull request as ready for review January 14, 2025 17:50

neodescis mentioned this pull request Jan 17, 2025

Add example using addProtocol to transform vector tile feature properties #5370

Merged

8 tasks

neodescis closed this Jan 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Feature Properties Transform #5343

Add Feature Properties Transform #5343

neodescis commented Jan 13, 2025 •

edited

Loading

wipfli commented Jan 13, 2025

codecov bot commented Jan 13, 2025 •

edited

Loading

neodescis commented Jan 13, 2025

neodescis Jan 13, 2025 •

edited

Loading

neodescis commented Jan 14, 2025

neodescis commented Jan 14, 2025 •

edited

Loading

neodescis Jan 14, 2025

neodescis commented Jan 14, 2025

HarelM commented Jan 14, 2025

neodescis commented Jan 14, 2025

neodescis commented Jan 15, 2025 •

edited

Loading

HarelM commented Jan 15, 2025

neodescis commented Jan 17, 2025

Add Feature Properties Transform #5343

Add Feature Properties Transform #5343

Conversation

neodescis commented Jan 13, 2025 • edited Loading

Launch Checklist

wipfli commented Jan 13, 2025

codecov bot commented Jan 13, 2025 • edited Loading

Codecov Report

neodescis commented Jan 13, 2025

neodescis Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

neodescis commented Jan 14, 2025

neodescis commented Jan 14, 2025 • edited Loading

neodescis Jan 14, 2025

Choose a reason for hiding this comment

neodescis commented Jan 14, 2025

HarelM commented Jan 14, 2025

neodescis commented Jan 14, 2025

neodescis commented Jan 15, 2025 • edited Loading

HarelM commented Jan 15, 2025

neodescis commented Jan 17, 2025

neodescis commented Jan 13, 2025 •

edited

Loading

codecov bot commented Jan 13, 2025 •

edited

Loading

neodescis Jan 13, 2025 •

edited

Loading

neodescis commented Jan 14, 2025 •

edited

Loading

neodescis commented Jan 15, 2025 •

edited

Loading