Repository navigation
feat(layers): new layers go on top, web tile layers take a place among the map's layers (#808) - #1031
Conversation
…g the map's layers (opengeos#808)
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Combined map and tile stack crates/map/src/lib.rs, crates/map/src/tiles.rs |
Tile layers now store an optional over target. The map exposes combined-stack inspection, tile movement, placement, and placement preservation. New map layers are inserted at the top. |
Command and selection integration crates/engine/src/cmd/* |
Tile commands expose placement and movement. Add-layer commands request top placement. Layer removal releases tile placements. identify ranks map and tile results by drawing order. |
Positioned tile rendering crates/render/src/lib.rs, crates/render/src/tiles.rs |
Tile layers are drawn between map layers according to their slots. Rendering and tile-output tests cover placement behavior. |
UI, context, and validation crates/ui-egui/src/*, crates/assistant/src/*, crates/engine/src/tests.rs, docs/user-guide/* |
The Layers pane, ribbon controls, assistant context, documentation, and tests now represent and validate the combined drawing order. |
Priority:
Estimated code review effort: 4 (Complex) | ~60 minutes
Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant User
participant LayersPane
participant TilesCommand
participant Map
participant Renderer
User->>LayersPane: move tile layer
LayersPane->>TilesCommand: set target stack position
TilesCommand->>Map: place_tile
Map-->>TilesCommand: updated tile placement
Renderer->>Map: read combined stack
Map-->>Renderer: map layers and tile slots
Renderer-->>User: draw tiles at configured positions
Possibly related issues
- opengeos/RustGIS issue 808: The pull request changes new-layer insertion and tile rendering order described by the issue.
Possibly related PRs
- opengeos/RustGIS#834: Related tile ordering and tile identity handling.
- opengeos/RustGIS#943: Related
identifyand WMS result ordering. - opengeos/RustGIS#870: Related PMTiles insertion and tile placement.
Suggested reviewers: giswqs
Merge Risk: 
Moderate · up to 7dec8
-
Moving a layer or group to its current position can unexpectedly move tile overlays beneath map content. Resolve those ordering regressions before merging.
Pre-merge checks: 
5
-
Full details: Description check
-
Explanation
The description includes the required Summary, Testing, UI evidence, and Related issue sections. It explains the behavior changes, test coverage, known environment-specific failures, screenshots, and linked issue.
Full details: Linked Issues check
-
Explanation
The PR addresses the active requirements in [
#808].Map::add_datasetand other layer-add paths now insert new layers at the top instead of by geometry rank.TileLayer::over,Map::stack, tile placement, movement, removal, grouping, and rendering place service and raster tiles among map layers. Basemap tiles remain below map layers. The Layers pane, overlay wording, identify ordering, and AI Assistant context match the drawing order. Tests cover insertion, tile placement, legacy projects, rendering, removal, groups, and UI behavior.
Full details: Out of Scope Changes check
-
Explanation
The changed files support [
#808]. Packaging and template cleanup preserve tile placement when layers are removed. Identify and AI Assistant changes expose the new drawing order. UI, documentation, and tests describe or verify the feature. No unrelated change is demonstrated by the reviewed summary and focused diff.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 84.21% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 57 functions across 17 files. (3 skipped: 2 unsupported, 1 too large.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit hops where tile layers lie
Between the map and evening sky
Up goes a layer, down goes another
The stack stays neat beside its brother
The basemap rests beneath the scene
And every row knows where it’s been
Comment @coderabbitai help to get the list of available commands.
Code reviewBugs
Security
Performance
Quality
AGENTS.md
What I checked
Change sizeCompared: the whole PR against main (merge base 3a2d52d to PR head 7dec859).
|
📖 Docs preview
GitHub Pages takes a minute or two to publish a new commit. |
Summary
Newly added layers go on top of the drawing order, as in GeoLibre (the direction in #808), and web tile layers (WMS, WMTS, XYZ, vector tiles, PMTiles, MBTiles, GeoTIFFs read in place) take a place among the map's layers instead of always drawing under all of them.
New layers on top, whatever adds them
"top"option ofmap.addTiles). Each goes above the one added before it, and the layers already there keep their order.Tile layers among the map's layers
TileLayer.overnames the map layer a tile layer is drawn just over. Without it, the tile layer is drawn under all of them, which is where every tile layer of a project saved before this stays. Saved projects draw as before (tested).Map::stack()is the drawing order of all layers.map.tilesis kept in that order, so index 0 is still the topmost tile layer for scripts, the pop-ups and the AI Assistant.LiveTiles, tile meshes). Slots are worked out once a frame.map.setTilesmove(up/down/top/bottom) now steps past map layers too, a group at once. A newtoputs a tile layer at a place in the drawing order. A badto, ormoveandtotogether, is refused.Behaviour changes for existing use
map.setTiles"move": "up"/"top"on a tile layer under all layers now moves it over map layers, where before it only moved among tile layers. Rows of the Basemap gallery keep the old among-tile-layers behaviour.Testing
top, and an MBTiles archive from Add Data drawn over the map.to, and bad parameters.map.tilesorder; old projects withoutover, and a saved place read back.rustgis-map,rustgis-engine,rustgis-ui-eguiandrustgis-assistantpass.rustgis-renderonly the two text tests that need fonts missing on my machine fail (text_takes_italic_alignment_and_letter_spacing,a_text_graphic_is_drawn_aligned_slanted_and_spaced); they fail onmainhere too.-D warnings,cargo fmtandcargo xtask layerspass.cargo xtask cirun on my machine, so the crates were tested one at a time. The crates this doesn't touch weren't run.ui_shot, the issue's GEBCO WMS on the World Atlas sample, against the real service.UI evidence
The GEBCO WMS from #808 added with Overlay on the World Atlas sample.
Before: listed and drawn under Countries, which hides it (only the sea shows):
After: added on top of the map's layers:
After, moved between layers (
map.setTiles {"to": 3}, or Move down / dragging in the Layers pane): over Countries, under Lakes, Rivers and Cities:Related issue
Closes #808
Summary by CodeRabbit