Skip to content

Skip mask IFDs when resolving overview_level (#1504)#1518

Merged
brendancol merged 2 commits intoxarray-contrib:mainfrom
brendancol:fix-1504-overview-filter
May 8, 2026
Merged

Skip mask IFDs when resolving overview_level (#1504)#1518
brendancol merged 2 commits intoxarray-contrib:mainfrom
brendancol:fix-1504-overview-filter

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #1504.

open_geotiff(path, overview_level=N) was indexing the IFD list directly. When a TIFF puts a transparency-mask IFD (NewSubfileType bit 2) between the full-res IFD and the overviews, as some GDAL COGs do, overview_level=1 returned the 1-bit mask instead of the first overview.

The reader now filters out IFDs whose NewSubfileType has bit 2 set before indexing, matching what GDAL and rasterio do. An out-of-range overview_level raises ValueError with the actual count of non-mask IFDs (and any mask IFDs present), instead of silently clamping to the last IFD.

Changes:

  • IFD.subfile_type and IFD.is_mask properties on the parsed IFD
  • select_overview_ifd helper in _header.py
  • routed the four overview_level call sites (local CPU read, COG-over-HTTP, GPU read, _read_geo_info) through the helper

Tests in xrspatial/geotiff/tests/test_overview_filter.py write a 3-IFD TIFF with a mask wedged between the full-res IFD and an overview, then check that open_geotiff(..., overview_level=1) returns the overview rather than the mask. The ValueError path and a normal 4-IFD COG (no masks) are covered too.

open_geotiff(path, overview_level=N) was indexing the IFD list directly.
When a TIFF puts a transparency-mask IFD (NewSubfileType bit 2) between
the full-res IFD and the overviews, as some GDAL COGs do,
overview_level=1 returned the 1-bit mask instead of the first overview.

The reader now filters out IFDs whose NewSubfileType has bit 2 set
before indexing, matching what GDAL and rasterio do. Out-of-range
overview_level raises ValueError with the actual count of non-mask IFDs
(and any masks) instead of silently clamping to the last IFD.

- IFD.subfile_type and IFD.is_mask properties on parsed IFDs
- select_overview_ifd helper in _header.py
- routed the four overview_level call sites (CPU read, COG-over-HTTP,
  GPU read, _read_geo_info) through the helper
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 8, 2026
@brendancol brendancol requested a review from Copilot May 8, 2026 14:33
Copy link
Copy Markdown

Copilot AI left a comment

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 fixes open_geotiff(..., overview_level=N) selecting the wrong IFD when GeoTIFF/COG files interleave transparency-mask IFDs (NewSubfileType bit 2) with overviews, by routing overview selection through a helper that skips mask IFDs and raises on out-of-range requests.

Changes:

  • Added IFD.subfile_type / IFD.is_mask helpers and a centralized select_overview_ifd() overview resolver.
  • Updated CPU, COG-over-HTTP, GPU, and geo-metadata read paths to use the shared overview-selection logic.
  • Added regression tests covering mask-interleaving behavior, out-of-range errors, and normal COG behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
xrspatial/geotiff/_header.py Adds NewSubfileType helpers and select_overview_ifd() to skip mask IFDs and validate overview_level.
xrspatial/geotiff/_reader.py Routes local and HTTP COG read paths through select_overview_ifd() instead of indexing/clamping raw IFDs.
xrspatial/geotiff/__init__.py Routes GPU read and _read_geo_info through select_overview_ifd() for consistent behavior.
xrspatial/geotiff/tests/test_overview_filter.py Adds regression/unit tests for mask-interleaved IFD chains and out-of-range handling.

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

Comment thread xrspatial/geotiff/tests/test_overview_filter.py Outdated
Comment thread xrspatial/geotiff/tests/test_overview_filter.py Outdated
Comment thread xrspatial/geotiff/_header.py
…orskip tifffile

- ``select_overview_ifd`` previously kept any non-mask IFD, which let
  multi-page-document IFDs (NewSubfileType bit 1) through. Tighten the
  filter to keep only the full-res IFD (subfile_type=0) plus genuine
  overview IFDs (bit 0 set, bit 2 clear). Pages and any other future
  flag combinations are now excluded so ``overview_level`` indexes the
  pyramid only. Diagnostic text updated to "pyramid IFDs" / "non-pyramid
  IFD".
- Test module had ``import tifffile`` at the top; switched to
  ``pytest.importorskip("tifffile")`` so the file doesn't fail at
  collection in environments without tifffile.
- Removed unused ``IFD`` import from the test module.
- Added two new test cases: page-IFD (subfile_type=2) is filtered, and
  overview-of-mask (subfile_type=5) is filtered (mask bit dominates).
@brendancol brendancol merged commit e510e78 into xarray-contrib:main May 8, 2026
11 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.

overview_level indexes IFDs blindly, returns wrong layer when mask/transparency IFDs are interleaved

2 participants