Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions crates/tessella-style/src/expression/evaluate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,20 @@ pub trait Feature {
fn properties(&self) -> Value {
Value::Null
}

/// Per-feature state a host set, by key, or `None` for a feature with none.
///
/// Not a property: it is not in the tile and not in the style, and it changes while the tile
/// it belongs to stays as it is -- a hover, a selection, a route segment marked. The default
/// answers `None` for every key, which is what a feature with no state means and what every
/// implementation that predates the operator keeps answering.
///
/// Keyed by the caller however it likes. The specification keys state by source, source layer
/// and feature id, and a feature here already knows which of those it is.
fn state(&self, key: &str) -> Option<Value> {
let _ = key;
None
}
}

/// A feature's coordinates, as the geometry operators read them.
Expand Down Expand Up @@ -783,6 +797,13 @@ pub(super) fn evaluate(expr: &Expr, context: &Context<'_>) -> Result<Value, Eval
None => Ok(context.feature()?.property(&key).unwrap_or(Value::Null)),
}
}
Expr::FeatureState { key } => {
let key = expect_str(key, context)?;
// Absent state is null, as an absent property is, and for the same reason: a style
// reads it through `case` or `coalesce` against a default. Erroring instead would
// make every unhighlighted feature a failed evaluation.
Ok(context.feature()?.state(&key).unwrap_or(Value::Null))
}
Expr::Has { key, object } => {
let key = expect_str(key, context)?;
match object {
Expand Down
61 changes: 59 additions & 2 deletions crates/tessella-style/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ impl Dependency {
/// Re-evaluated per frame. Never cached across a zoom interval, because it changes inside
/// one.
pub const CAMERA: Self = Self(1 << 2);
/// Per-feature state a host set, which is not in the tile and not in the style.
///
/// Always joined with [`Self::FEATURE`], because a state lookup has to know *which* feature,
/// and never on its own. The bit is separate anyway, because the two change on different
/// occasions and at different costs: a feature's properties arrive with the tile and are
/// fixed for its life, while its state is set by a host mid-frame -- a hover, a selection, a
/// route segment marked -- and what has to be redone for a change is less than a relayout.
///
/// mbgl keeps the same distinction for the same reason, as `Dependency::FeatureState`.
pub const STATE: Self = Self(1 << 3);

/// The least dependency covering both.
#[must_use]
Expand Down Expand Up @@ -110,6 +120,18 @@ impl Dependency {
pub const fn needs_camera(self) -> bool {
self.0 & Self::CAMERA.0 != 0
}

/// True when per-feature state is involved.
///
/// What a caller does with this is refuse or re-evaluate. The specification allows
/// `["feature-state", …]` in a *paint* property only -- a filter decides which features exist
/// at all and a layout property decides their geometry, and neither can depend on something
/// a host changes without the tile being rebuilt -- so a filter or a layout property reading
/// this is refused where it is compiled.
#[must_use]
pub const fn needs_state(self) -> bool {
self.0 & Self::STATE.0 != 0
}
}

/// A `["collator", …]`, whose members are themselves expressions.
Expand Down Expand Up @@ -841,6 +863,18 @@ pub enum Expr {
/// The object tested, or `None` to test the feature.
object: Option<Box<Expr>>,
},
/// Per-feature state a host set: `["feature-state", key]`.
///
/// Not a property and not in the style. A host sets it to mark a feature -- hovered,
/// selected, part of a route -- and a paint property reads it, which is what lets the
/// highlight change without the tile being rebuilt from its bytes.
///
/// Absent state is `null`, as an absent property is: a style reads it through `case` or
/// `coalesce` against a default, which is the idiom the specification's own examples use.
FeatureState {
/// State key.
key: Box<Expr>,
},
/// The feature's geometry type: `Point`, `LineString` or `Polygon`.
GeometryType,
/// The feature's id.
Expand Down Expand Up @@ -1242,14 +1276,29 @@ impl Expression {
///
/// As [`Expression::parse`].
pub fn parse_filter(value: &Value) -> Result<Self, ParseError> {
Self::parse_rooted(
let parsed = Self::parse_rooted(
value,
&PropertySpec {
default: None,
expected: None,
},
false,
)
)?;
// A filter decides which features exist at all, and what the index built from it holds.
// State is set by a host between frames, so a filter reading it would say that a hover
// changes which features are in the tile -- which is not a thing the tile can answer, and
// not a thing this would do: the filter runs once at build and the answer would simply be
// stale. The specification allows the operator in a paint property only, and mbgl refuses
// it here too.
if parsed.dependency().needs_state() {
return Err(ParseError::Malformed {
operator: "feature-state".into(),
detail: "a filter cannot read feature state; the specification allows it in a \
paint property only"
.into(),
});
}
Ok(parsed)
}

fn parse_rooted(
Expand Down Expand Up @@ -2102,6 +2151,7 @@ fn children(expr: &Expr) -> Vec<&Expr> {
out.extend(object.as_deref());
out
}
Expr::FeatureState { key } => alloc::vec![&**key],
Expr::Compare { lhs, rhs, .. } => alloc::vec![&**lhs, &**rhs],
#[cfg(feature = "collator")]
Expr::CompareWith {
Expand Down Expand Up @@ -2291,6 +2341,13 @@ fn classify(expr: &Expr) -> Dependency {
Some(object) => classify(key).join(classify(object)),
None => Dependency::FEATURE.join(classify(key)),
},
// Both bits. The feature, because a state lookup has to know whose state; and the state,
// because what has to be redone when it changes is not what has to be redone when a
// tile's features change. A caller that only looked at `needs_feature` still binds this
// as a per-vertex attribute, which is correct and is the behavior that existed before.
Expr::FeatureState { key } => Dependency::FEATURE
.join(Dependency::STATE)
.join(classify(key)),
// The join over whatever the options read, which is the feature in the specification's
// own case: it takes the locale and the digit bounds off the feature's tags.
Expr::NumberFormat { .. } => children(expr)
Expand Down
6 changes: 6 additions & 0 deletions crates/tessella-style/src/expression/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,12 @@ fn parse_rooted(
expect_arity(operator, args, 0, 0)?;
Ok(Expr::Properties)
}
"feature-state" => {
expect_arity(operator, args, 1, 1)?;
Ok(Expr::FeatureState {
key: Box::new(parse_in(&args[0], scope)?),
})
}
"get" => {
// With a second argument the lookup is in *that* object rather than in the feature,
// which also means the expression stops depending on the feature at all — the
Expand Down
50 changes: 44 additions & 6 deletions crates/tessella-style/src/property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,18 @@ pub enum PropertyError {
/// What the style wrote.
got: &'static str,
},
/// A layout property reads per-feature state, which the specification allows in a paint
/// property only.
///
/// Worth its own error for [`Self::NotDataDriven`]'s reason, and a sharper one: layout decides
/// a feature's geometry and is cut once when the tile is built, so a value a host changes
/// between frames has no way to reach it. Tolerating it would draw the state the tile happened
/// to be built with and never change again -- a highlight that works until the first pan.
#[error("`{property}` is a layout property and cannot read feature state")]
FeatureStateInLayout {
/// Property name.
property: String,
},
/// A property varies per feature that the spec does not allow to.
///
/// Worth its own error rather than being tolerated: a data-driven expression on a property
Expand Down Expand Up @@ -1127,10 +1139,18 @@ pub fn layout_value(
) -> Option<Value> {
match layer.layout.get(key)? {
PropertyValue::Literal(literal) => Some(literal.clone()),
PropertyValue::Expression(expression) => Expression::parse(expression.value())
.ok()?
.evaluate(Some(zoom), feature)
.ok(),
PropertyValue::Expression(expression) => {
let parsed = Expression::parse(expression.value()).ok()?;
// Layout cannot read per-feature state, for the reason `Half::Layout` gives: layout is
// cut once when the tile is built and state is set between frames. [`resolve_layout`]
// refuses it as an error; this answers `None`, which is the caller's own default --
// there is no error channel here, and the layer kinds that reach this function are
// exactly the ones that table has no specs for.
if parsed.dependency().needs_state() {
return None;
}
parsed.evaluate(Some(zoom), feature).ok()
}
}
}

Expand All @@ -1148,7 +1168,7 @@ pub fn resolve_paint(
layer: &Layer,
) -> Result<BTreeMap<&'static str, ResolvedProperty>, PropertyError> {
let specs = paint_specs(&layer.kind).unwrap_or(&[]);
let mut resolved = resolve(specs, &layer.paint)?;
let mut resolved = resolve(specs, &layer.paint, Half::Paint)?;
apply_layer_rules(layer, &mut resolved);
Ok(resolved)
}
Expand Down Expand Up @@ -1198,12 +1218,25 @@ pub fn resolve_layout(
layer: &Layer,
) -> Result<BTreeMap<&'static str, ResolvedProperty>, PropertyError> {
let specs = layout_specs(&layer.kind).unwrap_or(&[]);
resolve(specs, &layer.layout)
resolve(specs, &layer.layout, Half::Layout)
}

/// Which of a layer's two property maps is being resolved.
///
/// The only thing it decides is whether `["feature-state", …]` is allowed, which is the one rule
/// that differs between them: a paint property may read state, and a layout property may not --
/// layout decides a feature's geometry, which is cut once at build, so a value a host changes
/// between frames cannot reach it. The specification says so and mbgl refuses it the same way.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum Half {
Paint,
Layout,
}

fn resolve(
specs: &'static [PropertySpec],
written: &BTreeMap<String, PropertyValue>,
half: Half,
) -> Result<BTreeMap<&'static str, ResolvedProperty>, PropertyError> {
let mut resolved = BTreeMap::new();
for spec in specs {
Expand Down Expand Up @@ -1242,6 +1275,11 @@ fn resolve(
};

let dependency = expression.dependency();
if half == Half::Layout && dependency.needs_state() {
return Err(PropertyError::FeatureStateInLayout {
property: spec.name.to_string(),
});
}
if dependency.needs_feature() && !spec.data_driven {
return Err(PropertyError::NotDataDriven {
property: spec.name.to_string(),
Expand Down
Loading
Loading