Skip to content

vrt: stop build_vrt fabricating a GeoTransform for non-georeferenced sources#2971

Merged
brendancol merged 1 commit into
mainfrom
issue-2966
Jun 5, 2026
Merged

vrt: stop build_vrt fabricating a GeoTransform for non-georeferenced sources#2971
brendancol merged 1 commit into
mainfrom
issue-2966

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #2966

Problem

build_vrt wrote a <GeoTransform> into the VRT even when the source TIFF had no georeferencing. A non-georeferenced source carries a default identity transform (origin 0,0; pixel 1,-1) with has_georef=False, and that identity leaked into the VRT XML. A plain TIFF that reads as georef_status='none' came back from its VRT as transform_only with a fabricated (1, 0, 0, 0, -1, 0) transform.

Change

  • Track has_georef per source in build_vrt and emit <GeoTransform> only when the sources are georeferenced. A VRT without <GeoTransform> already reads back as none with integer pixel coords, so the reader side is unchanged.
  • Add _check_no_mixed_georef, a fail-loud gate that rejects mixing georeferenced and non-georeferenced sources. A plain tile only has identity values, so it cannot be placed on a georeferenced mosaic.
  • Two window-transform-shift tests in test_signatures.py relied on the fabricated identity GeoTransform. They now use a hand-built VRT with an explicit origin (0,0) GeoTransform so they still exercise the shift contract.

Backend coverage

build_vrt is a writer that produces VRT XML on disk; it is backend-independent. Reads of the resulting VRT were verified through open_geotiff (eager numpy path).

Test plan

  • Non-georeferenced source: VRT omits <GeoTransform> and reads back as georef_status='none' with transform=None.
  • Georeferenced source: VRT still emits <GeoTransform> and reads as a non-none status.
  • Mixed georef / non-georef sources: rejected with a clear ValueError.
  • Full xrspatial/geotiff/tests/ suite passes (6130 passed).

…sources (#2966)

A non-georeferenced source TIFF carries a default identity transform
(origin 0,0; pixel 1,-1) with has_georef=False. build_vrt stored that
transform unconditionally and always emitted a <GeoTransform>, so a VRT
over a plain TIFF read back as georef_status='transform_only' with a
fabricated (1, 0, 0, 0, -1, 0) transform instead of 'none'.

Track has_georef per source and emit <GeoTransform> only when the
sources are georeferenced. Add a fail-loud gate that rejects mixing
georeferenced and non-georeferenced sources, since a plain tile cannot
be placed on a georeferenced mosaic.

Two window-transform-shift tests relied on the fabricated identity
GeoTransform; point them at a hand-built VRT with an explicit
origin (0,0) GeoTransform so they still exercise the shift contract.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jun 5, 2026
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

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

PR Review: stop build_vrt fabricating a GeoTransform for non-georeferenced sources

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

None.

Nits (optional improvements)

  • _vrt.py (mosaic placement, ~lines 2178-2197): when more than one non-georeferenced source is passed, every tile gets a DstRect of (0,0). The identity transforms place each source at pixel origin, so later tiles overwrite earlier ones in the same window. This is pre-existing (the old fabricated-identity path did the same) and this PR does not make it worse, so it is out of scope here. Worth a separate follow-up if stacking multiple plain tiles is a real use case, since there is no spatial info to tile them by.
  • _vrt.py (~line 2207): for a non-georeferenced source, a caller-supplied crs_wkt still emits an <SRS> with no <GeoTransform>, which reads back as crs_only. That is a reasonable, non-fabricated result (CRS without a transform), so no change needed. Just flagging the interaction.

What looks good

  • Minimal fix on the exact mechanism: has_georef is tracked per source, and the <GeoTransform> is emitted only when sources are georeferenced.
  • _check_no_mixed_georef matches the existing fail-loud gate style (_check_no_rotated_source_transforms, _check_no_mixed_raster_type) and names the offending sources in the error.
  • Tests cover the three meaningful cases: non-georef VRT omits the transform and reads as none, georef VRT still emits the transform, mixed sources raise.
  • The two window-transform-shift tests that depended on the old fabricated identity were re-pointed at a VRT with an explicit GeoTransform, so they still cover the shift contract.
  • Full xrspatial/geotiff/tests/ suite passes (6130 passed).

Checklist

  • Contract matches the georef_status decision table: yes
  • NaN handling: not applicable (metadata-only change)
  • Edge cases covered by tests: single non-georef, georef, mixed
  • No premature materialization: yes (writer emits XML, no array work)
  • Benchmark: not needed (metadata path)
  • README feature matrix: not applicable (no new function)
  • Docstrings present and accurate: yes (new gate helper documented)

@brendancol brendancol merged commit 03b0d55 into main Jun 5, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build_vrt fabricates a GeoTransform for non-georeferenced TIFFs

1 participant