Unified RasterFieldSource render path + NaN-NoData fix (Part A of #121)#140
Merged
Conversation
Extract RasterFieldSource interface + shared GL renderer; migrate GggsTileLayer, SonarLiveCacheLayer, RasterLayer; NaN NoData fix in unified shader; single PR.
Extract the duplicated GL raster display path into one place (ADR-0007):
- docs/decisions/0007-raster-field-source-interface.md — RasterFieldSource API,
ownership + threading contract, scalar-vs-RGBA format branch, NaN-fix rationale,
marine_colormap deferral, and the corrected ADR references.
- raster/raster_field_source.h — the abstract source interface (bands / metadata /
items / dataRange) + the RasterFieldItem value type (Scalar R32F + LUT, or RGBA8
bypass; geographic warp vs already-Web-Mercator quad).
- raster/raster_gl_renderer.{h,cpp} — the ONE compiled program + colormap LUT (per
GL context), the unified fragment shader, the geo->Web-Mercator tessellation, and
the data-texture filter. The NaN NoData fix lands here once:
if(v != v) discard; // NaN (GLSL 1.20-portable)
if(u_has_nodata != 0 && v == u_nodata) discard; // finite sentinel
No call sites yet — the three adapters migrate in the following commits.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Delete this layer's kVertex/kFragment shaders, ensureProgram(), ensureLut(), per-vertex tessellation and its own GL context/FBO/program/LUT. The layer now holds a RasterGlRenderer, implements RasterFieldSource (bands/metadata/items/ dataRange), and renderImage() collects the loaded tiles as Scalar items and delegates the warp + draw. Tile textures stay owned by the GggsTiles and are released under the renderer's context in the dtor / band switch. QtConcurrent load + atomic pixelsLoaded() publish are unchanged. The colormap is now held by the renderer (setColormap re-bakes the LUT). Behaviour is identical plus the NaN NoData fix inherited from the unified shader. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Delete the duplicated shader block (and its `// NOTE: shader duplicated; unify via RasterFieldSource camp#134` marker), ensureProgram/ensureLut, the per-vertex tessellation, and the layer's own GL context/FBO/program/LUT. The layer now holds a RasterGlRenderer, implements RasterFieldSource, and renderImage() collects the held tiles' selected band as Scalar items and delegates to the renderer. Entry textures stay owned by the Entries (released under the renderer's context). ADR-0006 is untouched: persistence (write-through / warm-load), the opt-in tile-stream subscription, the catalog reconcile/prune, and the GUI-thread marshalling invariant are all outside the render path and unchanged here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the QPainter/mipmap path with the shared GL renderer. GDAL still warps the chart to Web-Mercator on load; the reprojected pixels are uploaded as a GL texture (scalar -> R32F shaded by the LUT; palette/RGB -> CPU-composited RGBA8 sampled directly, LUT bypassed, with mipmaps for zoomed-out LOD) and drawn through RasterGlRenderer as a single non-geographic quad. Adopts the GggsTileLayer placement convention (NW anchor + fromScale(1,-1)). - Remove the mipmap pyramid + QPixmap chain + the CPU ColorMap shading loop. - A scalar colormap change is now an LUT re-bake (no re-warp). - Keep the async GDAL load + abort flag and the RAII handle-close. - Implement RasterFieldSource; the texture is owned here and released under the renderer's context in the dtor. CPU auto-range still excludes NaN + the finite sentinel, so it agrees with the shader's discard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- test/test_raster_gl_renderer.cpp (new): drives the shared renderer directly with hand-built textures — scalar NaN + finite-sentinel discard, colormap LUT bake, Nearest sampling (no bleed across a NoData boundary), and the RGBA bypass path (sampled directly, a==0 discards). Mirrors test_gggs_render's offscreen-GL harness + the offscreenGLAvailable() SKIP guard (SKIPs in-container, runs on a GL host). - test/test_gggs_render.cpp: add NanNoDataDiscardsTransparent — a Float32 tile with a NaN NoData sentinel and a NaN interior block must render the block transparent (was opaque before the fix); same dense-column discriminator as the finite case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r_layer error/alpha fixes
…edundant filter set
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part A of #121 — consolidates CAMP's raster rendering behind one source-agnostic path,
and (as the forcing function) fixes NaN-NoData null transparency for the float data stores.
Closes #134.
Why
The GGGS GL shader (colormap LUT + Nearest data texture + NoData discard) was duplicated
in
GggsTileLayer(file store) andSonarLiveCacheLayer(live cache, camp#121), and untiledrasters (
RasterLayer) rendered via a separate QPainter path with no shader/colormap. Threecopies of "draw a colormapped raster," and a NoData fix would have to land in each.
Worse, the discard used exact equality (
v == u_nodata), which silently fails for NaN(
NaN == NaNis false in GLSL). The Massabesic contour bathy and the processed backscatterstores use NaN as NoData, so their null cells rendered opaque. (Sidescan uses 0, so it
was unaffected.)
What
RasterFieldSourceinterface +RasterGlRenderer— one compiled shader program +colormap LUT per GL context, the geo→Web-Mercator tessellation, and the data-texture filter,
all in one place (ADR-0007).
GggsTileLayer,SonarLiveCacheLayer(duplicated shader + its// unify via camp#134marker deleted;ADR-0006 persistence/subscription contract untouched), and
RasterLayer(QPainter/mipmap →GPU).
RasterFieldItem::Format{Scalar,Rgba}serves scalar bands (R32F+LUT) and palette/RGBcharts (CPU-composited RGBA8, LUT bypassed).
ColorMapkept — themarine_colormap-lib migration is deferred (separable).Parity (the main risk on a refactor this size)
Verified per adapter: multi-band select, colormap LUT + per-band auto-range, NoData discard
(NaN and finite), Nearest sampling, and RasterLayer's QPainter→GPU change (scalar colormap,
palette/RGB, NoData transparency). Round-1 review caught two real parity/lifecycle bugs — a
sub-unit-range colormap regression (the shader floored the denominator at
1.0, crushingcontrast for small-range charts; now true-span) and a prune-time GPU-texture leak (destroy
without a current GL context; now
makeCurrent-guarded) — both fixed and tested.Tests
test_raster_gl_renderer.cpp(new): NaN discard, finite-sentinel discard, LUT bake, Nearest,RGBA-bypass, sub-unit-range colormap.
test_gggs_render.cpp: NaN-NoData render. Host-verified(GL available): 141 tests, 0 failures, 1 skipped — the GL render/parity tests actually ran.
Review
/run-issue; 3 rounds of Deep pre-pushreview-code. Final: approved, 0 must-fix, 0suggestions.
Follow-ups
marine_colormap-lib migration (deferred) — per-band colormap defaults.RasterLayer per-band channel-select (RGB/palette composite is CPU today) — noted in the plan's
open question, not yet filed.
Authored-By:
Claude Code AgentModel:
Claude Opus 4.8 (1M context)