Repository navigation
fix: keep the bytes after a Hue/Saturation block's range records (#645) - #794
Merged
kyamagu merged 5 commits intoSep 9, 2026
Merged
Conversation
Photoshop writes a 136-byte 'hue2' block. HueSaturation.read() in src/psd_tools/psd/adjustments.py models the 100 bytes the format documents -- version, colorization, master and six range records -- and BaseElement.frombytes() drops whatever is left in the block's stream, so write() emitted 100 bytes and the remaining 36 were lost on every save. Those 36 bytes are six (hue, 100, 50) triples, one per range, and they vary with the layer: across the 16 Hue/Saturation blocks in the fixtures each triple's hue is the midpoint of that range's inner band, which the user moves. They are data, not padding, which is what #645 read them as. Capture them in an `unknown: bytes` field and write them back, the same way ChannelMixer two structures above already handles its own trailer. Files written by older Photoshop stop at 100 bytes and leave the field empty, so they are unaffected. Sweeping every tagged block in all 295 fixture files through read then write, HUE_SATURATION was the only key whose bytes changed apart from the known 2-byte UNICODE_LAYER_NAME padding of the non-Adobe files already in SKIP_BYTE_ROUND_TRIP. The four huesaturation fixtures now reproduce byte for byte, so test_psd_read_write drops its xfail.
kyamagu
reviewed
Sep 6, 2026
kyamagu
left a comment
kyamagu
left a comment
Contributor
There was a problem hiding this comment.
Thanks for this — I checked the claim independently and it holds: decoding every hue2 block under tests/psd_files/adjustments/ gives 16 blocks of 136 bytes whose 36-byte tail is six (hue, 100, 50) triples, and the hue values genuinely vary per layer (e.g. huesaturation_saturation_rgb.psd layer 1 → 134 for inner band 111..157). So it is data, not padding, and preserving it verbatim is the right call. Full suite on the branch: 2890 passed, 3 skipped, 21 xfailed, 2 xpassed — the four HUESATURATION_MISMATCH xfails are legitimately gone.
One small thing about the write path, inline.
docs/changelog.rst was the only conflict: both sides opened a 1.19.1 (unreleased) section. Kept every entry, with the two stroke entries from main above this branch's Hue/Saturation entry, which matches how main prepends the newest entry to the section.
`unknown` is read with `fp.read()` to the end of the block, so it already carries whatever padding the producer wrote. Padding again after writing it grew a block that was not already 4-byte aligned: a 102-byte `hue2` payload came back 104. Write the bytes and stop, the way `ChannelMixer.write` does with the same `unknown: bytes` field.
The existing cases are 100 and 136 bytes, both already aligned, so neither caught the write path adding padding of its own. 102 and 103 bytes both came back 104 before the previous commit.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The code change is small, targeted, and is backed by regression tests that demonstrate the previously failing byte-for-byte round-trip now passes.
Pull request overview
Fixes byte-for-byte round-tripping for modern Photoshop Hue/Saturation adjustment tagged blocks by preserving the trailing bytes that psd-tools previously dropped during parse/serialize.
Changes:
- Capture and re-emit the remaining bytes after Hue/Saturation range records via a new
unknown: bytesfield. - Add/extend regression tests for Hue/Saturation block round-tripping (with and without trailing bytes).
- Remove the now-unnecessary
xfailexceptions for Hue/Saturation fixtures in byte-for-byte PSD round-trip tests and document the fix in the changelog.
File summaries
| File | Description |
|---|---|
src/psd_tools/psd/adjustments.py |
Preserve trailing Hue/Saturation block bytes by reading/writing an unknown byte payload. |
tests/psd_tools/psd/test_adjustments.py |
Add Hue/Saturation block fixtures/tests to ensure trailing bytes are preserved. |
tests/psd_tools/psd/test_psd.py |
Remove Hue/Saturation-specific xfail logic from byte-for-byte round-trip tests. |
docs/changelog.rst |
Add a 1.19.1 (unreleased) entry describing the Hue/Saturation round-trip fix. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Contributor
|
@MaxFreedomPollard Thanks! |
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.
Saving a file whose Hue/Saturation layers came from a recent Photoshop drops 36 bytes out of each
hue2block. The block Photoshop writes is 136 bytes; psd-tools writes 100.HueSaturation.read()atsrc/psd_tools/psd/adjustments.py:515reads exactly the 100 bytes the format documents: version and colorization flag, the colorization triple, the master triple, and six range records of four range values plus three settings. It never touches the rest of the stream, andBaseElement.frombytes()atsrc/psd_tools/psd/base.py:73handsread()aBytesIOover the whole block and throws away what is left in it, so the trailing bytes are gone by the timewrite()atsrc/psd_tools/psd/adjustments.py:533runs, and the block comes back out 36 bytes shorter.Those 36 bytes are six
(hue, 100, 50)triples, one per range, and they are not constant. Across the 16 Hue/Saturation blocks intests/psd_files/adjustments/each triple's hue is the midpoint of that range's inner band, which moves when the user drags the range sliders: inhuesaturation_saturation_rgb.psdone layer's first range is(103, 111, 157, 165)and its first trailing triple is(134, 100, 50). That is why #645 came out 36 bytes per layer short, and it is data rather than the padding difference the issue was closed as.The fix adds an
unknown: bytesfield that captures the remainder and writes it back, the same wayChannelMixertwo structures above in the same file already handles its own trailer. Files from older Photoshop stop at 100 bytes and leave the field empty, so nothing about them changes.To check that no other block was doing this, I monkeypatched
TaggedBlock.readto re-encode every block it parsed and compare against the raw bytes, then ran it over all 295 fixture files. Before the change: 16HUE_SATURATIONblocks shrinking 136 to 100, plus 4UNICODE_LAYER_NAMEblocks growing by 2 bytes, which is the known non-Adobe padding already listed inSKIP_BYTE_ROUND_TRIP. After the change: only the 4UNICODE_LAYER_NAMEblocks.Verification, all on Python 3.11 with
uv sync --group dev --extra composite:uv run pytest --no-cov -qgives2890 passed, 3 skipped, 21 xfailed, 2 xpassedagainst2883 passed, 3 skipped, 25 xfailed, 2 xpassedonmain. The four fewer xfails are the four huesaturation fixtures, which now reproduce byte for byte, sotest_psd_read_writeno longer needsHUESATURATION_MISMATCH.uv run pytest --no-cov -q tests/psd_tools/psd/test_adjustments.py -k hue_saturationgives3 passed. Reverting onlysrc/psd_tools/psd/adjustments.pyand rerunning gives2 failed, 1 passed:test_hue_saturation_rwwith the trailer fails with136 vs 100, andtest_hue_saturation_keeps_trailing_bytesfails withAttributeError: 'HueSaturation' object has no attribute 'unknown'. The parametrised case with no trailer passes either way, which is the older 100-byte shape staying put.uv run ruff checkgivesAll checks passed!,uv run ruff format --check .gives110 files already formatted, anduv run mypygivesSuccess: no issues found in 109 source files.Changelog entry added under a new
1.19.1 (unreleased)heading.