Skip Menu |
 

This queue is for tickets about the Data-FormValidator-Filters-Image CPAN distribution.

Report information
The Basics
Id: 40969
Status: resolved
Worked: 5 min
Priority: 0/
Queue: Data-FormValidator-Filters-Image

People
Owner: Nobody in particular
Requestors: GTERMARS [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 0.30
Fixed in: (no value)



Subject: Resizes image, even if not required
Download (untitled) / with headers
text/plain 1.1k
Saw this while trying to figure out why an animated GIF image that was being uploaded by one of my customers was rejected. The image being uploaded matched the "max width" and "max height" for the image filter _exactly_, but that DFV::F::Image was still resizing the image. By resizing the image, it got pumped through ImageMagick, which then caused the resulting file size to *grow*; the original uploaded image was ~24KBytes, but the one that ImageMagick spat out was ~44KBytes. It _looks like_ ImageMagick remapped the animation frames in the GIF and that caused the file size to increase. I'd like to submit a patch, but wasn't sure what the purpose of the undocumented "@the_rest" was in "__shrink_image()". Well, I see what its for, but can't see where its being used (in part because its undocumented). As the documentation reads, I get the impression that the image will be resized *down* to fit within the given size restrictions. If the image fits within the restrictions, though, I read the docs as implying that no resize is necessary. Can we just short-circuit this so that if the image height/width already fits within the given bounds that we just exit early without doing anything to the provided image?
Subject: your-ad-will-help.gif
Download (untitled) / with headers
text/plain 199b
Here's a quick patch that fixes the issue in my test environment. Not sure how to integrate this into your test setup, though; been using Test::More for so long that _not_ using it seems alien... :o
--- Image.pm.bk 2008-11-14 19:58:45.000000000 -0800 +++ Image.pm 2008-11-14 20:00:02.000000000 -0800 @@ -120,6 +120,14 @@ return $fh; } + if ( $max_width && ($ow <= $max_width) + && $max_height && ($oh <= $max_height) + ) { + #warn "Image does not need to be resized"; + seek( $fh, 0, 0 ); + return $fh; + } + if ( $max_width && $nw > $max_width ) { $nw = $max_width; $nh = $oh * ( $max_width / $ow );
Download (untitled) / with headers
text/plain 287b
Updated version of previous patch, which takes into consideration that we may _not_ have been given *both* the height+width as restrictions. This patch checks against the "new height/width" and compares it to the "original height/width" in order to see if the image needs to be resized.
--- Image.pm.bk 2008-11-14 19:58:45.000000000 -0800 +++ Image.pm 2008-11-14 20:09:44.000000000 -0800 @@ -129,6 +129,12 @@ $nw = $ow * ( $max_height / $oh ); } + if (($nh <= $oh) && ($nw <= $ow)) { + #warn "Image does not need to be resized"; + seek( $fh, 0, 0 ); + return $fh; + } + $result = $image->Resize( width => $nw, height => $nh, @the_rest ); if ("$result") { # quotes are there as per the Image::Magick examples #warn "$result";
Cees, I've just made a commit on my Github fork which fixes this...
Download (untitled) / with headers
text/plain 159b
Marking ticket as "resolved"; Cees pulled my fork on Github and integrated the change into his master branch. Was released as part of v0.40, on March 23, 2009


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.