Repository navigation
Query rendered features from tessella.h - #360
Merged
Merged
Conversation
The last of #338. ```c tessella_result tessella_query_rendered_features(const tessella_map*, double x0, double y0, double x1, double y1, const uint8_t* const* layer_ids, const size_t* layer_lens, size_t layer_count, uint8_t* out, size_t cap, size_t* out_len); ``` Byte ranges rather than C strings, for the reason nothing else here takes one, and parallel arrays so each is contiguous. A `layer_count` of zero considers every layer. `web/map.js` gets `queryRenderedFeatures(x0, y0, x1, y1, layers)`, where equal corners default from the first pair so a tap is `queryRenderedFeatures(event.offsetX, event.offsetY)`. ## A doc from #358, corrected That PR said `Map::query_rendered_features` took its pixels "in the same space `tessella_screen_to_geo` uses". It does not, and the difference is the one that matters: the Rust call measures y **up from the bottom edge**, because the unprojection under it is `from_screen_detail`, which works in that convention; `tessella_screen_to_geo` takes a host's top-down y and flips. Measured to settle it -- at a camera on 51.505, `to_screen` puts 51.515 at y 571 and 51.495 at y 197 of a 768-pixel viewport, so a more *northern* coordinate has a *larger* y. So the C entry point flips, as its sibling does, and the Rust doc now says which convention it is in. This is the one thing about the call that cannot come out right by accident, so all three test layers attack it the same way: the fixture's feature is deliberately **off center**, and each layer asks about both the point and its mirror across the middle of the viewport. A feature in the center is its own mirror and would pass either way, which is why the distance between the two is asserted rather than assumed. ## The answer A GeoJSON `FeatureCollection`, topmost first, written without a terminator with `out_len` set. `layer`, `source`, `sourceLayer` and `geometryType` sit beside `properties`, where mbgl's own query puts them, so a host that has read those docs finds them where it expects. `geometry` is always null. A record names a range of extruded, clipped, tile-local vertices, and turning that back into a source geometry would hand a host a different shape from the data it already has -- which it can key by the id this returns. #338 asked for "properties, source and layer", and that is what this carries. `TESSELLA_TOO_SMALL` is 19 and is not a truncation: half a JSON document is a syntax error rather than a smaller answer, so nothing is written and the length is reported. One call to size, one to fill. ## Mutations | mutation | caught by | | --- | --- | | the y flip dropped | three of four FFI tests | | the layer filter never passed through | the filter test | | `TooSmall` never reported, so a short buffer truncates | all four | | `out_len` never set | all four | | the JS length array aimed at the pointer array | the JS query test | | the JS layer count dropped | the JS query test | | the JS reported length ignored, decoding the whole buffer | the JS query test | And the C surface checks the ABI itself: the sizing call, a four-byte buffer leaving the buffer untouched, a named layer as a byte range, and the four refusals. ## The fixture, and two wrong ways to wait for it The FFI test waits for the query to answer, which is the condition every assertion in it needs. Two counts were tried first and both were wrong. A fixed forty ticks read an empty answer because the *sources* had not resolved. Waiting for readiness 2 and then ticking eight more times passed here and failed CI -- resolved is not drawn, because a GeoJSON source is still being cut into tiles after its sources are known. The staircase is readiness 2, then cover built, then cover drawn, then the thing asserted; each is a later state than the last and only the final one is what a test wants. The wait is asserted once in the setup, so a fixture that stops drawing says so plainly instead of four times as "the feature was not named". Proven both ways: with the deadline cut to zero the test fails with the setup's message rather than the assertion's, and twelve concurrent copies pass. ## The sweep's three moving rows `hill_p` z14 p0 read 1, `hill_p` z11 p0 read 14 and `terrain_flat_p` z14 p60 read 28, against 0, 0 and 29 on the previous run. All three are the rows `sweep.sh`'s own header documents as unstable, and all three settle to the documented value when run alone -- 0, 0 and 29, twice each. It also cannot be this change: the diff is one test file. Closes #338 Signed-off-by: Joel Winarske <joel.winarske@linux.com>
jwinarske
force-pushed
the
query-from-c
branch
from
October 7, 2026 15:05
74c034e to
d9811c0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The last of #338.
Byte ranges rather than C strings, for the reason nothing else here takes one, and parallel arrays
so each is contiguous. A
layer_countof zero considers every layer.web/map.jsgetsqueryRenderedFeatures(x0, y0, x1, y1, layers), where equal corners default from the first pair soa tap is
queryRenderedFeatures(event.offsetX, event.offsetY).A doc from #358, corrected
That PR said
Map::query_rendered_featurestook its pixels "in the same spacetessella_screen_to_geouses". It does not, and the difference is the one that matters: the Rust call measures y up from the
bottom edge, because the unprojection under it is
from_screen_detail, which works in thatconvention;
tessella_screen_to_geotakes a host's top-down y and flips. Measured to settle it -- at acamera on 51.505,
to_screenputs 51.515 at y 571 and 51.495 at y 197 of a 768-pixel viewport, so amore northern coordinate has a larger y.
So the C entry point flips, as its sibling does, and the Rust doc now says which convention it is in.
This is the one thing about the call that cannot come out right by accident, so all three test layers
attack it the same way: the fixture's feature is deliberately off center, and each layer asks about
both the point and its mirror across the middle of the viewport. A feature in the center is its own
mirror and would pass either way, which is why the distance between the two is asserted rather than
assumed.
The answer
A GeoJSON
FeatureCollection, topmost first, written without a terminator without_lenset.layer,source,sourceLayerandgeometryTypesit besideproperties, where mbgl's own queryputs them, so a host that has read those docs finds them where it expects.
geometryis always null. A record names a range of extruded, clipped, tile-local vertices, andturning that back into a source geometry would hand a host a different shape from the data it already
has -- which it can key by the id this returns. #338 asked for "properties, source and layer", and
that is what this carries.
TESSELLA_TOO_SMALLis 19 and is not a truncation: half a JSON document is a syntax error ratherthan a smaller answer, so nothing is written and the length is reported. One call to size, one to
fill.
Mutations
TooSmallnever reported, so a short buffer truncatesout_lennever setAnd the C surface checks the ABI itself: the sizing call, a four-byte buffer leaving the buffer
untouched, a named layer as a byte range, and the four refusals.
The fixture, and two wrong ways to wait for it
The FFI test waits for the query to answer, which is the condition every assertion in it needs. Two
counts were tried first and both were wrong. A fixed forty ticks read an empty answer because the
sources had not resolved. Waiting for readiness 2 and then ticking eight more times passed here and
failed CI -- resolved is not drawn, because a GeoJSON source is still being cut into tiles after its
sources are known.
The staircase is readiness 2, then cover built, then cover drawn, then the thing asserted; each is a
later state than the last and only the final one is what a test wants. The wait is asserted once in
the setup, so a fixture that stops drawing says so plainly instead of four times as "the feature was
not named".
Proven both ways: with the deadline cut to zero the test fails with the setup's message rather than
the assertion's, and twelve concurrent copies pass.
The sweep's three moving rows
hill_pz14 p0 read 1,hill_pz11 p0 read 14 andterrain_flat_pz14 p60 read 28, against 0, 0and 29 on the previous run. All three are the rows
sweep.sh's own header documents as unstable, andall three settle to the documented value when run alone -- 0, 0 and 29, twice each. It also cannot be
this change: the diff is one test file.
Closes #338
What #338 came to, across six slices
web/map.jsLimits, all stated in the header rather than left to be found: under terrain the answer is for where
the ground would have been, because the rectangle is unprojected onto the plane; a globe answers
nothing rather than something wrong; and a feature with no id can be answered twice, because a tile is
built from a buffered box and dedupe needs a name.
Gate: clippy pinned and stable, rustdoc on default features and all features, the wasm32 no_std lane
over all six crates, the wasm32 module built and its ABI checked, 2328 tests, five JS tests,
benches built and run, fmt last then clippy again, and the pixel gate -- quad
22 / 22 / 22 / 32with
image_stable 1.One honest note on that gate. The last sweep here ran without
build.shin front of it, which isthe rule and I skipped it. It happened not to matter -- the probe links the producer's static library,
the diff at that point was a single test file, and rebuilding afterwards produced a byte-identical
render_probe(4fcdb2cc…), so the sweep did measure the current binary. Verified rather thanassumed, because "it cannot have mattered" is the assumption that rule exists to check.
Closes #338