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

Alternative aspect ratios, srcset, cropped images and gravity #1783

Closed
mickenorlen opened this issue Apr 9, 2020 · 1 comment
Closed

Alternative aspect ratios, srcset, cropped images and gravity #1783

mickenorlen opened this issue Apr 9, 2020 · 1 comment
Labels

Comments

@mickenorlen
Copy link
Contributor

mickenorlen commented Apr 9, 2020

Problem description

  1. Create an element with an image eg:
- name: main_image
  type: EssencePicture
  settings:
    size: 1280x720
    crop: true
  1. Save the element without cropping. Here's what is rendered in small screen 400w. Notice that srcset is not respecting aspect ratio (600x600).

Update, i realize using srcset to change aspect ratio in this way isnt good practise as the pixel density would make the breakpoints unpredictable. But the point that srcset sizes don't respect cropping is still an issue.

image

  1. Now go into the code essence_picture_view.rb And add crop: @options[:crop] to srcset picture_url
def srcset
  options[:srcset].map do |size|
    url = essence.picture_url(size: size, crop: @options[:crop])
    width, height = size.split('x')
    width.present? ? "#{url} #{width}w" : "#{url} #{height}h"
  end
end

image

That looks good! Seems like problem solved :) But no!

  1. Now crop the image
    image

Now the cropped image version takes precedence in determining aspect ratio and both srcset and size fail to override it.
image

  1. Remove crop: @options[:crop] from srcset picture_url as set in step 3. This is current behavior. In this case srcset doesn't respect neither cropping nor aspect ratio.

image

Expected behavior

I think the stated aspect ratio should always be respected if crop: true, as in images in step 3.

Possible solutions

I started working on a PR for this (#1781) while thinking the problem was simple and specific for srcset. Then i realized it was more complicated and causing problems when cropping images with size option also.

Looking through here: https://markevans.github.io/dragonfly/imagemagick
I can see 2 (possible?) solutions.

  1. First crop the image to crop_size and then crop again (possible? costly?) using (!) option:
    'widthxheight!' # force resize, don't maintain aspect ratio

  2. Or adjusting the crop_size manually before cropping (probably better performance?)
    This is basically what I did in the previous PR (see relevant code below). I haven't yet investigated much of what helper methods you have that could be of use. But the idea was to calculate the difference in aspect ratio and downsize/offset crop_size/crop_from to maintain aspect ratio and center. This should probably be done somewhere in transformations.rb#crop though. This method could also potentially take gravity options for how to crop the image to size.
    image

This would be the different sizing options when gravity x/y=center. I think gravity is very useful when creating backgrounds/banners as it would translate to background-position alignment options.
image

# Recalculate cropping to respect aspect ratio and maintain center for any size
# This example uses only "shrink" sizing option atm.
def crop_for_size(size)
  crop_size = essence.crop_size.presence

  return [nil, nil] unless @options[:crop].present? && crop_size.present?

  width, height = size.split('x').map(&:to_f)
  crop_width, crop_height = crop_size.split('x').map(&:to_f)

  crop_ratio = (crop_width / crop_height)
  size_ratio = (width.to_f / height)

  # Return same cropping if aspect ratio unchanged
  return essence.crop_from, "#{crop_width}x#{crop_height}" if "%.2f" % crop_ratio == "%.2f" % size_ratio

  crop_from_x, crop_from_y = essence.crop_from.split('x').map(&:to_i)
  new_crop_height, new_crop_width = crop_height, crop_width
  new_crop_from_x, new_crop_from_y = crop_from_x, crop_from_y

  if size_ratio > crop_ratio # new size wider => offset y and reduce height
    new_crop_height = crop_height * (crop_ratio / size_ratio)
    new_crop_from_y = crop_from_y + (crop_height - new_crop_height) / 2
  else # offset x and and reduce width
    new_crop_width = (crop_width * (size_ratio / crop_ratio))
    new_crop_from_x = crop_from_x + (crop_width - new_crop_width) / 2
  end

  new_crop_from = "#{new_crop_from_x.to_i}x#{new_crop_from_y.to_i}"
  new_crop_size = "#{new_crop_width.to_i}x#{new_crop_height.to_i}"

  [new_crop_from, new_crop_size]
end

Questions

  • If you give me a go ahead I can create a new PR and try to implement this. But please take a look at this and give me some feedback as I'm not well acquainted with dragonfly etc. Or even better if you want to look at it?
  • One thing I've been wondering about a lot when working with images is how caching works for dragonfly/Alchemy pictures. Does it matter if I put my srcset specification in the view template or in the elements.yml? What options do I have? What do recommend? Something from here?
    https://markevans.github.io/dragonfly/cache

System configuration

  • Alchemy Version: master (before webpack)
  • Rails Version: 6
@mickenorlen mickenorlen changed the title Alternative aspect ratios of cropped images Alternative aspect ratios, srcset and cropped images Apr 9, 2020
@mickenorlen mickenorlen changed the title Alternative aspect ratios, srcset and cropped images Alternative aspect ratios, srcset, cropped images and gravity Apr 10, 2020
@github-actions
Copy link

This issue has not seen any activity in a long time. If the issue described still exists in recent versions of Alchemy, please open a new issue or preferably open a PR with a fix. Thanks for reporting.

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