Skip to content

orthophoto merge: avoid boundless-read VRT serialization (gated reads)#2036

Open
Chouffe wants to merge 3 commits into
OpenDroneMap:masterfrom
Chouffe:orthophoto-gated-reads
Open

orthophoto merge: avoid boundless-read VRT serialization (gated reads)#2036
Chouffe wants to merge 3 commits into
OpenDroneMap:masterfrom
Chouffe:orthophoto-gated-reads

Conversation

@Chouffe

@Chouffe Chouffe commented Jun 23, 2026

Copy link
Copy Markdown

Summary

merge() reads every source window with boundless=True. rasterio's boundless mode builds an in-memory VRT and serializes it via Python's ElementTree (_serialize_xml) on every read — a large per-read overhead on large merges (a 15.9 Gpx mosaic is ~61k output blocks, each doing 3 passes over the submodel rasters).

_read_window_gated() produces identical output but skips the VRT for the common cases:

  • window fully inside the source → plain non-boundless read (no VRT);
  • window fully outside → zeros (identical to the 0 nodata fill boundless produces there);
  • window straddling the edge → fall back to boundless=True (rare; only the true border blocks).

Correctness

Pixel-identical. An oracle sampled hundreds of fully-in-bounds windows across a real merge grid and compared a boundless=True read against the plain read into a zeroed array — 0 mismatches.

Scope

Serial; no behavior change beyond the speedup. This also unblocks parallelizing the block loop (follow-up PR): under thread concurrency the per-read VRT serialization becomes pathological (workers park in _serialize_xml), so gating the reads is a prerequisite.

merge() reads each source window with rasterio boundless=True, which builds an
in-memory VRT and serializes it via Python's ElementTree (_serialize_xml) on
every read — a large per-read overhead on big merges (tens of thousands of
blocks x 3 passes x N submodels).

_read_window_gated() keeps identical output but avoids the VRT for the common
cases: a plain non-boundless read when the window is fully inside the source,
zeros when fully outside (== the 0 nodata fill boundless produces there), and
boundless only for the rare partial-edge windows.

Pixel-identical (verified: hundreds of fully-in-bounds windows across a real
merge grid compared boundless vs plain read, 0 mismatches). Serial; no behavior
change beyond the speedup. Also a prerequisite for parallelizing the merge:
boundless's per-read VRT serialization is pathological under concurrency.

@spwoodcock spwoodcock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice change, useful docstring as it's not an obvious one 👍

Some unit tests would be lovely, but overall this is well scoped and looks good to me!

Small thing: as with the other PR, it would also be great if you could indicate how this was tested / results / logs, and also to what extent AI was used to assist 🙏

A minor related nitpick would be the use of em-dash , a common sign of AI-gen text, for which I don't think we have any in the repo until this point!

Chouffe added 2 commits July 8, 2026 11:35
Assert the gated read is pixel-identical to a boundless=True read across all
three branches (fully inside, fully outside each edge, partial-edge overlap),
which is the correctness property the gated-read optimization relies on.
Runs under the ODM image's rasterio/GDAL: python3 -m unittest tests.test_orthophoto.
@Chouffe

Chouffe commented Jul 8, 2026

Copy link
Copy Markdown
Author

Thanks for the approval. Addressed the two follow-ups:

  • Dropped the em-dash from the _read_window_gated docstring (0737c13). It was the only one in the repo, so good spot.
  • Added tests/test_orthophoto.py (6773d00): 9 unit tests asserting the gated read is pixel-identical to a boundless=True read across all three branches (fully inside, fully outside each edge, and partial-edge overlaps). They run under the ODM image's rasterio/GDAL with python3 -m unittest tests.test_orthophoto.

@Chouffe

Chouffe commented Jul 8, 2026

Copy link
Copy Markdown
Author

For context on where this came from: this change originally lived inside #2035 (the parallel orthophoto merge). The gated-read fix stands on its own, though. It avoids the boundless-read VRT serialization and speeds up the existing serial merge without changing its output, independent of any parallelization. So I pulled it out into this smaller, standalone PR to make it easier to review, and #2035 now stacks on top of it.

Suggested review/merge order is this one first, then #2035.

@Chouffe

Chouffe commented Jul 15, 2026

Copy link
Copy Markdown
Author

@spwoodcock let me know if there is anything left I can address for this PR to get in.

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