Skip to content

Decode TIFF predictor=2 sample-wise for multi-byte integer round-trip#1499

Closed
brendancol wants to merge 1 commit intomainfrom
fix-predictor2-samplewise
Closed

Decode TIFF predictor=2 sample-wise for multi-byte integer round-trip#1499
brendancol wants to merge 1 commit intomainfrom
fix-predictor2-samplewise

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

  • TIFF predictor=2 decode and encode were byte-wise at a sample stride, which loses carries inside multi-byte samples. Files xrspatial wrote round-tripped because both halves were wrong in matching ways, but uint16, int16, uint32 and int32 files written by libtiff/GDAL with predictor=2+deflate/lzw silently decoded to wrong pixel values whenever an adjacent-sample difference crossed a byte boundary.
  • Decode now operates on whole sample values in the file's byte order, per-channel for chunky multi-band data; the 8-bit fast path stays. Predictor=2 on floats stays byte-wise (the spec restricts predictor=2 to integers; floats are out of spec for this predictor) so old xrspatial-written float files still read back unchanged.
  • The GPU encode dispatch was also fixed: it had been called with row_bytes multiplied by samples-per-pixel twice, which was inert only because the byte-wise math was wrong on both sides.

Repro before the fix

import rasterio, numpy as np
arr = np.array([[1000, 2000, 3000, 4000]], dtype='uint16')
with rasterio.open('/tmp/r.tif', 'w', driver='GTiff',
                   height=1, width=4, count=1, dtype='uint16',
                   compress='deflate', predictor=2) as dst:
    dst.write(arr, 1)

from xrspatial.geotiff._reader import read_to_array
print(read_to_array('/tmp/r.tif')[0])  # before: [1000 1744 2488 3232]

Test plan

  • New test_predictor2_samplewise.py: rasterio (libtiff) round-trips for LE uint16/int16/uint32/int32 with carries
  • tifffile round-trips for big-endian uint16/int16/uint32/int32
  • Multi-band chunky uint16 round-trip against libtiff
  • xrspatial-written uint16 readable by rasterio (interop the other direction)
  • GPU multi-byte decode parity for libtiff predictor=2 files
  • Existing predictor and writer test suites still pass (CPU and GPU paths)

…-trip with libtiff

The horizontal-differencing predictor was implemented as a byte-stream
cumulative sum at a sample stride. That works for 8-bit data but loses
carries between bytes inside the same multi-byte sample, so any difference
between adjacent samples that crossed a byte boundary came out wrong. Files
written by xrspatial read back fine because both halves were wrong in
matching ways, but TIFFs from libtiff/GDAL with predictor=2 and uint16,
int16, uint32 or int32 samples decoded silently to incorrect pixel values.

Now the decode operates on whole sample values, viewed as the file's
sample dtype with the file's byte order, and per-channel for chunky
multi-band data. The 8-bit byte-wise fast path stays. Predictor=2 on
floats stays byte-wise to keep the historical xrspatial round-trip
working (the spec restricts predictor=2 to integers; floats are out of
spec for this predictor).

The GPU decode and encode kernels gained sample-wise variants and the
encode dispatch was also fixed: it had been called with row_bytes
multiplied by samples-per-pixel twice, which only happened to be inert
because the byte-wise math was wrong on both sides.

New regression tests in test_predictor2_samplewise.py read libtiff- and
tifffile-written files (LE and BE; uint16, int16, uint32, int32; chunky
multi-band) and confirm exact equality on both CPU and GPU paths.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 6, 2026
@brendancol
Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #1498 (already merged). The audit agent that opened this PR was working from a stale local main that didn't have #1498 yet, so it re-derived the same fix. Verified: with #1498 applied, the repro [1000, 2000, 3000, 4000] uint16 predictor=2 round-trips correctly through rasterio.

@brendancol brendancol closed this May 6, 2026
brendancol added a commit that referenced this pull request May 6, 2026
The third-pass agent worked from a stale local main and re-derived the
predictor=2 sample-wise fix that already shipped in #1498. That PR has been
closed as a duplicate. Only the predictor=3 big-endian fix in #1500 is a
genuine new finding.
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.

1 participant