Skip to content

FEATURE: automatically downsize large images#10

Open
akshayutture-augment wants to merge 1 commit into
image-processing-optimizationfrom
large-image-processing
Open

FEATURE: automatically downsize large images#10
akshayutture-augment wants to merge 1 commit into
image-processing-optimizationfrom
large-image-processing

Conversation

@akshayutture-augment

@akshayutture-augment akshayutture-augment commented Nov 17, 2025

Copy link
Copy Markdown

No description provided.

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Comment augment review to trigger a new review at any time.


def self.optimize(operation, from, to, width, height, opts={})
dim = dimensions(width, height)
def self.downsize(from, to, dimensions, opts={})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining def self.downsize(from, to, dimensions, opts={}) here overrides the earlier downsize(from, to, max_width, max_height, opts={}) (Ruby uses the last definition), changing arity and causing ArgumentError at existing call sites that pass width/height (e.g., Jobs::ResizeEmoji).

🤖 Was this useful? React with 👍 or 👎

if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename)
attempt = 5
while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes
OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing a percentage string ("80%") to OptimizedImage.downsize will propagate to the animated path (gifsicle --resize-fit #{dimensions}), which typically expects WxH, so GIF downsizing can fail when allow_animation is true. This can leave large GIFs unshrunk and still over the site limit after the loop (also applies to other locations in the PR).

🤖 Was this useful? React with 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants