rkalla / imgscalr

Simple Java image-scaling library implementing Chris Campbell's incremental scaling algorithm as well as Java2D's "best-practices" image-scaling techniques.
Apache License 2.0
1.23k stars 243 forks source link

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

Closed ghost closed 10 years ago

ghost commented 11 years ago

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 commented 10 years ago

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

ghost commented 10 years ago

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

elygre commented 10 years ago

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

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 commented 10 years ago

What would become the pull request is currently available at https://github.com/elygre/imgscalr/commit/464203f35d012217bca071d0f13ba13a9036cf6f, so you might as well comment it there. If it looks reasonable, I'll submit the pull request.

elygre commented 10 years ago

@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 commented 10 years ago

@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 commented 10 years ago

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.

ghost commented 10 years ago

Closed, feature submitted by @elygre