Add resize_to_cover

[brendon]

This method allows one to provide a desired size to cover (width and height) and the image will be resized maintaining the ratio so that it covers that box without cropping the other oversized dimension. This behaves similar to object-fit: cover; in CSS.

I've tested that this works in my Active Storage code

image.file.variant(resize_to_cover: [696, 464])

but unfortunately I'm a bit confused about how the tests work. I need to know the dimensions of the image for the ratio calculations but the tests provide the file as a string. Is there something I can call in my resize_to_cover method to load the image so I can be sure I have the dimensions?

[brendon]

It seems to be related to assumptions around method names. Pretty tricky :D

image_processing/lib/image_processing/processor.rb

Line 9 in c20d147

if operations.dig(0, 0).to_s.start_with?("resize_") &&

My method requires knowing the dimensions of the image, so it needs to be loaded. Changing the method name so that it doesn't start with resize_ makes the test pass, but that seems like a bad workaround.

I see this PR too, that seems slightly related: https://github.com/janko/image_processing/pull/75/files

[brendon]

I've gone with renaming the method to cover and have added the method for MiniMagick also (along with tests and a CHANGELOG). I don't see anywhere for documentation other than inline so hopefully that's ok?

If you're happy, I think this one is ready to go.

[brendon]

Hi @janko, just wondering if you needed me to do anything else to this PR? I'm happy to discuss the reasons behind it if it's not making sense :)

[etherbob]

I'll close mine my PR in favor of this one.

[brendon]

Hi @janko, just following this up again. Do you have any feedback on the PR? Happy to change what needs changing.

[janko]

Thanks for the PR, and sorry for the delay. The implementation looks good to me, would you mind adding documentation to docs/minimagick.md and docs/vips.md?

[brendon]

No worries at all. I've just pushed that change now.

[janko]

@brendon I was under the impression it would be called #resize_to_cover, for consistency with other resizing methods 🤔 I read your previous comments, but didn't understand exactly what was the blocker.

[janko]

Oh right, re-read it now and got the issue. OK, I will see if I can fix that after merging, so that it can be called #resize_to_cover. Appreciate the documentation update 🙏🏻

[brendon]

Thanks @janko, yea it was a bit of a mind-bender at the time :D Weird test failures going on there (not related to this PR by the looks...)

Also happy to rename it in this PR if you make your mod on main first.

[janko]

Not sure what's up with Ruby 2.6 & 2.7 failures, but I'll go ahead and merge this, and fix that afterwards.

[brendon]

Thanks again :) Locally I was getting random test failures because of:

Errno::EMFILE: Too many open files - magick

Re-running usually let them pass. That's on macOS but I suppose it could be a similar issue on the CI.

[janko]

Oh, maybe the test suite should be explicitly cleaning out tempfiles between runs. I might have upped the OS limit for file descriptors somewhere. I'm also on macOS.

[janko]

FYI, in 9d72e46 I added support for resize-on-load and renamed it back to #resize_to_cover. Plan to release it in the next week or two.

[brendon]

Nice @janko! :) Looking forward to using it :D

[brendon]

Hi @janko, no major rush, but would you mind doing a release with this update in it in the next few weeks? :)

[janko]

@brendon Just released 1.13.0 with this change 😉

[brendon]

Thanks @janko, much appreciated :)