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

Add FIT_BOTH Mode that honors both Width and Height sizes. #94

Closed
ghost opened this issue Oct 2, 2013 · 8 comments
Closed

Add FIT_BOTH Mode that honors both Width and Height sizes. #94

ghost opened this issue Oct 2, 2013 · 8 comments
Labels

Comments

@ghost
Copy link

ghost commented Oct 2, 2013

Requested by a few folks, a mode that honors both the dimensions not just the primary one or the preferred one.

From Olli Kallioinen:

I have a certain size area reserved for displaying an image. For example 500x300 pixels. In no case must either of the image dimensions be larger than this area.

Now if an image is for example 500x400 pixels the automatic scaling sees it's landscape, and that it fits the width of 500 so it does nothing. Clearly the image still does not fit the area that was reserved for it.

elygre added a commit to elygre/imgscalr that referenced this issue Jan 28, 2014
@elygre
Copy link
Contributor

elygre commented Jan 28, 2014

This feature would be very useful to us, too. I'll see if I can submit a pull request for this tonight.

@ghost
Copy link
Author

ghost commented Jan 28, 2014

Eirik, are you thinking to distort the image or to crop to make the image meet the W/H requirements?

@elygre
Copy link
Contributor

elygre commented Jan 28, 2014

I'm thinking of an algorithm that chooses the biggest possible picture that will not expand beyond the boundaries.

  • AUTO maps into either FIT_TO_WIDTH or FIT_TO_HEIGHT
  • When using FIT_TO_WIDTH, the resize will honor width even if the height grows outside the box. Likewise, FIT_TO_HEIGHT will honor height, even if the width grows outside the box.
  • FIT_BOTH will in these cases create a smaller image, to avoid this growth. There will be no distortion or cropping, just a "smaller" resize. Proportions will be maintained, as for AUTO, WIDTH and HEIGHT mode.

For example, see https://github.com/elygre/imgscalr/blob/464203f35d012217bca071d0f13ba13a9036cf6f/src/test/java/org/imgscalr/ScalrResizeTest.java#L164, where an image of dimensions 500x250 are resized to 800x300. AUTO will adjust target size to 800x400 (ignoring the 300-limitation), while FIT_BOTH will adjust to 600x300.

BufferedImage landscape = new BufferedImage(500, 250, BufferedImage.TYPE_INT_RGB);
testResizeAutoVsBoth(landscape, 800, 300, 800, 400, 600, 300);  // FitBoth restricts y to 300, and adjusts x

The test code is built mostly for identifying and verifying the scenarios where FIT_BOTH differs from AUTO, and testResizeAutoVsBoth shows target size, expected size from auto and expected size from fit_both.

@elygre
Copy link
Contributor

elygre commented Jan 28, 2014

What would become the pull request is currently available at elygre@464203f, so you might as well comment it there. If it looks reasonable, I'll submit the pull request.

@elygre
Copy link
Contributor

elygre commented Jan 31, 2014

@thebuzzmedia, any thoughts on this one? Does my understanding match yours, or are we talking about different things here? And did you get the chance to look at the code itself?

@ghost
Copy link
Author

ghost commented Jan 31, 2014

@elygre my apologies, haven't had time to go through the code yet.

Your algorithm is effectively a BEST-FIT-BOTH solution -- in English it would be: without cropping or distortion, fit the width and height to the biggest possible dimensions that they fit within the bounding box.

I actually think this is a much better default behavior for the "AUTO" mode than what I have now (assuming one dimension is preferred over the other).

FIT_BOTH would be misleading because with that mode I would expect that if I passed in "227x333" that I would get back EXACTLY an image of dimensions "227x333" -- this introduces another opportunity for a variable, basically "STRETCH" or "CROP" in order to hit those dimensions.

In summary, what you are proposing I think should replace the AUTO behavior because it is not "FIT_BOTH", but I am not sure I am ready to pull that switch on such a long-time established default behavior without more thought.

In the back of my mind I am also balancing all these requests with what I want v5.0 to eventually look like and trying to figure out the best fit.

Thoughts?

@elygre
Copy link
Contributor

elygre commented Jan 31, 2014

We need the name to be non-confusing and as self-describing as possible, and I guess I went with the issue title (though the intended functionality there might be different). The existing ones are .AUTOMATIC, .FIT_EXACT, .FIT_TO_WIDTH and .FIT_TO_HEIGHT, and I'm good with .BEST_FIT_BOTH.

Regarding AUTOMATIC (explicit and default), I think the wiser choice is to leave it "as is". I agree that this new mode would probably be a better default, but the hassle for existing users would be much greater than the benefit for new ones. I'll be an existing user soon enough, and I like it when things stay stable :-).

I would of course like to see this coming into the library quickly, since we are of course getting ready to use it. This seems like a sweet little patch (famous last words...), and shipping a 4.3 with this would be nice.

elygre added a commit to elygre/imgscalr that referenced this issue Jan 31, 2014
elygre added a commit to elygre/imgscalr that referenced this issue Jan 31, 2014
elygre added a commit to elygre/imgscalr that referenced this issue Jan 31, 2014
elygre added a commit to elygre/imgscalr that referenced this issue Jan 31, 2014
elygre added a commit to elygre/imgscalr that referenced this issue Jan 31, 2014
@ghost
Copy link
Author

ghost commented Feb 3, 2014

Closed, feature submitted by @elygre

This issue was closed.
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

1 participant