Skip to content

Area-weight pyramid compose to remove interim zoom banding (#153)#217

Merged
Alek99 merged 2 commits into
mainfrom
claude/issue-153-diagnosis-b59345
Jul 22, 2026
Merged

Area-weight pyramid compose to remove interim zoom banding (#153)#217
Alek99 merged 2 commits into
mainfrom
claude/issue-153-diagnosis-b59345

Conversation

@adhami3310

Copy link
Copy Markdown
Member

Fixes #153.

Diagnosis

The vertical banding is a resampling beat (moiré) artifact in the tile-pyramid compose path — visible in the interim density frames served while zooming a dense point cloud.

The client requests a density grid at ~screen resolution (w×h), and compose picks the coarsest pyramid level that still meets that resolution. By construction that level packs between w and 2w source cells across the window (source→output ratio in [1, 2)). The old code assigned each source cell whole to the single output bin under its center:

out_row[ox as usize] += c as f32;   // ox = floor of the cell center

At a ratio like 1.33, adjacent output bins receive 1 vs 2 source cells apiece — a ~2:1 brightness beat that renders as the regular vertical stripes in the issue screenshot (dominant on X because the canvas is wider than tall, so the X ratio lands in the beat zone).

Fix

Replace the center-only assignment with area-weighted resampling (a box downsample): each source cell splits its count across the output bins its extent overlaps, in proportion to overlap. A uniform field now composes flat instead of beating. A new axis_weights helper precomputes the per-axis split (hoisted, so compose stays O(visible cells)).

Exactness preserved: weights within COMPOSE_SNAP_EPS (1e-6) of a bin edge collapse to a single bin, so cell-aligned windows (ratio exactly 1) remain bit-exact against bin_2d — the documented contract and existing tests hold.

Verification

  • All 9 tiles Rust tests pass, including the two exact-alignment tests and conservation.
  • New regression test compose_unaligned_ratio_has_no_banding — confirmed it fails under the old nearest-neighbor logic (column = 128 vs mean 85.3) and passes with the fix.
  • Full cargo test (103 tests), release build, ABI smoke (121 checks), clippy, fmt all clean.
  • End-to-end through the Python binding on the built library: full-window compose still bit-exact; uniform field composes flat (max column deviation 0.000 vs the prior 2× swing).

Files changed

  • src/tiles.rs — area-weighted compose + axis_weights helper + regression test + module doc.
  • spec/design/lod-architecture.md — disclosed the resampling method (§28 "no silent decisions").

The tile-pyramid compose path served interim density frames with a
visible vertical beat when zooming a dense cloud. compose picks the
coarsest level that still meets the render resolution, so the window
holds 1-2 source cells per output bin; assigning each source cell whole
to the bin under its center then handed adjacent bins 1 vs 2 cells
apiece, a moire against the output grid.

Area-weight each source cell across the bins its extent overlaps
instead. A snap tolerance collapses near-aligned edges to one bin, so
cell-aligned windows stay bit-exact against bin_2d.
…nosis-b59345

# Conflicts:
#	spec/design/lod-architecture.md
#	src/tiles.rs
@codspeed-hq

codspeed-hq Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 39.54%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 101 untouched benchmarks
⏩ 1 skipped benchmark1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_pyramid_compose 5.9 ms 9.8 ms -39.54%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/issue-153-diagnosis-b59345 (ca3b04f) with main (ad29127)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR removes tile-pyramid zoom banding with area-weighted resampling. The main changes are:

  • Adds separable area weights for downsampling.
  • Preserves exact output for aligned cells with snap tolerance.
  • Adds uniform-field coverage for non-integer scaling.
  • Documents the compose resampling behavior.

Confidence Score: 4/5

Mixed-axis composition can still skip area weighting on the downsampled axis.

  • The square downsampling path uses the new area weights.
  • A joint branch still chooses pull-sampling for both axes when either axis is upsampled.

src/tiles.rs

T-Rex T-Rex Logs

What T-Rex did

  • The pre-capture contract validation shows the parent revision 49d4088 exited 0 with 9/9 focused tests passing.
  • The new capture at current HEAD shows exit status 0 with 10/10 focused tests passing.
  • We verified that both logs include the exact command, working directory, revision, exit status, and per-test results.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
src/tiles.rs Adds area-weighted composition and tests for aligned and square downsampling.
spec/design/lod-architecture.md Documents the new downsampling and upsampling behavior.

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

@adhami3310

Copy link
Copy Markdown
Member Author

Re: the CodSpeed test_pyramid_compose regression

Flagging this as an intentional, understood tradeoff — the acknowledge action is left to a maintainer.

What the number is. CodSpeed runs this in Simulation mode (cachegrind-style), which scores a cost model of instructions executed, not wall-clock. It doesn't model the out-of-order execution / memory-level parallelism that dominate this kernel on real silicon.

Why it moved. This PR is the #153 banding fix: it replaces main's center-only (nearest) binning with area-weighted resampling. Area weighting inherently does ~2× the arithmetic per source cell (each cell splits across up to two bins per axis, with float weights) — so under an instruction-count model, ~2× is unavoidable. It's the cost of not lying about density at interim zoom levels.

Real wall-clock is at parity or better. The baseline is memory-bound, so the extra arithmetic hides behind memory stalls. Measured locally on the exact CodSpeed shape (2.1M points, 512×384 served from the 1024 level):

version wall-clock
main (nearest) baseline 0.919 ms
one-pass 2D area-weight scatter (first cut) 2.22 ms
separable pull-based (current HEAD, e73487b) 0.950 ms

e73487b makes the area-weighted compose essentially free relative to the nearest-neighbor baseline it "regressed" from (@alekpetuskey measured 0.53 ms vs 0.63 ms on his machine — faster than baseline). All 10 tiles tests pass and the uniform-field column deviation is 0.0 (banding gone).

Recommendation: acknowledge on CodSpeed. Instruction count is genuinely higher by design; there's no way to make area weighting as cheap as nearest without reintroducing the banding, and on-hardware latency is unchanged.

@Alek99
Alek99 force-pushed the claude/issue-153-diagnosis-b59345 branch from e73487b to ca3b04f Compare July 22, 2026 23:36
@Alek99
Alek99 merged commit 31d3959 into main Jul 22, 2026
88 of 89 checks passed
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.

vertical "banding" when zooming in through LoD chart

2 participants