Apply TIFF Orientation tag in read_geotiff_gpu (#1540)#1546
Open
brendancol wants to merge 1 commit intomainfrom
Open
Apply TIFF Orientation tag in read_geotiff_gpu (#1540)#1546brendancol wants to merge 1 commit intomainfrom
brendancol wants to merge 1 commit intomainfrom
Conversation
PR #1521 wired up the Orientation tag (274) on the CPU read path but left the GPU path on the raw stored buffer, so any TIFF written with orientation 2-8 decoded to different pixel values on CPU vs GPU. This adds two helpers in xrspatial/geotiff/__init__.py: - _apply_orientation_gpu mirrors the CPU _apply_orientation slicing logic against a cupy ndarray (eight orientations, 2-D and 3-D variants). - _apply_orientation_geo_info centralises the post-flip transform update so both read_to_array (CPU) and read_geotiff_gpu update GeoTransform consistently. read_geotiff_gpu now applies orientation post-decode for every pure-GPU code path, and reuses the geo_info returned by read_to_array when the CPU fallback path runs (since that path already applies the remap). A flag (arr_was_cpu_decoded) keeps us from double-applying. Test coverage: test_orientation_gpu.py exercises every orientation on single-band tiled, single-band stripped, and 3-band planar=2 tiled files, plus georef sel-fidelity for the mirror-flip cases.
There was a problem hiding this comment.
Pull request overview
This PR brings read_geotiff_gpu in line with the CPU GeoTIFF reader by applying the TIFF Orientation tag (274) on the GPU path, and adds GPU-focused regression tests to ensure pixel buffers and (intended) coordinate semantics match across backends.
Changes:
- Added
_apply_orientation_gputo remap decoded CuPy arrays according to TIFF orientation values 1–8. - Added
_apply_orientation_geo_infoand integrated orientation application into the pure-GPU tiled decode path; CPU-fallback branches now reusegeo_inforeturned byread_to_array. - Added a new GPU test module covering tiled/stripped/multiband orientations and coordinate-selection behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
xrspatial/geotiff/__init__.py |
Applies TIFF Orientation tag to GPU outputs; introduces shared geo-transform update helper and avoids double-applying orientation on CPU-fallback paths. |
xrspatial/geotiff/tests/test_orientation_gpu.py |
Adds GPU regression tests for orientation remapping, multiband handling, and georeferenced coordinate selection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+141
to
+147
| (33550, 'd', 3, (1.0, 1.0, 0.0)), | ||
| (33922, 'd', 6, (0.0, 0.0, 0.0, 100.0, 50.0, 0.0)), | ||
| (34735, 'H', 12, ( | ||
| 1, 1, 0, 2, | ||
| 1024, 0, 1, 2, | ||
| 2048, 0, 1, 4326, | ||
| )), |
Comment on lines
+1761
to
1769
| # Fall back to CPU for stripped files. read_to_array already | ||
| # applies the orientation remap to both the array and the | ||
| # geo_info transform (#1537), so reuse its geo_info here | ||
| # rather than the pre-orientation one we just extracted. | ||
| src.close() | ||
| arr_cpu, _ = read_to_array(source, overview_level=overview_level) | ||
| arr_cpu, geo_info = read_to_array( | ||
| source, overview_level=overview_level) | ||
| arr_gpu = cupy.asarray(arr_cpu) | ||
| coords = _geo_to_coords(geo_info, arr_gpu.shape[0], arr_gpu.shape[1]) |
Comment on lines
+1992
to
+2001
| # Apply the TIFF Orientation tag (274) to the GPU array. The CPU | ||
| # reader does this in `read_to_array` (#1521 + #1537), so any path | ||
| # that already went through the CPU fallback above is a no-op here. | ||
| # The pure GPU paths land at this point with a raw stored-order | ||
| # buffer, and without this remap they would silently disagree with | ||
| # the CPU reader (#1540). | ||
| if orientation != 1 and not arr_was_cpu_decoded: | ||
| arr_gpu = _apply_orientation_gpu(arr_gpu, orientation) | ||
| geo_info = _apply_orientation_geo_info( | ||
| geo_info, orientation, file_h=height, file_w=width) |
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.
Summary
read_geotiff_gpunow applies the TIFF Orientation tag (274) so the GPU and CPU readers return the same pixel buffer for every orientation._apply_orientation_gpu(mirrors_apply_orientationon a cupy array) and_apply_orientation_geo_info(post-flip transform update used by both readers).read_to_arrayso the orientation update isn't applied twice.Context
PR #1521 added
_apply_orientationto the CPU path; the GPU pipeline never picked it up. Any TIFF carrying orientation 2-8 (cameras, scanners, some legacy GIS exports) silently disagreed across backends. See #1540 for the reproducer.The PR also folds in the mirror-flip transform update from #1537 into a shared helper so both readers keep the same coord-fidelity guarantees. (#1537 covers the CPU side standalone; this PR is callable independently because
_apply_orientation_geo_infois local to__init__.py.)Test plan
pytest xrspatial/geotiff/tests/test_orientation_gpu.py -q(28 new tests: tiled, stripped, 3-band, mirror-flip sel-fidelity, default no-tag passthrough).pytest xrspatial/geotiff/tests/test_orientation.py xrspatial/geotiff/tests/test_planar_multiband.py xrspatial/geotiff/tests/test_lerc_valid_mask_gpu.py xrspatial/geotiff/tests/test_predictor2_big_endian_gpu_1517.py xrspatial/geotiff/tests/test_gpu_byteswap_1508.py xrspatial/geotiff/tests/test_gpu_strict_fallback_1516.py xrspatial/geotiff/tests/test_gpu_stripped_multiband.py -q(127 passed).pytest xrspatial/geotiff/tests/ -q --no-header --deselect xrspatial/geotiff/tests/test_features.py::TestPalette(960 passed; deselected tests fail on Python 3.14 / matplotlib deepcopy, unrelated).Closes #1540.