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

adaptive_resize cropping issue #26

Closed
scazzy opened this issue Oct 10, 2013 · 18 comments
Closed

adaptive_resize cropping issue #26

scazzy opened this issue Oct 10, 2013 · 18 comments
Labels

Comments

@scazzy
Copy link

scazzy commented Oct 10, 2013

No description provided.

@claviska
Copy link
Owner

Have you tried the fill method?

@scazzy
Copy link
Author

scazzy commented Oct 10, 2013

Yeah. That will fill entire image into white, ie. there would remain no image, just the color

@claviska
Copy link
Owner

Can you create the background image using fill and then overlay it with the PNG?

@scazzy
Copy link
Author

scazzy commented Oct 10, 2013

Also would be great if you can tell me how can I use box_crop in SimpleImage.
First adaptive_resize or best_fit to fit within size. and then put that in a box of the required size centered.
For example, a facebook profile photo. I want to adaptive resize the photo, but don't want to leave it with it's own dimensions, but fixed dimenstions, say 180x180.
Hope you understood the situation. lemme know if I need to explain further.

@claviska
Copy link
Owner

Adaptive resize should be all you need:

This function attempts to get the image to as close to the provided dimensions as possible, and then crops the remaining overflow (from the center) to get the image to be the size specified

Let me know if the fill/overlay solution works.

@scazzy
Copy link
Author

scazzy commented Oct 10, 2013

  1. Fill with overlay should work. but fill won't work without an image. Ofcourse can do by manual coding, but with SimpleImage.php, u can't create an empty image with some bg that you can overlay with.
  2. I did try adaptive_resize assuming it the same, but it won't work. it instead crops image after a point.

Screendump http://i.imgur.com/CwKoE2B.png

@claviska
Copy link
Owner

Actually, just use create:

$img = new SimpleImage();
$img->create(100, 100, '#0088cc')->overlay('overlay.png')->save('result.png');

I just tested this using overlay.png in the example directory and it works fine. (You could also have used fill, but you'd need to do it on an existing image.)

For adaptive resize, it will resize the image as close as possible and then trim (crop) the edges. I'm not sure why your image obtained a black background...just tried with a similar PNG and it resized it with transparency:

$img = new SimpleImage();
$img->load('firefox.png')->adaptive_resize(50, 50)->save('result.png');

Closing this since the original issue is resolved.

@scazzy
Copy link
Author

scazzy commented Oct 10, 2013

Ok. You can close.
But that won't work still.

  1. overlay excepts image path. So even if you try to save the uploaded image (which has turned black background), it makes no sense. plus one cannot use the temporary uploaded image ($_FILE['file']['tmp..']) as a parm for overlay()
  2. adaptive_resize won't work. Do try this image https://upload.wikimedia.org/wikipedia/commons/6/6d/Mozilla_Firefox_logo.png

:)

@claviska
Copy link
Owner

For #1, look at the example again. It's creating a filled image from scratch, then overlaying overlay.png on top, and saving it as result.png. Using that image as an example (filename changed for my own sanity):

$img = new SimpleImage();
$img->create(500, 500, '#0088cc')->overlay('firefox.png')->save('result.png');

Results in this image:

result

one cannot use the temporary uploaded image ($_FILE['file']['tmp..']) as a parm for overlay()

I'm using many methods on files that are uploaded—they are in a temp directory so you should be able to read/write to them. Maybe talk to your host about why you can't read/write to the system temp directory, or try using tempname() to create a file that you'll be able to read/write to. This issue is really outside the scope of the library.

For #2, using the same code and your image:

$img = new SimpleImage();
$img->load('firefox.png')->adaptive_resize(50, 50)->save('result.png');

Here is the resulting image:

result

@scazzy
Copy link
Author

scazzy commented Oct 10, 2013

#1 is okay. my bad. I was passing the simple's $img object ;)
#2, try a size like adaptive_resize(500, 250)

How it should be -> http://i.imgur.com/IWHBSbP.png
How it is http://i.imgur.com/L6tLqbB.png

And thanks for the help on #1 :)

@scazzy
Copy link
Author

scazzy commented Oct 10, 2013

#2 though it can be solved by, again, using a pseudo image and overlapping it with actual image using best_fit.

@claviska
Copy link
Owner

I'll have to take a closer look at adaptive_resize...I didn't write that method so I'm not sure if that's the intended behavior or not. If I remember correctly, it replaced two other methods—one of them being square_crop() which is what I think you really need here.

I don't have time today, but hopefully by this weekend I'll be able to address this. You should be able to find square_crop in the commit history if you want to try it out. Chances are, if adaptive_resize is behaving as expected, I'll add square_crop back. Otherwise, I'll just fix adaptive_resize so it works properly.

Ultimately, we need to document exactly what behavior adaptive_resize has on portrait, landscape, and square-shaped images and then make sure it's doing that.

Reopening under a different title.

@claviska claviska reopened this Oct 10, 2013
@scazzy
Copy link
Author

scazzy commented Oct 10, 2013

#Appreciate.
Also, would suggest to add the png-background func in your class.
Am adding in mine, but if you add it directly in your class, would help ppl save some lines of code for each image.

#Logic
function fillBackground($color=#fff,$width,$height){
if(!$width) $width = $this->get_width; // same width of the image loaded in the instance
if(!$height) $width = $this->get_width; // same width of the image loaded in the instance
// Create and overlay code
....
}

@claviska
Copy link
Owner

It turns out adaptive_resize is working correctly according to the comments:

This function attempts to get the image to as close to the provided dimensions as possible, and then crops the remaining overflow (from the center) to get the image to be the size specified.

However, I'm wondering what use cases exist for this behavior. It seems to me that the resulting image should be scaled to fit inside the new dimensions (like you suggest) instead of being cropped.

@nazar-pc - do you have any thoughts about why adaptive_resize works this way? (re: this comment)

@nazar-pc
Copy link
Collaborator

adaptive_resize works such way to be used in thumbnails. Usually, thumbnails have some specific dimentions, and it is better to crop something, than to leave empty space.
Probably, adaptive_resize can be renamed to adaptive_crop, and to add adaptive_resize with additional parameter of color for filling empty space.

@claviska
Copy link
Owner

Makes sense...unfortunately that introduces an API change and would mean we'd have to bump to 3.0. Instead, let's deprecate adaptive_resize() and re-introduce the same functionality under a different name—perhaps thumbnail(). adaptive_resize() will remain intact until 3.0, per SemVer. It will simply become an alias of thumbnail() until then.

I think best_fit can have an optional third argument that will maintain the specified size, which would be a useful additional and will solve @scazzy's case.

I'll post an update shortly.

@claviska
Copy link
Owner

Didn't get time to add the new best_fit() argument, but you can probably get by with using create() and overlay() for now. Feel free to contribute a patch if you'd like :)

@scazzy
Copy link
Author

scazzy commented Oct 31, 2013

Cool. sounds great to add thumbnail() method.

Yeah I saw changes for the create() method. Thanks. I'll try to make a workaround until then using this.

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

No branches or pull requests

3 participants