From 1ffecc63f9c2596d2ad0e8eeeeb9c44f282947c2 Mon Sep 17 00:00:00 2001 From: Boris Petrov Date: Wed, 24 Jan 2024 03:33:29 +0200 Subject: [PATCH 1/2] Fix rounding inconsistencies when cropping --- lib/dotdiff/image/cropper.rb | 48 +++++++++++---------------- spec/unit/image/cropper_spec.rb | 57 +++++++-------------------------- 2 files changed, 31 insertions(+), 74 deletions(-) diff --git a/lib/dotdiff/image/cropper.rb b/lib/dotdiff/image/cropper.rb index 1389c32..c1eafd2 100644 --- a/lib/dotdiff/image/cropper.rb +++ b/lib/dotdiff/image/cropper.rb @@ -6,15 +6,27 @@ module DotDiff module Image module Cropper def crop_and_resave(element) - image = load_image(fullscreen_file) + crop_left = element.rectangle.x.floor + crop_right = (element.rectangle.x + element.rectangle.width).ceil + crop_top = element.rectangle.y.floor + crop_bottom = (element.rectangle.y + element.rectangle.height).ceil + + crop_width = crop_right - crop_left + crop_height = crop_bottom - crop_top - # @see http://www.imagemagick.org/script/command-line-options.php?#crop - crop_area = - '' + width(element, image).to_s + - 'x' + height(element, image).to_s + - '+' + element.rectangle.x.floor.to_s + - '+' + element.rectangle.y.floor.to_s + if crop_width - element.rectangle.width > 1 + crop_left += 1 + crop_width -= 1 + end + if crop_height - element.rectangle.height > 1 + crop_top += 1 + crop_height -= 1 + end + # @see https://www.imagemagick.org/script/command-line-options.php?#crop + crop_area = "#{crop_width}x#{crop_height}+#{crop_left}+#{crop_top}" + + image = load_image(fullscreen_file) image.crop crop_area image.write(cropped_file) end @@ -22,28 +34,6 @@ def crop_and_resave(element) def load_image(file) MiniMagick::Image.open(file) end - - def height(element, image) - element_height = element.rectangle.height + element.rectangle.y - image_height = image.height - - if element_height > image_height - image_height - element.rectangle.y - else - element.rectangle.height - end - end - - def width(element, image) - element_width = element.rectangle.width + element.rectangle.x - image_width = image.width - - if element_width > image_width - image_width - element.rectangle.x - else - element.rectangle.width - end - end end end end diff --git a/spec/unit/image/cropper_spec.rb b/spec/unit/image/cropper_spec.rb index faa317e..8b38298 100644 --- a/spec/unit/image/cropper_spec.rb +++ b/spec/unit/image/cropper_spec.rb @@ -44,10 +44,7 @@ def height; end allow(element).to receive(:rectangle).and_return(rectangle) expect(subject).to receive(:load_image).with('/home/se/full.png').and_return(mock_png).once - expect(subject).to receive(:width).with(element, mock_png).and_return(13).once - expect(subject).to receive(:height).with(element, mock_png).and_return(14).once - expect(mock_png).to receive(:crop).with("13x14+1+2").once expect(mock_png).to receive(:write).with('/tmp/T/cropped.png').once end @@ -56,59 +53,29 @@ def height; end { 'top' => 2, 'left' => 1, 'height' => 4, 'width' => 3 } ) + expect(mock_png).to receive(:crop).with("3x4+1+2").once + subject.crop_and_resave(element) end - it 'rounds down the x and y values received from the browser' do + it 'correctly crops when the browser returns floating point numbers' do allow(rectangle).to receive(:rect).and_return( - { 'top' => 2.3, 'left' => 1.7, 'height' => 4.5, 'width' => 3.6 } + { 'top' => 2.3, 'left' => 1.3, 'height' => 4.2, 'width' => 3.2 } ) - subject.crop_and_resave(element) - end - end - - describe '#height' do - before { allow(element.rectangle).to receive(:rect).and_return(rect) } - - context 'when element height is larger than the image height' do - let(:rect) { { 'top' => -180, 'left' => 0, 'width' => 800, 'height' => 1400 } } - - it 'returns the image height minus the top point' do - allow(mock_png).to receive(:height).and_return(1200) - expect(subject.height(element, mock_png)).to eq 1380 - end - end - - context 'when element height is smaller than the image height' do - let(:rect) { { 'top' => -180, 'left' => 0, 'width' => 500, 'height' => 800 } } + expect(mock_png).to receive(:crop).with("4x5+1+2").once - it 'returns the element height' do - allow(mock_png).to receive(:height).and_return(1200) - expect(subject.height(element, mock_png)).to eq 800 - end + subject.crop_and_resave(element) end - end - - describe '#width' do - before { allow(element.rectangle).to receive(:rect).and_return(rect) } - - context 'when element width is larger than the image width' do - let(:rect) { { 'top' => -180, 'left' => -30, 'width' => 731, 'height' => 1200 } } - it 'returns the image width minus the left point' do - allow(mock_png).to receive(:width).and_return(700) - expect(subject.width(element, mock_png)).to eq 730 - end - end + it 'correctly shifts the cropped position when rounding leads to a bigger image' do + allow(rectangle).to receive(:rect).and_return( + { 'top' => 2.9, 'left' => 1.2, 'height' => 3.6, 'width' => 2.9 } + ) - context 'when element width is smaller than the image width' do - let(:rect) { { 'top' => -180, 'left' => -20, 'width' => 800, 'height' => 800 } } + expect(mock_png).to receive(:crop).with("3x4+2+3").once - it 'returns the element width' do - allow(mock_png).to receive(:width).and_return(850) - expect(subject.width(element, mock_png)).to eq 800 - end + subject.crop_and_resave(element) end end end From ee394e769b2fbadcbbada8613b9a020283ec1657 Mon Sep 17 00:00:00 2001 From: Boris Petrov Date: Wed, 24 Jan 2024 03:57:10 +0200 Subject: [PATCH 2/2] Update `mini_magick` --- dotdiff.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotdiff.gemspec b/dotdiff.gemspec index 698700b..beb0eaf 100644 --- a/dotdiff.gemspec +++ b/dotdiff.gemspec @@ -22,7 +22,7 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ['lib'] - spec.add_runtime_dependency 'mini_magick', '>= 4.11.0' + spec.add_runtime_dependency 'mini_magick', '>= 4.12.0' spec.add_development_dependency 'bundler', '>= 2' spec.add_development_dependency 'capybara', '>= 2.6'