diff --git a/lib/dynamic_image/helper.rb b/lib/dynamic_image/helper.rb index 426d951..2479b83 100644 --- a/lib/dynamic_image/helper.rb +++ b/lib/dynamic_image/helper.rb @@ -54,9 +54,11 @@ def dynamic_image_path(record_or_array, options = {}) # # => Avatar def dynamic_image_tag(record_or_array, options = {}) record = extract_dynamic_image_record(record_or_array) + requested = options[:size] size = fit_size!(record_or_array, options) url_options = options.extract!(*allowed_dynamic_image_url_options) - html_options = { size:, alt: record.alt_text }.merge(options) + html_options = { size: display_size(size, requested), alt: record.alt_text } + .merge(options) image_tag(dynamic_image_path_with_size(record_or_array, size, @@ -78,8 +80,8 @@ def dynamic_image_tag(record_or_array, options = {}) # an array is negotiated against the image by {DynamicImage::FormatNegotiator}. Defaults to # {DynamicImage.default_formats}, or {DynamicImage.mailer_formats} in a mailer view. # @return [String] - # @raise [DynamicImage::Errors::InvalidSizeOptions] if crop: true is given without both dimensions, or - # if the size works out to less than a pixel + # @raise [DynamicImage::Errors::InvalidSizeOptions] if the size is zero on both axes, or if crop: true + # is given without both dimensions or leaves less than a pixel to render # # Any options supported by +polymorphic_url+ are also accepted. # @@ -218,7 +220,16 @@ def fit_size!(record_or_array, options) def image_sizing(record, size_opts, uncropped) ImageSizing .new(record, uncropped:) - .fit(size_opts[:size], size_opts).floor.to_s + .fit_renderable(size_opts[:size], size_opts).floor.to_s + end + + # Ensure either dimension is at least 1px + def display_size(size, requested) + return size unless requested + + Vector2d.parse(size) + .fit(Vector2d.parse(requested), upscale: false) + .floor.max(1).to_s end end end diff --git a/lib/dynamic_image/helper/pictures.rb b/lib/dynamic_image/helper/pictures.rb index 22da663..994046b 100644 --- a/lib/dynamic_image/helper/pictures.rb +++ b/lib/dynamic_image/helper/pictures.rb @@ -45,6 +45,8 @@ def dynamic_picture(record_or_array, options = {}) # @option options [Integer] :fallback_width The width to ask for the img, overriding # {DynamicImage.picture_fallback_width} # @return [String] the picture element + # @raise [DynamicImage::Errors::InvalidSizeOptions] if the image has less than a pixel to render at the + # requested ratio # # @example # dynamic_picture_tag(image, sizes: "50vw", alt: "A kitten") diff --git a/lib/dynamic_image/image_sizing.rb b/lib/dynamic_image/image_sizing.rb index bcc676b..5040482 100644 --- a/lib/dynamic_image/image_sizing.rb +++ b/lib/dynamic_image/image_sizing.rb @@ -114,30 +114,59 @@ def fit(fit_size, options = {}) fit_size end + # Fits the size like {#fit}, but returns the smallest size the image can be rendered at rather than raising when + # the result lands under a pixel. + # + # @param fit_size [Vector2d, String] the size to fit within, as taken by {#fit} + # @param options [Hash] as taken by {#fit} + # @return [Vector2d] the resulting size + # @raise [DynamicImage::Errors::InvalidSizeOptions] if the size is zero on both axes, or if crop: true + # is given and the crop is less than a pixel + # + # @example + # image = Image.find(params[:id]) # 2000x1 image + # sizing = DynamicImage::ImageSizing.new(image) + # + # sizing.fit_renderable("1200x") # => Vector2d(2000.0, 1.0) + def fit_renderable(fit_size, options = {}) + require_nonzero!(parse_vector(fit_size)) + return fit(fit_size, options) if options[:crop] || renderable?(fit_size, options) + + size.cover(1).round + end + + # Returns true if the image can be rendered at +fit_size+, false if {#fit} rejects it. + # + # @param fit_size [Vector2d, String] the size to fit within, as taken by {#fit} + # @param options [Hash] as taken by {#fit} + # @return [Boolean] + # + # @example + # image = Image.find(params[:id]) # 320x200 image + # sizing = DynamicImage::ImageSizing.new(image) + # + # sizing.renderable?(Vector2d(100, 0)) # => true + # sizing.renderable?(Vector2d(1, 0)) # => false + def renderable?(fit_size, options = {}) + vector = parse_vector(fit_size) + return false if options[:crop] && !(vector.x.positive? && vector.y.positive?) + return false if vector.x.zero? && vector.y.zero? + + pixels?(snap(scale(vector, options))) + end + private def crop_gravity - if uncropped? && !record.crop_gravity? - size / 2 - else - record.crop_gravity - end + uncropped? && !record.crop_gravity? ? size / 2 : record.crop_gravity end def crop_start - if uncropped? - Vector2d.new(0, 0) - else - record.crop_start - end + uncropped? ? Vector2d.new(0, 0) : record.crop_start end def size - if uncropped? - record.real_size - else - record.size - end + uncropped? ? record.real_size : record.size end # Clamps the rectangle defined by +start+ and +size+ to fit inside 0, 0 and +max_size+. It is assumed that +size+ @@ -182,12 +211,16 @@ def require_nonzero!(vector) # Rejects sizes that don't round to at least one pixel in each dimension, since there is no image to render at # that point. def require_pixels!(vector) - return if vector.x >= 1 && vector.y >= 1 + return if pixels?(vector) raise DynamicImage::Errors::InvalidSizeOptions, "#{vector} has a dimension smaller than one pixel" end + def pixels?(vector) + vector.x >= 1 && vector.y >= 1 + end + def shift_vector(vect) Vector2d.new( vect.x.negative? ? vect.x.abs : 0, diff --git a/lib/dynamic_image/picture.rb b/lib/dynamic_image/picture.rb index d97f510..c1809cb 100644 --- a/lib/dynamic_image/picture.rb +++ b/lib/dynamic_image/picture.rb @@ -28,7 +28,7 @@ class Picture # @!attribute [r] breakpoints # @return [DynamicImage::Breakpoints] the candidate widths # @!attribute [r] fallback_width - # @return [Integer] the width asked for the fallback image + # @return [Integer] the width configured for the fallback image attr_reader :template, :record_or_array, :ratio, :sizes, :breakpoints, :fallback_width, :url_options # @param template [ActionView::Base] the view context, for routing @@ -77,9 +77,13 @@ def available_width # The candidate widths, smallest first. # + # Widths the image can't be rendered at are left out. One that can't be rendered at any of them gets a single + # candidate at its available width. + # # @return [Array] def widths - @widths ||= breakpoints.widths(available_width) + @widths ||= breakpoints.widths(available_width).select { renderable?(it) }.presence || + [available_width].select { renderable?(it) } end # Every candidate, as the URL and the size it is actually rendered at. @@ -128,9 +132,12 @@ def fallback_format # The size asked for the fallback image, as a "{width}x{height}" string. # + # This is {#fallback_width}, unless the image can't be rendered that wide, in which case the widest candidate + # it can be rendered at stands in. + # # @return [String] def fallback_size - @fallback_size ||= size_for(fallback_width) + @fallback_size ||= size_for(renderable?(fallback_width) ? fallback_width : (widths.last || fallback_width)) end # The size the fallback image is actually rendered at. Smaller than {#fallback_size} when the image is. @@ -171,6 +178,10 @@ def breakpoints_from(options) DynamicImage::Breakpoints.new(options[:breakpoints], step: options[:step]) end + def renderable?(width) + sizing.renderable?(size_for(width), crop: crop?) + end + def record record_or_array.last end diff --git a/spec/dynamic_image/helper/pictures_spec.rb b/spec/dynamic_image/helper/pictures_spec.rb index 9e2141c..b5cd670 100644 --- a/spec/dynamic_image/helper/pictures_spec.rb +++ b/spec/dynamic_image/helper/pictures_spec.rb @@ -89,6 +89,15 @@ def fixture(name, content_type) end end + context "with an extremely wide image" do + # 1000x2, less than a pixel tall at any of the breakpoints + let(:image) { Image.create(file: fixture("wide.png", "image/png")) } + + it "renders the widths it can, rather than raising" do + expect(markup).to include('width="1000" height="2"') + end + end + context "with an animated image" do let(:image) { Image.create(file: fixture("animated.gif", "image/gif")) } let(:options) { {} } diff --git a/spec/dynamic_image/helper_spec.rb b/spec/dynamic_image/helper_spec.rb index 192412d..0b4d1a9 100644 --- a/spec/dynamic_image/helper_spec.rb +++ b/spec/dynamic_image/helper_spec.rb @@ -43,6 +43,28 @@ def generate_digest(str) end end + context "with an image too wide to render that small" do + # 1000x2 + let(:image) do + Image.create( + file: Rack::Test::UploadedFile.new( + File.open(File.expand_path("../support/fixtures/wide.png", + __dir__)), + "image/png" + ) + ) + end + let(:options) { { size: "400x" } } + + it "renders the smallest size that holds the whole frame" do + expect(tag).to include("/500x1/") + end + + it "lays it out at the size asked for" do + expect(tag).to include('width="400" height="1"') + end + end + context "with HTML options" do let(:options) { { size: "100x100", alt: "Foobar", class: "foo" } } let(:digest) { generate_digest("show-#{image.id}-100x62") } diff --git a/spec/dynamic_image/image_sizing_spec.rb b/spec/dynamic_image/image_sizing_spec.rb index 11d37f6..a526d5b 100644 --- a/spec/dynamic_image/image_sizing_spec.rb +++ b/spec/dynamic_image/image_sizing_spec.rb @@ -314,4 +314,74 @@ def v(width, height) it { is_expected.to eq(v(520, 500)) } end end + + describe "#fit_renderable" do + subject(:renderable) { sizing.fit_renderable(v(100, 100)) } + + it { is_expected.to eq(v(100, 62.5)) } + + context "when the result is less than a pixel tall" do + subject(:renderable) { sizing.fit_renderable(v(1, 1)) } + + it { is_expected.to eq(v(2, 1)) } + end + + context "with an extremely wide image" do + subject(:renderable) { sizing.fit_renderable("400x") } + + let(:real_size) { v(1000, 2) } + + it { is_expected.to eq(v(500, 1)) } + end + + context "with an extremely tall image" do + subject(:renderable) { sizing.fit_renderable("x400") } + + let(:real_size) { v(2, 1000) } + + it { is_expected.to eq(v(1, 500)) } + end + + context "with a crop that leaves less than a pixel" do + let(:real_size) { v(1000, 2) } + + it "raises an error" do + expect { sizing.fit_renderable(v(1, 3), crop: true) }.to( + raise_error(DynamicImage::Errors::InvalidSizeOptions) + ) + end + end + + context "with an empty size" do + it "raises an error" do + expect { sizing.fit_renderable("x") }.to( + raise_error(DynamicImage::Errors::InvalidSizeOptions) + ) + end + end + end + + describe "#renderable?" do + subject(:renderable) { sizing.renderable?(v(100, 0)) } + + it { is_expected.to be(true) } + + context "when the result is less than a pixel tall" do + subject(:renderable) { sizing.renderable?(v(1, 0)) } + + it { is_expected.to be(false) } + end + + context "with an extremely wide image" do + let(:real_size) { v(1000, 2) } + + it { is_expected.to be(false) } + end + + context "when the options rule the size out" do + subject(:renderable) { sizing.renderable?(v(100, 0), crop: true) } + + it { is_expected.to be(false) } + end + end end diff --git a/spec/dynamic_image/images_controller_spec.rb b/spec/dynamic_image/images_controller_spec.rb index 6ad29c8..dd3cb30 100644 --- a/spec/dynamic_image/images_controller_spec.rb +++ b/spec/dynamic_image/images_controller_spec.rb @@ -146,6 +146,28 @@ def get_original(image, format) end end + context "with a size only a pixel tall" do + # 1000x2, which is what a candidate width of 510 comes to + let(:image) do + Image.create( + file: Rack::Test::UploadedFile.new( + File.open(File.expand_path("../support/fixtures/wide.png", + __dir__)), + "image/png" + ) + ) + end + + before do + get :show, + params: digested(:show, id: image.id, size: "510x1", format: :png) + end + + it "renders it" do + expect(metadata.dimensions).to eq(Vector2d.new(510, 1)) + end + end + context "when format is GIF" do before do get :show, diff --git a/spec/dynamic_image/picture_spec.rb b/spec/dynamic_image/picture_spec.rb index a406391..1ad1f5b 100644 --- a/spec/dynamic_image/picture_spec.rb +++ b/spec/dynamic_image/picture_spec.rb @@ -56,6 +56,26 @@ def fixture(name, content_type) expect(picture.widths).to eq([110, 160, 220, 320]) end end + + context "with an extremely wide image" do + # 1000x2 + let(:image) { Image.create(file: fixture("wide.png", "image/png")) } + let(:options) { { breakpoints: 100..1000, step: 1.4 } } + + it "leaves out the widths it would be less than a pixel tall at" do + expect(picture.widths).to eq([510, 710, 1000]) + end + end + + context "with an image too wide for any of the breakpoints" do + # 1000x2 + let(:image) { Image.create(file: fixture("wide.png", "image/png")) } + let(:options) { { breakpoints: 100..320, step: 1.4 } } + + it "offers a single candidate at its own width" do + expect(picture.widths).to eq([1000]) + end + end end describe "#variants" do @@ -78,6 +98,16 @@ def fixture(name, content_type) expect(picture.variants.last).to include(width: 200, height: 200) end end + + context "with an extremely wide image" do + # 1000x2 + let(:image) { Image.create(file: fixture("wide.png", "image/png")) } + let(:options) { { breakpoints: 100..1000, step: 1.4 } } + + it "advertises a whole pixel of height" do + expect(picture.variants.pluck(:height)).to eq([1, 1, 2]) + end + end end describe "#srcset" do @@ -203,6 +233,20 @@ def fixture(name, content_type) it { expect(picture.width).to eq(100) } end + + context "when the image is too wide to render at the width asked for" do + # 1000x2 + let(:image) { Image.create(file: fixture("wide.png", "image/png")) } + let(:options) { { breakpoints: 100..1000, step: 1.4, fallback_width: 100 } } + + it "asks for the widest candidate it can render instead" do + expect(picture.fallback_size).to eq("1000x") + end + + it "reserves a box at least a pixel tall" do + expect([picture.width, picture.height]).to eq([1000, 2]) + end + end end describe "url options" do diff --git a/spec/support/fixtures/wide.png b/spec/support/fixtures/wide.png new file mode 100644 index 0000000..239bb51 Binary files /dev/null and b/spec/support/fixtures/wide.png differ