Make resized images exactly the size we asked for (BL-16829) - #8340
Open
JohnThomson wants to merge 1 commit into
Open
JohnThomson wants to merge 1 commit into
JohnThomson wants to merge 1 commit into
Conversation
GetDesiredImageSize truncated the non-binding dimension, and RunGraphicsMagick passed that size to -scale without the "!" flag, so GraphicsMagick treated it as a box to fit inside. The truncated dimension then became the tighter constraint and the other one came out a pixel short: a 3992x2242 photo imported as 3839x2156 instead of 3840x2157, and every publish setting lost pixels the same way (1278x718 instead of 1280x719 at the BloomPUB default). Round the non-binding dimension instead of truncating, and add "!" so GraphicsMagick produces exactly the computed size. Every size handed to GraphicsMagick comes from GetDesiredImageSize on the same file, so the forced size never distorts by more than half a pixel. Tests pin the exact output size on all three resize paths (file resize used by the folder shrink and BookCompressor, import into a book folder, and AdjustImageForDisplay for publishing), and the folder-shrink tests drop the Within(2) tolerance that had been absorbing the shortfall. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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.
Fixes BL-16829.
Problem
Every resize Bloom does through GraphicsMagick came out a pixel short of the maximum.
GetDesiredImageSizetruncated the non-binding dimension (2242 × 3840/3992 = 2156.6 → 2156), andRunGraphicsMagickpassed that to-scalewithout the!flag, so GraphicsMagick treated the geometry as a box to fit inside. The truncated height became the tighter constraint and the width came out as 3839 instead of 3840. The same happened at every BloomPUB/ePUB image setting (1278×718 instead of 1280×719 at the default).This has no visible effect for readers; images are scaled to their container by CSS and the aspect ratio was never actually wrong. It is a correctness tidy-up: the code now does what its comments, the docs, and the manual test plan say it does, and the folder-shrink tests no longer need a
Within(2)tolerance to pass.Fix
GetDesiredImageSizerounds the non-binding dimension instead of truncating.RunGraphicsMagickappends!to the-scalegeometry so GraphicsMagick produces exactly the computed size. Every size handed to GraphicsMagick comes fromGetDesiredImageSizeon dimensions read from the same file, so the forced size never distorts by more than half a pixel.Tests
New tests pin the exact output size on all three resize paths (the file-level resize used by the folder shrink and
BookCompressor, import into a book folder, andAdjustImageForDisplayat the 4K, Full HD, and default publish settings), using a generated 3992×2242 JPEG. Two existingTestGetImageSizescases update for rounding (3546.67 → 3547). The folder-shrink tests now assert exact sizes.🤖 Generated with Claude Code
Devin review
This change is