feat: pure deck.gl renderer — replace MapLibre with MVTLayer basemap #178
Conversation
Add a new DeckGLRenderer that uses MapLibre GL standalone + deck.gl overlay for WebGL-accelerated entity rendering. Selectable via ?renderer=deckgl URL param; deck.gl bundle (~817KB) is only loaded when requested. Pre-work: extract shared constants (icon sizes/paths/states, transition duration, grid utils) from Leaflet-specific files into src/renderers/shared/ so both renderers can reuse them.
…ibre tuning - Replace markDirty() with per-collection dirty methods (dirtyEntities, dirtyLines, dirtyBriefing, dirtyPulses) and revision counters - Cache data arrays in DeckState to avoid Array.from() every frame - Use revision counters as updateTriggers instead of N-element arrays - Add MapLibre ScaleControl, maxBounds, renderWorldCopies: false, fadeDuration: 0, maxTileCacheSize: 256, collectResourceTiming: false - Enable interleaved: true for camera-synced rendering - Add CSS offsets for MapLibre control containers matching panel layout
Eliminate MapLibre GL entirely from the deck.gl renderer path — vector tiles are now rendered directly via deck.gl TileLayer + MVTLoader, and raster tiles via TileLayer + BitmapLayer. New files: - deckgl-expressions.ts: compile MapLibre GL style expressions to JS fns - deckgl-style-parser.ts: parse topo.json into compiled layer definitions - deckgl-basemap.ts: MVTLayer, raster, background, sprite atlas builders - deckgl-scale-control.ts: HTML scale bar (deck.gl has no built-in one) Performance optimizations: - Batch vector sub-layers by geometry type (~6 per tile instead of ~30) - Stable getTileData reference preserves tile cache across zoom changes - RAF-batched flush on zoom change instead of synchronous flushNow() Deps: +@deck.gl/geo-layers, -@deck.gl/mapbox
Summary of ChangesHello @fank, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural change by integrating a new deck.gl-based map renderer. The primary goal is to enhance rendering performance and flexibility by moving away from MapLibre GL for basemap display. It achieves this by directly processing vector and raster tiles within deck.gl, compiling MapLibre styles for compatibility, and implementing several performance-focused optimizations. This change allows for a more modern and efficient rendering pipeline while maintaining the existing map styling definitions. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive and significant refactoring that replaces MapLibre GL with a pure deck.gl rendering path for the basemap. The changes are well-structured, introducing a new DeckGLRenderer and a suite of helper modules for style parsing, state management, and layer generation. I appreciate the clear focus on performance, with several key optimizations like batched sub-layers, stable tile data fetching, and RAF-batched rendering updates. The refactoring of shared utilities into a common directory is also a great improvement for code maintainability.
I've found a couple of potential rendering bugs in the new deck.gl implementation and a minor code duplication issue. My detailed feedback is in the review comments. Overall, this is a high-quality contribution that modernizes the rendering stack.
| let pixelOffset: [number, number] = [0, 0]; | ||
| if (textOffset) { | ||
| const off = textOffset({}, zoom); | ||
| if (Array.isArray(off)) pixelOffset = [off[0] * 16, off[1] * 16]; | ||
| } | ||
|
|
||
| for (const f of filtered) { | ||
| const text = textField(f.properties, zoom); | ||
| if (text != null && String(text) !== "") { | ||
| texts.push({ | ||
| position: f.geometry?.coordinates ?? [0, 0], | ||
| _text: String(text), | ||
| _size: evalNumber(textSize, f.properties, zoom, 14), | ||
| _color: evalColor(layer.paint["text-color"], f.properties, zoom, "#000"), | ||
| _anchor: mappedAnchor, | ||
| _pixelOffset: pixelOffset, | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The calculation for pixelOffset seems incorrect. It's calculated once per style layer using a hardcoded font size of 16, but MapLibre's text-offset unit is em, which is relative to the text-size. Since text-size can be data-driven and is evaluated per-feature, the offset should also be calculated per-feature using the evaluated size. Additionally, textOffset itself is evaluated with empty properties, which would be incorrect if it's also data-driven.
for (const f of filtered) {
const text = textField(f.properties, zoom);
if (text != null && String(text) !== "") {
const size = evalNumber(textSize, f.properties, zoom, 14);
let pixelOffset: [number, number] = [0, 0];
if (textOffset) {
const off = textOffset(f.properties, zoom);
if (Array.isArray(off)) {
pixelOffset = [off[0] * size, off[1] * size];
}
}
texts.push({
position: f.geometry?.coordinates ?? [0, 0],
_text: String(text),
_size: size,
_color: evalColor(layer.paint["text-color"], f.properties, zoom, "#000"),
_anchor: mappedAnchor,
_pixelOffset: pixelOffset,
});
}
}| ])); | ||
| } | ||
| poly.polygon = ring; | ||
| poly.fillColor[3] = Math.round(Math.min(poly.fillColor[3] / 255, state.alpha) * 255); |
There was a problem hiding this comment.
The logic for updating the polygon's fill opacity appears to be stateful in a way that might be unintentional. It uses the polygon's current alpha (poly.fillColor[3]) in the calculation. This means if state.alpha ever decreases, the polygon's alpha will be permanently capped at that lower value, even if state.alpha increases again in subsequent frames. The calculation should likely use the base fill opacity defined by the marker's brush type, which is stored in internal.shapeOpts.
| poly.fillColor[3] = Math.round(Math.min(poly.fillColor[3] / 255, state.alpha) * 255); | |
| poly.fillColor[3] = Math.round(Math.min(internal.shapeOpts.fillOpacity, state.alpha) * 255); |
| cy + sin * dx + cos * dy, | ||
| ]), | ||
| ); | ||
| poly.fillColor[3] = Math.round(Math.min(poly.fillColor[3] / 255, state.alpha) * 255); |
There was a problem hiding this comment.
Similar to the ellipse shape, the logic for updating the rectangle's fill opacity appears to be stateful in a way that might be unintentional. It uses the polygon's current alpha (poly.fillColor[3]) in the calculation. This means if state.alpha ever decreases, the polygon's alpha will be permanently capped at that lower value. The calculation should use the base fill opacity defined by the marker's brush type from internal.shapeOpts.
| poly.fillColor[3] = Math.round(Math.min(poly.fillColor[3] / 255, state.alpha) * 255); | |
| poly.fillColor[3] = Math.round(Math.min(internal.shapeOpts.fillOpacity, state.alpha) * 255); |
| export function hexToRGBA(hex: string, alpha = 1): [number, number, number, number] { | ||
| let r = 0, g = 0, b = 0; | ||
| const h = hex.replace("#", ""); | ||
| if (h.length === 3) { | ||
| r = parseInt(h[0] + h[0], 16); | ||
| g = parseInt(h[1] + h[1], 16); | ||
| b = parseInt(h[2] + h[2], 16); | ||
| } else if (h.length >= 6) { | ||
| r = parseInt(h.slice(0, 2), 16); | ||
| g = parseInt(h.slice(2, 4), 16); | ||
| b = parseInt(h.slice(4, 6), 16); | ||
| } | ||
| return [r, g, b, Math.round(alpha * 255)]; | ||
| } |
There was a problem hiding this comment.
This hexToRGBA function duplicates color parsing logic found in deckgl-expressions.ts. The version in deckgl-expressions.ts (parseCssColor) is more comprehensive, handling rgb(), rgba(), and 8-digit hex values with alpha. To avoid code duplication and potential inconsistencies, consider using a single, robust color parsing utility. You could move parseCssColor to a shared utility file and use it here.
1 participant
Summary
TileLayer+@loaders.gl/mvt, raster tiles viaTileLayer+BitmapLayertopo.json) to JS accessor functions at load time, so the existing style pipeline (styles.go) requires no changesgetTileDatareference to preserve tile cache across zoom, and RAF-batched flush instead of synchronousflushNow()during zoom gesturesNew files
deckgl-expressions.ts["interpolate", ...]) to(props, zoom) => valuefunctionsdeckgl-style-parser.tstopo.jsonstyle document into compiled layer definitionsdeckgl-basemap.tsdeckgl-scale-control.tsWhat we lose (acceptable)
symbol-placement: "line")sans-seriffont usedTest plan
npx vite build— no errors, deck.gl still code-split (~911KB chunk)npx vitest run— all 673 tests pass?renderer=leafletstill works (MapLibre unchanged for Leaflet path)