Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Adding JImage::cropResize #1751

Merged
merged 1 commit into from Mar 16, 2013
Merged

Adding JImage::cropResize #1751

merged 1 commit into from Mar 16, 2013

Conversation

dongilbert
Copy link
Contributor

A lot of setup is required if you would like to both resize an image down to something more manageable, and then crop that created image from the center. This is very useful for creating thumbnails that are both cropped and resized.

You can achieve a similar result using createThumbs($sizes, JImage::SCALE_FILL), but it squashes the image to fit, resulting in a skewed image. See attached imageFill.png for what this looks like.
imageFill

This method gives a clean resize and crop from center. See attached cropResize.png.
cropResize

@dongilbert
Copy link
Contributor Author

I'll add the proper documentation once #1753 gets accepted

Integrating with createThumbs method

Code style fixes
@Lemings
Copy link

Lemings commented Dec 13, 2012

t would be nice to have optional parameters to the cropResize function specifying top left offset for cropping. Let's say I want to crop 20% from top and 80% from bottom, I could set this parameter to 20 and top offset is calculated. Same happens for wide images Now it crops from right, but setting 50 would crop from both sides.

@dongilbert
Copy link
Contributor Author

My goal for cropResize was to resize the image either the given width if it's smaller or the given height if it's smaller (based on the width/height of the loaded image), and then crop. That way, you will see the either the actual top and bottom margins of the images with the sides cropped in, OR you'll see the actual left and right margins with the top and bottom cropped in.

@Lemings
Copy link

Lemings commented Dec 13, 2012

In this line it is possible to calculate pixels to be cropped before and pass this value to crop function, if it is tall image top offset, wide image - left offset. 715

  • return $this->crop($width, $height, null, null, $createNew);

Something like
if (($this->getWidth() / $width) < ($this->getHeight() / $height))
{
$this->resize($width, 0, false);
$topOffset = ($this->getHeight() - $height) * $topRate (specified in function call)
$leftOffset = null;
}
else
{
$this->resize(0, $height, false);
$leftOffset = ($this->getWidth() - $width) * $leftRate (specified in function call)
$topOffset = null
}
return $this->crop($width, $height, $leftOffset, $topOffset, $createNew);

@realityking
Copy link
Contributor

Code wise this looks fine to me. We need to document all these constants better though. Would you mind adding docblocks for all of them or at least for the one you're adding that descibe what each mode does? Thanks.

@dongilbert
Copy link
Contributor Author

Can do. (BTW - glad to see you back active. I've missed you for a month.)
👍

On Mon, Jan 7, 2013 at 6:28 PM, Rouven Weßling notifications@github.comwrote:

Code wise this looks fine to me. We need to document all these constants
better though. Would you mind adding docblocks for all of them or at least
for the one you're adding that descibe what each mode does? Thanks.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1751#issuecomment-11979121.

eddieajau added a commit that referenced this pull request Mar 16, 2013
@eddieajau eddieajau merged commit 6aca76d into joomla:staging Mar 16, 2013
@elinw
Copy link
Contributor

elinw commented Mar 18, 2013

@dongilbert Would you kindly send a pr for this to the cms?

@AmyStephen
Copy link
Contributor

And, this is what I am worried about. Can this be automated somehow?

We've already had people complain because they have been requested to share their fix with the CMS after they shared it already with the platform. Given that means they have to post to three repositories in order to share one improvement with the same project, that does make the Joomla project look very dysfunctional.

If this can't be automated, then it would be best to turn it into a job for volunteers to comb through these improvements once a week and create the patches desired. But, please do not do this by posting in after contributors have shared their improvements and ask that they do so again (and again).

@dongilbert
Copy link
Contributor Author

I don't mind, but it is a little bit of a hassle. :) The biggest heartache for me is that I haven't updated my joomla-cms clone in a long time. We'll have to figure something out, I think after 3.1 is released and the dust settles with the new framework, that this won't be as much of an issue, if at all.

@dongilbert
Copy link
Contributor Author

Does it need a tracker item for updating to latest platform version of the file?

@AmyStephen
Copy link
Contributor

It's not just you. The project needs a process for handling this that does
not involve contributors doing this three times.

I'll post on list.

On Sun, Mar 17, 2013 at 9:21 PM, Don Gilbert notifications@github.comwrote:

Does it need a tracker item for updating to latest platform version of the
file?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1751#issuecomment-15036391
.

@AmyStephen
Copy link
Contributor

Posted https://groups.google.com/forum/#!topic/joomla-dev-cms/9NqpezPCg2k

Thanks.

On Sun, Mar 17, 2013 at 9:24 PM, Amy Stephen amystephen@gmail.com wrote:

It's not just you. The project needs a process for handling this that does
not involve contributors doing this three times.

I'll post on list.

On Sun, Mar 17, 2013 at 9:21 PM, Don Gilbert notifications@github.comwrote:

Does it need a tracker item for updating to latest platform version of
the file?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1751#issuecomment-15036391
.

@dongilbert
Copy link
Contributor Author

I agree - however, as I said, this shouldn't be much of an issue in the near future. No reason to set up systems for something that's not going to last for more than another month.

@eddieajau
Copy link
Contributor

@AmyStephen we will have information coming out soon. No need to panic or cause unnecessary concern. Just be patient. Thanks.

@mariopro
Copy link
Contributor

@don, can you please confirm that the constant's SCALE_INSIDE and SCALE_OUTSIDE definitions in the docs you've written are correct. It looks like SCALE_INSIDE resizes to the smaller size and not to the larger side provided and the SCALE_OUTSIDE the opposite. I might be wrong but the tests don't give that result.

@dongilbert
Copy link
Contributor Author

I'll double check on them, I might be having them mixed up.

@mariopro
Copy link
Contributor

Thanks. Just a couple humble suggestions for the docs that might be useful for less experienced developers regarding image manipulation. Wouldn't it be useful to warn about possible image distortion in SCALE_FILL? The other is related with the "thumbnail" word that might confuse the less experienced. I would suggest using "image" instead of "thumbnail" wherever we aren't exactly dealing with a thumbnail but with a resized image. BTW, another great job!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants