developmentseed / developmentseed/deck.gl-raster

Remove CutlineBbox module in favor of upstream ClipExtension?

Open
#561 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
228
Forks
29
Avg merge
12h 27m
Merged PRs (30d)
4

Description

I don't think we need both

----

## `CutlineBbox` vs deck.gl's `ClipExtension`

**They do the same job, in different reference frames — both correct under auto-offset.**

- `CutlineBbox` captures the raw `positions.xy` attribute (absolute common space) and
compares against an absolute-common-space bbox that's projected **once** at definition
time (pass-through `getUniforms`).
- `ClipExtension` (fragment path) captures `geometry.position.xy`, which under
`WEB_MERCATOR_AUTO_OFFSET` (zoom ≥ 12) is **camera-relative / offset** common space
(`project.glsl` subtracts `coordinateOrigin`). Its `draw()` then re-projects `clipBounds`
through `this.projectPosition(...)`, and `project-functions.js` applies the *same* offset
subtraction in `offsetMode`. So both sides carry the offset and it cancels — clipping
stays correct at every zoom.

The `CutlineBbox` comment ("capture raw positions to keep the test stable across zoom") was
avoiding the failure mode of comparing offset positions against an *un-offset* bbox.
`ClipExtension` simply offsets both sides instead. Both are valid; if anything
`ClipExtension`'s small camera-relative magnitudes are marginally more float-friendly at the
clip edge.

**`ClipExtension` already works end-to-end on `COGLayer`** — the vermont example proves it
(`extensions: [new ClipExtension()]` is auto-forwarded down the composite hierarchy to the
mesh sublayer).

**What `CutlineBbox` does that `ClipExtension` doesn't:** essentially nothing for this use
case. Both handle only axis-aligned rectangles (USGS quads are axis-aligned in common space).
The only differences are ergonomic:

| | CutlineBbox | ClipExtension |
|---|---|---|
| API | typed `RasterModule` in `renderPipeline` | `extensions` + `clipBounds`, needs `@ts-expect-error` |
| Gotcha | none | must set `clipByInstance: false` (auto-detect sees `instancePositions`) |
| Per-frame | pass-through | 2-point reproject (negligible) |
| Maintained by | us | deck.gl |

**Recommendation: deprecate `CutlineBbox`.** deck.gl ships an equivalent we already use;
maintaining a redundant shader module + its tests + docs isn't worth it, and we're pre-1.0 so
removing a `gpu-modules` export is cheap. The only wart on the `ClipExtension` side is the
`clipBounds` typing cast and the `clipByInstance: false` footgun — both of which we could
smooth with a tiny typed helper if we want.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.