Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix thumbnail allow_upscale option #616

Closed
wants to merge 1 commit into from
Closed

fix thumbnail allow_upscale option #616

wants to merge 1 commit into from

Conversation

yplam
Copy link

@yplam yplam commented Jul 2, 2015

Fix thumbnail allow_upscale option

For smaller image, resize first before creating thumbnail.

Resolve #560 and #428

@makasim
Copy link
Collaborator

makasim commented Jul 2, 2015

Could you please add a test?

@lsmith77 lsmith77 added the State: Reviewing This item is being reviewed to determine if it should be accepted. label Jul 2, 2015
$ratio = max($width / $origWidth, $height / $origHeight);
$resizeFilter = new Resize(new Box(round($origWidth * $ratio), round($origHeight * $ratio)));
$image = $resizeFilter->apply($image);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is duplicate code. Why not just do:

$upscale = new UpscaleFilterLoader;
$image = $upscale->load($image, array('min' => array($width, $height));

See https://github.com/liip/LiipImagineBundle/blob/master/Imagine/Filter/Loader/UpscaleFilterLoader.php

Copy link
Contributor

Choose a reason for hiding this comment

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

cab you bake this into a PR of your own?

@lsmith77
Copy link
Contributor

ping

@lsmith77
Copy link
Contributor

lsmith77 commented Nov 1, 2015

please rebase on master to get the test fixed

@JulienItard
Copy link

Need it !

@lsmith77
Copy link
Contributor

ping

@lsmith77 lsmith77 closed this Jan 20, 2017
@lsmith77 lsmith77 removed the State: Reviewing This item is being reviewed to determine if it should be accepted. label Jan 20, 2017
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.

5 participants