Skip a paint that is already data-driven, not one that merely looks like an expression
exp-shepherd's copy of this pre-warm is better than mine in one way and accidentally right in another, and both are worth taking. BETTER: it leaves an existing expression alone instead of wrapping it. Mine wrapped regions-border's line-color — which arrives as a region-coloured match on a feature property — into a case around that match. No visual change, but a gratuitous rewrite of an expression that was already data-driven and had no variant left to warm. ACCIDENTALLY RIGHT: their test is "is it an array". For this layer that coincides with the right answer, because the only array here is that match. It is not the right test in general — a zoom expression like ['interpolate',['linear'],['zoom'],…] is also an array and is NOT data-driven, so skipping it would leave that property to compile its variant on the first gesture. That is the partial pre-warm this whole exercise exists to avoid, and it would measure as a failure with no visible cause. The test is now whether the expression reads the FEATURE — get, has, or feature-state, at any depth. Constants and zoom-only expressions get wrapped; anything already keyed on a feature is left exactly as the layer wrote it. The regions layers have no zoom expression today, so this changes nothing that runs right now; it means the rule is correct for the reason it is correct. Verified: gates:pre 13/13. Chained on 790d8838.
This commit is contained in:
parent
f83b3a195d
commit
e020e68bca
1 changed files with 12 additions and 1 deletions
|
|
@ -1117,11 +1117,22 @@ const REGION_IDLE_WIDTH = ['case', ['has', 'iso_3166_1_alpha_3'], 0.6, 0.6]
|
|||
function prewarmRegionPaint(m) {
|
||||
if (!m || !m.getLayer('regions-fill')) return
|
||||
const wrap = (v) => ['case', ['has', 'iso_3166_1_alpha_3'], v, v]
|
||||
// ★ SKIP WHAT IS ALREADY DATA-DRIVEN, and note the test is not "is it an array". exp-shepherd wrote
|
||||
// their copy to skip any array, reasoning that an expression is already data-driven so there is no
|
||||
// variant to warm. That is right for this layer — line-color arrives as a match on a feature property
|
||||
// — but it is right by accident: a ZOOM expression like ['interpolate',['linear'],['zoom'],…] is also
|
||||
// an array and is NOT data-driven, so skipping it would leave that property to compile its variant on
|
||||
// the first gesture and produce exactly the partial pre-warm this is trying to avoid.
|
||||
// The property that matters is whether the expression reads the FEATURE. Wrapping one that already
|
||||
// does is pointless anyway — it would rewrite a region-coloured match into a case around it, which is
|
||||
// what my first version did.
|
||||
const readsFeature = (e) => Array.isArray(e)
|
||||
&& (e[0] === 'get' || e[0] === 'has' || e[0] === 'feature-state' || e.some(readsFeature))
|
||||
const warm = (layer, prop) => {
|
||||
if (!m.getLayer(layer)) return
|
||||
const cur = m.getPaintProperty(layer, prop)
|
||||
if (cur === undefined || cur === null) return
|
||||
if (Array.isArray(cur) && cur[0] === 'case') return
|
||||
if (readsFeature(cur)) return
|
||||
m.setPaintProperty(layer, prop, wrap(cur))
|
||||
}
|
||||
try {
|
||||
|
|
|
|||
Loading…
Reference in a new issue