Skip to content

Visible-region render at the raster seam — crisp zoomed-in large stores (Part of #103) - #173

Merged
rolker merged 8 commits into
jazzyfrom
feature/issue-103
Jul 24, 2026
Merged

Visible-region render at the raster seam — crisp zoomed-in large stores (Part of #103)#173
rolker merged 8 commits into
jazzyfrom
feature/issue-103

Conversation

@rolker

@rolker rolker commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

Implements the visible-region half of #103 (Part of #103 — does NOT close it; the pyramid-LOD half is deferred pending store overview pyramids, unh_marine_autonomy#188).

All three raster layers (GggsTileLayer, RasterLayer, SonarLiveCacheLayer) now render only the viewport-visible clip of their extent, sized to its on-screen pixels, instead of one whole-extent ≤4096 px image upscaled over boundingRect() — fixing the field-observed zoomed-in blur (2026-07-23) on stores larger than kMaxImageEdge.

Design (ADR-0011)

  • Shared raster/viewport_clip.h derives the clip from painter->clipBoundingRect()not exposedRect, which silently defaults to the full bounding rect without ItemUsesExtendedStyleOption (set nowhere in camp). Empty clip falls back to the whole extent (headless/test renders unchanged).
  • Clip-aware public renderImage(size, clip_bounds) overloads pass the clip as renderToImage()'s scene_boundsno RasterGlRenderer API change (ADR-0007 seam).
  • Tiled sources filter tiles against the clip before the lazy texture upload; the live layer filters both pools preserving the overviews-first LOD-fallback order (ADR-0010). This is also the natural seam for camp#172's on-demand reload (not implemented here).
  • Cache keyed by FBO size AND clip: pan re-renders (cheap at viewport size).

Testing

  • 160 tests, 0 failures (worktree host build). Two new headless GL tests: clip filter keeps partial intersections / excludes disjoint tiles; clip-render pixel density scales with the clip (the blur fix).
  • Manual GUI verification recommended before merge (reviewer suggestion — headless tests exercise the clip seam, not paint()'s live derivation): source layers/worktrees/issue-camp-103/ui_ws/install/setup.bash; ros2 launch camp camp_launch.py → open a large store (e.g. ~/data/stores/sidescan/processed) → zoom in → confirm crisp rendering.

Review

Pre-push /review-code (Deep, Opus): approved, 0 must-fix, Ship: recommended (round 1). Non-blocking suggestions recorded in progress.md: per-frame geoToMap trig during pan is O(tiles) on the GUI thread (cache tile scene-rects if large stores pan sluggishly); comment the float-equality cache key.

Plan: .agent/work-plans/issue-103/plan.md (review-plan verdict changes-requested → all 3 must-fixes folded; positive deviation: clip derivation factored into one shared header instead of per-layer copies).

Part of #103.


Authored-By: Claude Code Agent
Model: Claude Fable 5

Claude Code Agent added 7 commits July 24, 2026 13:27
Visible-region render: size FBO to viewport clip rect, filter tiles to those
intersecting the visible region, pass clip rect as scene_bounds to renderToImage.
… clip overload, extend test_gggs_render, pan-cache + overview-order notes
… stores (Part of #103, ADR-0011)

All three RasterFieldSource layers (GggsTileLayer, RasterLayer,
SonarLiveCacheLayer) now render only the viewport-visible clip of their
extent, sized to its on-screen pixels, instead of one whole-extent
<=4096px image upscaled over boundingRect() — fixing the field-observed
zoomed-in blur on stores larger than kMaxImageEdge.

- shared raster/viewport_clip.h derives {local, scene, size} from
  painter->clipBoundingRect() (NOT exposedRect, which is a no-op without
  ItemUsesExtendedStyleOption); empty clip falls back to the full extent
- clip-aware public renderImage(size, clip_bounds) overloads pass the
  clip as renderToImage()'s scene_bounds — no renderer API change
- tiled sources filter tiles against the clip BEFORE the lazy texture
  upload; the live layer filters both pools preserving the
  overviews-first LOD-fallback order (ADR-0010)
- cache keyed by size AND clip: pan re-renders (viewport-sized FBO makes
  it cheap); stale pan-reuse comments updated
- tests: clip filter keeps partial intersections + excludes disjoint
  tiles; clip render pixel density scales with the clip (the blur fix)
- ADR-0011 records the convention; LOD half of #103 deferred (needs
  store pyramids, uma#188)
Copilot AI review requested due to automatic review settings July 24, 2026 15:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements the “visible-region render” half of camp#103 by changing raster rendering to draw only the viewport-visible clip of each layer’s extent (sized to the clip’s on-screen pixel dimensions), avoiding the previous whole-extent ≤4096px render that produced blurred results when zoomed in on very large stores.

Changes:

  • Add a shared raster::deriveViewportClip() helper to compute local/scene clip rectangles and a viewport-sized FBO target from QPainter::clipBoundingRect().
  • Update GggsTileLayer, RasterLayer, and SonarLiveCacheLayer to use clip-aware renderImage(size, clip_bounds) and cache by both FBO size and clip rect (pan/zoom re-render behavior).
  • Extend headless GL tests to validate clip filtering (intersecting vs disjoint tiles) and that pixel density scales with the clip (regression coverage for the “zoomed-in blur” issue); add ADR-0011 documenting the seam convention.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/test_gggs_render.cpp Adds headless GL regression tests for clip filtering and clip-scaled resolution.
src/camp_map/ros/live_coverage/sonar_live_cache_layer.h Adds clip-aware renderImage overload, itemsIntersecting, and clip-based cache key state.
src/camp_map/ros/live_coverage/sonar_live_cache_layer.cpp Implements clip filtering before texture upload/draw; uses viewport-derived clip in paint().
src/camp_map/raster/viewport_clip.h Introduces shared viewport-clip derivation helper (local/scene/size).
src/camp_map/raster/raster_layer.h Extends cached render keying to include clip rect.
src/camp_map/raster/raster_layer.cpp Uses shared viewport clip derivation; adds clip-aware renderImage overload passing clip to renderer.
src/camp_map/raster/gggs_tile_layer.h Documents viewport-clip behavior; adds clip-aware renderImage overload and clip-based cache key state.
src/camp_map/raster/gggs_tile_layer.cpp Filters tiles by clip before texture upload; uses viewport-derived clip in paint() and clip-scoped render bounds.
docs/decisions/0011-viewport-clip-render-convention.md Adds ADR-0011 documenting the viewport-clip calling convention at the raster seam.
.agent/work-plans/issue-103/progress.md Work-plan progress record for issue-103.
.agent/work-plans/issue-103/plan.md Work-plan describing the intended implementation and verification approach.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…pty on live repaints (Part of #103)

Field test (Roland, 2026-07-24) showed the blur unchanged in the GUI:
painter->clipBoundingRect() is only populated on repaint paths where
QGraphicsView sets a clip, and is EMPTY on live full-viewport repaints —
so deriveViewportClip() fell back to the whole extent, reproducing the
pre-#103 behavior. Same trap as the exposedRect default the plan review
caught, one door over.

The viewport now comes from the attached view itself
(mapToScene(viewport) mapped into the item via mapRectFromScene), which
is repaint-path-independent; painter clip stays as a secondary fallback
for painter-only renders, then the full extent (headless QA). First view
only — CAMP has a single MapView.

New regression test drives the REAL paint() path through an attached
QGraphicsView (grab of the viewport widget): a 2000x2000 per-pixel
checker at 40 px/cell must render with ~half dark pixels and <15%
mid-tones. Verified fails-without-fix (broken derivation smears ~50%
mid-tones — the field-observed blur) and catches the layer-not-painted
blank case. ADR-0011 derivation section updated to match.
@rolker

rolker commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

Field-test finding (Roland, 2026-07-24): the blur was unchanged in the live GUI. Root cause: painter->clipBoundingRect() is empty on live full-viewport repaints (QGraphicsView only sets a painter clip on some repaint paths), so the derivation fell back to the whole extent — the same trap as the exposedRect default, one door over.

Fixed in 65224cf: the viewport clip now comes from the attached view (mapToScene(viewport) mapped into the item), which is repaint-path-independent; painter clip demoted to secondary fallback, whole extent for headless renders. ADR-0011 updated.

New regression test drives the real paint() path through an attached QGraphicsView (viewport grab, per-pixel checker at 40 px/cell, assertions on dark-cell presence + mid-tone fraction). Verified it fails without the fix (reproduces the field blur — ~50% mid-tones) and catches the blank-layer case. 161 tests, 0 failures.

@rolker please pull + rebuild the worktree (./ui_ws/build.sh camp in layers/worktrees/issue-camp-103) and re-check the sidescan zoom.


Authored-By: Claude Code Agent
Model: Claude Fable 5

Copilot AI review requested due to automatic review settings July 24, 2026 18:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment on lines +405 to +409
const QPointF lo = web_mercator::geoToMap(
QGeoCoordinate(tile->minLat(), tile->minLon()));
const QPointF hi = web_mercator::geoToMap(
QGeoCoordinate(tile->maxLat(), tile->maxLon()));
if(!QRectF(lo, hi).normalized().intersects(clip_scene))
Comment on lines +850 to +854
const QPointF lo = web_mercator::geoToMap(
QGeoCoordinate(entry.tile.minLat(), entry.tile.minLon()));
const QPointF hi = web_mercator::geoToMap(
QGeoCoordinate(entry.tile.maxLat(), entry.tile.maxLon()));
if(!QRectF(lo, hi).normalized().intersects(clip_scene))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants