Skip to content

Mark a feature from tessella.h - #354

Merged
jwinarske merged 1 commit into
mainfrom
mark-a-feature
Oct 6, 2026
Merged

jwinarske merged 1 commit into
mainfrom
mark-a-feature

Conversation

@jwinarske

Copy link
Copy Markdown
Owner

The last of #337.

tessella_result tessella_set_feature_state(tessella_map*, const uint8_t* source, size_t,
                                           const uint8_t* source_layer, size_t,
                                           uint64_t feature_id,
                                           const uint8_t* state_json, size_t);
tessella_result tessella_clear_feature_state(tessella_map*);

Null state is the feature no longer marked, which a host has to be able to say: a feature's state is
replaced whole rather than merged per key, as mbgl's setFeatureState takes it, so a merge could not
remove anything. A state that is not a JSON object answers TESSELLA_BAD_FEATURE_STATE -- an object
is what a style reads keys out of, and an array names nothing.

The map owns the state, not the source

A source is shared by every map on its style and a hover is one map's, so two views of one basemap
highlight independently. That is also what keeps the tile shareable between them: it is built
without state, and stays that way.

So what applies the state is the frame. It re-paints the recorded features of a state-reading layer
as it encodes, from the buffer the bucket was built with -- PaintBinder::restated, the non-mutating
form of what #353 added for exactly this. Nothing is refetched, no tile is rebuilt, and no bucket is
touched.

The re-announce is the existing mechanism

The content stamp gains a fifth term: the host's state revision for the buckets that recorded
features, and zero for every other one -- exactly what pattern_rev beside it does for a sheet
arriving late. A mark therefore re-announces a highlight layer's drawables and leaves the rest of the
cover alone.

Read off the recorded features rather than off the paint, which matters for the layer #353
refuses to index: a highlight that also reads ["within", …] is not re-painted, so it must not be
re-announced either.

What the test asserts

The style has two fill layers over one source -- earth reads state, landuse does not:

  • marking feature 1 re-announces, with the camera unmoved;
  • it re-announces fewer drawables than the first frame did, which is the layer that reads no
    state not being touched;
  • server.requests() does not move, so no tile was rebuilt;
  • unmarking is the same shape, which is what makes a hover that moves affordable;
  • and a style that reads no state is wholly unaffected by a host that marks features anyway: zero
    re-announces, zero requests.

The fixture's earth layer carries two features and the second's id is 1 -- checked against the
tile with a throwaway probe rather than assumed, after the first version of this test marked an id
that named nothing and passed for the wrong reason.

mutation caught by
the stamp's state term always zero no re-announce
the stamp applied to every bucket, not just the indexed ones both tests -- the plain layer is re-announced too
the revision not bumped on a mark no re-announce

One thing this test does not do, and says so in its own docs: it does not read the attribute bytes
off the wire. Doing that means resolving a SlabRef through the region's slab table, which is the
consumer's job and is tested as such in web/; my first attempt at it read 96 bytes of zeros and
would have "passed" by comparing nothing with nothing. That the bytes themselves change is
tessella-layout's test, where a restated buffer is compared to a rebuilt one byte for byte.

The cost of the Frame field

Twenty-two files of one line each. Frame is a public struct literal with no default, so every test
that builds one names the new field. A Late bundle for fonts, patterns and state would be the tidier
shape if that ever matters again; I did not reshape it here because the churn is mechanical and the
compiler names every site.

Gate: clippy pinned and stable, rustdoc on default features and all features, the wasm32 check, the
no_std lane, 2282 tests, fmt last, and the pixel gate with probes rebuilt -- the sweep
byte-identical to the last run, zero holes, quad 22 / 22 / 22 / 32 with image_stable 1.

Closes #337

The last of #337.

    tessella_set_feature_state(map, source, len, source_layer, len, id, state_json, len)
    tessella_clear_feature_state(map)

Null state is the feature no longer marked, which a host has to be able
to say: a feature's state is replaced whole rather than merged per key,
as mbgl's setFeatureState takes it, so a merge could not remove
anything.

The map owns the state, not the source. A source is shared by every map
on its style and a hover is one map's, so two views of one basemap
highlight independently -- and a tile stays built without state, which
is what keeps it shareable between them.

So what applies the state is the frame: it re-paints the recorded
features of a state-reading layer as it encodes, from the buffer the
bucket was built with. Nothing is refetched, no tile is rebuilt, and the
bucket is not touched. `PaintBinder::restated` is the non-mutating form
of what #353 added for exactly this.

The re-announce is the existing mechanism rather than a new one. The
content stamp gains a state term, which is the host's revision for the
buckets that recorded features and zero for every other -- exactly what
`pattern_rev` beside it does for a sheet arriving late. A mark therefore
re-announces a highlight layer's drawables and leaves the rest of the
cover alone, which the test asserts by counting: the style has two fill
layers over one source, and a mark re-announces fewer drawables than the
first frame did.

Read off the recorded features rather than off the paint, which matters
for the layer #353 refuses to index: a highlight that also reads
`["within", …]` is not re-painted, so it is not re-announced either.

The cost of the Frame field is twenty-two files of one line each.
`Frame` is a public struct literal with no default, so every test that
builds one names the new field; a `Late` bundle for fonts, patterns and
state would be the tidier shape if that ever matters again.

Signed-off-by: Joel Winarske <joel.winarske@linux.com>
@jwinarske
jwinarske merged commit d7b9a67 into main Oct 6, 2026
9 checks passed
@jwinarske
jwinarske deleted the mark-a-feature branch October 6, 2026 18:31
jwinarske added a commit that referenced this pull request Oct 6, 2026
The second of #338's slices. #355 and #356 wrote down what each feature left in a bucket; this says
which of those records a tap or a box lands on, in one tile's own units. No map-level entry point
yet -- that is the next slice, which has the view transform and the cover.

## The paint has to be put back

#355 said a bucket's vertices are already extruded, so a range into them was closer to what was
drawn than mbgl's query is. That was wrong for every family but fill, and it is corrected on the
issue. `line.rs` writes `pos_normal: [p[0] * 2 | bit(round), p[1] * 2 | bit(up)]` -- the centerline,
doubled, with cap and side flags in the low bits -- and the extrusion arrives in the vertex shader
from `line-width`. `circle.rs` writes the center doubled plus a 0-or-1 corner bit, so all four
corners of a circle's quad sit within one tile unit of each other whatever `circle-radius` says.

So this does what `queryIntersectsFeature` does: grows the geometry by the paint. Evaluated against
the record, not read out of the binder -- the record carries the feature's properties and id, which
is what a data-driven expression reads, and it gives the same number without this having to know
each slot's byte encoding.

Per family: a fill and an extrusion are triangles against the region; a line is its centerline
within `line-width / 2`, or `line-gap-width / 2 + line-width` where a gap is set, which is mbgl's
`getLineWidth` and not half the width; a circle is its center within `circle-radius` plus
`circle-stroke-width`; a heatmap the same by `heatmap-radius`. A paint property is in screen pixels
and a region is in tile units, so the caller passes the scale between them -- `pixelsToTileUnits`.

## A line's shape comes out of the index buffer

Its vertices do not say which centerline points are joined. Two per point in emission order, and a
feature clipped into pieces leaves runs of them with nothing between, so connecting consecutive
points invents a segment from the end of one piece to the start of the next. In the fixture here
that phantom segment is four thousand units long, and a tap in the middle of it reports a road in
open water.

`segments` cannot break it either: `LineBucket::add_geometry` starts a new one only at the 64k
vertex cap, so one segment spans many features and many pieces.

The triangles can. Two centerline points are joined if and only if some triangle mentions a vertex
of each, because that triangle is the quad between them, and no triangle spans a gap. The triangles
are degenerate in position -- every vertex is on the centerline -- but their topology is the line's
shape, and that is what is read.

## Two bugs these tests caught

`real` shifted a fill's vertices as well. A fill is the one family that writes the tile coordinate
itself, so the shift halved every polygon and a tap inside a square found nothing. Split into `real`
and `plain`, with the reason written down: at a glance it reads as a projection bug.

And the first version of the two-pieces test asserted the wrong record. The fixture produces three,
one per world copy, and only the middle one carries both pieces -- the eastern copy holds the western
piece off-tile at x 10012 and the western copy the eastern piece at x -2048. The middle one is the
record that holds the gap, so it is the one the test needs, and it now says so.

## Mutations

| mutation | caught by |
| --- | --- |
| the width ignored, every line one unit wide | three tests |
| the gap ignored, always half the width | the gap test |
| the circle stroke ignored | the radius test |
| a fill vertex shifted like a packed one | two tests |
| `units_per_pixel` dropped from the line arm | the line scale test |
| line adjacency taken from consecutive vertices | the two-pieces test |

Two of those passed first time round. The scale one: the only assertion about it was over a circle,
so the line arm's conversion was unguarded, and there is now one per arm. And my first adjacency
mutation did not implement the thing it was meant to -- it allowed a degenerate pair rather than
replacing the index walk -- so it changed nothing and proved nothing. Rewritten to actually connect
consecutive vertices, it fails the two-pieces test and nothing else.

## And a flake in two FFI tests, which this PR exposed

CI's stable canary failed on `tessella-ffi`'s `feature_state.rs`, both tests reporting that nothing
was drawn. Those tests are #354's and this branch does not touch them; they passed on #354, #355 and
#356. What changed is the load -- a new test file more in the workspace run.

The cause is mine from #354. `settle` stopped after twelve consecutive quiet ticks whatever had
arrived, and twelve ticks is a hundred and twenty milliseconds. A cold map is quiet for as long as
its first tiles take, so on a loaded runner the loop returned `adds: 0` and the assertion read "the
first frames drew nothing" -- which is the message a real regression produces. Reproduced here, with
CI's two messages exactly, by setting the window to one tick.

`restyle.rs` from #349 has the same loop, on eight call sites.

The first fix was wrong and measuring it is what said so. Counting quiet only after any geometry or
view record still fails, because a restyle emits the old revision's removes and releases *before*
the new document's adds: a window closing between the two returns `adds: 0, removes: 8` and fails
the same way. So the condition belongs to the caller. `settle_until(seconds, wanted)` waits for what
the phase is about to assert -- `adds > 0`, or `adds > 0 && removes > 0`, or
`removes > 0 || releases > 0` -- and a phase whose claim is silence passes one that is never true and
waits out a short deadline, which is the honest answer for it.

Proven both ways. With the window cut to one tick, the shape that reproduced CI, all six tests pass.
With the caller's condition thrown away, they fail with CI's messages. The verdict no longer depends
on how long the window is.

## The sweep's one moving row

`terrain_flat_p` z14 p60 read 29, then 28, then 29 across three sweeps while the code only grew.
That row is documented in `sweep.sh`'s own header as having read 26 through 29, and run alone it
reads 29 three times out of three -- the other value appears only under a full sweep, where the
reconciliation can land after the frame the probe settled on. A number that moves both ways under
monotonic code is the scene, not the change.

It also cannot be this change. The only edit to a tracked file is one line of `lib.rs` adding the
module, nothing in the render path calls into it, and the rest is two new files.

Signed-off-by: Joel Winarske <joel.winarske@linux.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature-state is in the operator registry and nothing evaluates it

1 participant