diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index b5e72a0973b..4c707ab5cc8 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -9,7 +9,7 @@ RUN su vscode -c "source /usr/local/share/nvm/nvm.sh && nvm install ${NODE_VERSI # [Optional] Uncomment this section to install additional OS packages. RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \ - && apt-get -y install --no-install-recommends libicu-dev libidn11-dev ffmpeg imagemagick libpam-dev + && apt-get -y install --no-install-recommends libicu-dev libidn11-dev ffmpeg libvips42 libpam-dev # [Optional] Uncomment this line to install additional gems. RUN gem install foreman diff --git a/.github/actions/setup-ruby/action.yml b/.github/actions/setup-ruby/action.yml index 3a6fba94020..3e232f134c9 100644 --- a/.github/actions/setup-ruby/action.yml +++ b/.github/actions/setup-ruby/action.yml @@ -14,7 +14,7 @@ runs: shell: bash run: | sudo apt-get update - sudo apt-get install -y libicu-dev libidn11-dev ${{ inputs.additional-system-dependencies }} + sudo apt-get install -y libicu-dev libidn11-dev libvips42 ${{ inputs.additional-system-dependencies }} - name: Set up Ruby uses: ruby/setup-ruby@v1 diff --git a/.github/workflows/test-ruby.yml b/.github/workflows/test-ruby.yml index 3a78f8b43d4..57a7e0c6c53 100644 --- a/.github/workflows/test-ruby.yml +++ b/.github/workflows/test-ruby.yml @@ -133,7 +133,7 @@ jobs: uses: ./.github/actions/setup-ruby with: ruby-version: ${{ matrix.ruby-version}} - additional-system-dependencies: ffmpeg imagemagick libpam-dev + additional-system-dependencies: ffmpeg libpam-dev - name: Load database schema run: './bin/rails db:create db:schema:load db:seed' @@ -205,7 +205,7 @@ jobs: uses: ./.github/actions/setup-ruby with: ruby-version: ${{ matrix.ruby-version}} - additional-system-dependencies: ffmpeg imagemagick + additional-system-dependencies: ffmpeg - name: Set up Javascript environment uses: ./.github/actions/setup-javascript @@ -309,7 +309,7 @@ jobs: uses: ./.github/actions/setup-ruby with: ruby-version: ${{ matrix.ruby-version}} - additional-system-dependencies: ffmpeg imagemagick + additional-system-dependencies: ffmpeg - name: Set up Javascript environment uses: ./.github/actions/setup-javascript diff --git a/Dockerfile b/Dockerfile index a95d41a65b5..52c54c25561 100644 --- a/Dockerfile +++ b/Dockerfile @@ -97,7 +97,7 @@ RUN \ curl \ ffmpeg \ file \ - imagemagick \ + libvips42 \ libjemalloc2 \ patchelf \ procps \ diff --git a/Gemfile b/Gemfile index a10613b30b9..a667b2b2094 100644 --- a/Gemfile +++ b/Gemfile @@ -23,6 +23,7 @@ gem 'fog-core', '<= 2.4.0' gem 'fog-openstack', '~> 1.0', require: false gem 'kt-paperclip', '~> 7.2' gem 'md-paperclip-azure', '~> 2.2', require: false +gem 'ruby-vips', '~> 2.2' gem 'active_model_serializers', '~> 0.10' gem 'addressable', '~> 2.8' diff --git a/Gemfile.lock b/Gemfile.lock index 7068f5dd55d..e46a5f90b67 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -681,6 +681,8 @@ GEM ruby-saml (1.16.0) nokogiri (>= 1.13.10) rexml + ruby-vips (2.2.1) + ffi (~> 1.12) ruby2_keywords (0.0.5) rubyzip (2.3.2) rufus-scheduler (3.9.1) @@ -925,6 +927,7 @@ DEPENDENCIES rubocop-rspec ruby-prof ruby-progressbar (~> 1.13) + ruby-vips (~> 2.2) rubyzip (~> 2.3) sanitize (~> 6.0) scenic (~> 1.7) diff --git a/app/models/concerns/account/avatar.rb b/app/models/concerns/account/avatar.rb index 39f599db182..ebf8b976926 100644 --- a/app/models/concerns/account/avatar.rb +++ b/app/models/concerns/account/avatar.rb @@ -9,7 +9,7 @@ module Account::Avatar class_methods do def avatar_styles(file) styles = { original: { geometry: '400x400#', file_geometry_parser: FastGeometryParser } } - styles[:static] = { geometry: '400x400#', format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' + styles[:static] = { geometry: '400x400#', format: 'png', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' styles end @@ -18,7 +18,7 @@ module Account::Avatar included do # Avatar upload - has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, convert_options: { all: '+profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, processors: [:lazy_thumbnail] + has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, processors: [:lazy_thumbnail] validates_attachment_content_type :avatar, content_type: IMAGE_MIME_TYPES validates_attachment_size :avatar, less_than: LIMIT remotable_attachment :avatar, LIMIT, suppress_errors: false diff --git a/app/models/concerns/account/header.rb b/app/models/concerns/account/header.rb index 44ae774e94d..aae19abbf3e 100644 --- a/app/models/concerns/account/header.rb +++ b/app/models/concerns/account/header.rb @@ -10,7 +10,7 @@ module Account::Header class_methods do def header_styles(file) styles = { original: { pixels: MAX_PIXELS, file_geometry_parser: FastGeometryParser } } - styles[:static] = { format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' + styles[:static] = { format: 'png', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' styles end @@ -19,7 +19,7 @@ module Account::Header included do # Header upload - has_attached_file :header, styles: ->(f) { header_styles(f) }, convert_options: { all: '+profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, processors: [:lazy_thumbnail] + has_attached_file :header, styles: ->(f) { header_styles(f) }, processors: [:lazy_thumbnail] validates_attachment_content_type :header, content_type: IMAGE_MIME_TYPES validates_attachment_size :header, less_than: LIMIT remotable_attachment :header, LIMIT, suppress_errors: false diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index f53da04a975..d573ed40a6d 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -170,18 +170,13 @@ class MediaAttachment < ApplicationRecord DEFAULT_STYLES = [:original].freeze - GLOBAL_CONVERT_OPTIONS = { - all: '-quality 90 +profile "!icc,*" +set date:modify +set date:create +set date:timestamp -define jpeg:dct-method=float', - }.freeze - belongs_to :account, inverse_of: :media_attachments, optional: true belongs_to :status, inverse_of: :media_attachments, optional: true belongs_to :scheduled_status, inverse_of: :media_attachments, optional: true has_attached_file :file, styles: ->(f) { file_styles f }, - processors: ->(f) { file_processors f }, - convert_options: GLOBAL_CONVERT_OPTIONS + processors: ->(f) { file_processors f } before_file_validate :set_type_and_extension before_file_validate :check_video_dimensions @@ -192,8 +187,7 @@ class MediaAttachment < ApplicationRecord has_attached_file :thumbnail, styles: THUMBNAIL_STYLES, - processors: [:lazy_thumbnail, :blurhash_transcoder, :color_extractor], - convert_options: GLOBAL_CONVERT_OPTIONS + processors: [:lazy_thumbnail, :blurhash_transcoder, :color_extractor] validates_attachment_content_type :thumbnail, content_type: IMAGE_MIME_TYPES validates_attachment_size :thumbnail, less_than: IMAGE_LIMIT diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb index 9fe02bd1681..af412f3c75a 100644 --- a/app/models/preview_card.rb +++ b/app/models/preview_card.rb @@ -55,7 +55,7 @@ class PreviewCard < ApplicationRecord has_one :trend, class_name: 'PreviewCardTrend', inverse_of: :preview_card, dependent: :destroy - has_attached_file :image, processors: [:thumbnail, :blurhash_transcoder], styles: ->(f) { image_styles(f) }, convert_options: { all: '-quality 90 +profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, validate_media_type: false + has_attached_file :image, processors: [:lazy_thumbnail, :blurhash_transcoder], styles: ->(f) { image_styles(f) }, validate_media_type: false validates :url, presence: true, uniqueness: true, url: true validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES diff --git a/app/models/site_upload.rb b/app/models/site_upload.rb index 03d472cdb2e..75d2b837160 100644 --- a/app/models/site_upload.rb +++ b/app/models/site_upload.rb @@ -41,7 +41,7 @@ class SiteUpload < ApplicationRecord mascot: {}.freeze, }.freeze - has_attached_file :file, styles: ->(file) { STYLES[file.instance.var.to_sym] }, convert_options: { all: '-coalesce +profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, processors: [:lazy_thumbnail, :blurhash_transcoder, :type_corrector] + has_attached_file :file, styles: ->(file) { STYLES[file.instance.var.to_sym] }, processors: [:lazy_thumbnail, :blurhash_transcoder, :type_corrector] validates_attachment_content_type :file, content_type: %r{\Aimage/.*\z} validates :file, presence: true diff --git a/config/imagemagick/policy.xml b/config/imagemagick/policy.xml deleted file mode 100644 index e2aa202f274..00000000000 --- a/config/imagemagick/policy.xml +++ /dev/null @@ -1,27 +0,0 @@ - - - - - - - - - - - - - - - - diff --git a/config/initializers/vips.rb b/config/initializers/vips.rb new file mode 100644 index 00000000000..578c5b99efd --- /dev/null +++ b/config/initializers/vips.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +Vips.block_untrusted(true) if Vips.at_least_libvips?(8, 13) diff --git a/lib/paperclip/blurhash_transcoder.rb b/lib/paperclip/blurhash_transcoder.rb index c22c20c57ad..3f5a397fb6e 100644 --- a/lib/paperclip/blurhash_transcoder.rb +++ b/lib/paperclip/blurhash_transcoder.rb @@ -5,10 +5,9 @@ module Paperclip def make return @file unless options[:style] == :small || options[:blurhash] - pixels = convert(':source -depth 8 RGB:-', source: File.expand_path(@file.path)).unpack('C*') - geometry = options.fetch(:file_geometry_parser).from_file(@file) + image = Vips::Image.thumbnail(@file.path, 100) - attachment.instance.blurhash = Blurhash.encode(geometry.width, geometry.height, pixels, **(options[:blurhash] || {})) + attachment.instance.blurhash = Blurhash.encode(image.width, image.height, image.to_a.flatten, **(options[:blurhash] || {})) @file end diff --git a/lib/paperclip/color_extractor.rb b/lib/paperclip/color_extractor.rb index d2f7e7c6025..6d4491e083b 100644 --- a/lib/paperclip/color_extractor.rb +++ b/lib/paperclip/color_extractor.rb @@ -7,15 +7,23 @@ module Paperclip MIN_CONTRAST = 3.0 ACCENT_MIN_CONTRAST = 2.0 FREQUENCY_THRESHOLD = 0.01 + BINS = 10 def make - depth = 8 + image = Vips::Image.new_from_file(@file.path, access: :random).thumbnail_image(100) + block_edge_dim = (image.height * 0.25).floor + line_edge_dim = (image.width * 0.25).floor - # Determine background palette by getting colors close to the image's edge only - background_palette = palette_from_histogram(convert(':source -alpha set -gravity Center -region 75%x75% -fill None -colorize 100% -alpha transparent +region -format %c -colors :quantity -depth :depth histogram:info:', source: File.expand_path(@file.path), quantity: 10, depth: depth), 10) + edge_image = begin + top = image.crop(0, 0, image.width, block_edge_dim) + bottom = image.crop(0, image.height - block_edge_dim, image.width, block_edge_dim) + left = image.crop(0, block_edge_dim, line_edge_dim, image.height - (block_edge_dim * 2)) + right = image.crop(image.width - line_edge_dim, block_edge_dim, line_edge_dim, image.height - (block_edge_dim * 2)) + top.join(bottom, :vertical).join(left, :horizontal).join(right, :horizontal) + end - # Determine foreground palette from the whole image - foreground_palette = palette_from_histogram(convert(':source -format %c -colors :quantity -depth :depth histogram:info:', source: File.expand_path(@file.path), quantity: 10, depth: depth), 10) + background_palette = palette_from_image(edge_image) + foreground_palette = palette_from_image(image) background_color = background_palette.first || foreground_palette.first foreground_colors = [] @@ -78,6 +86,28 @@ module Paperclip private + def palette_from_image(image) + histogram = image.hist_find_ndim(bins: BINS) + _, colors = histogram.max(size: 10, out_array: true, x_array: true, y_array: true) + + colors['out_array'].map.with_index do |v, i| + x = colors['x_array'][i] + y = colors['y_array'][i] + + rgb_from_xyv(histogram, x, y, v) + end.reverse + end + + # rubocop:disable Naming/MethodParameterName + def rgb_from_xyv(image, x, y, v) + pixel = image.getpoint(x, y) + z = pixel.find_index(v) + r = (x + 0.5) * 256 / BINS + g = (y + 0.5) * 256 / BINS + b = (z + 0.5) * 256 / BINS + ColorDiff::Color::RGB.new(r, g, b) + end + def w3c_contrast(color1, color2) luminance1 = (color1.to_xyz.y * 0.01) + 0.05 luminance2 = (color2.to_xyz.y * 0.01) + 0.05 @@ -89,7 +119,6 @@ module Paperclip end end - # rubocop:disable Naming/MethodParameterName def rgb_to_hsl(r, g, b) r /= 255.0 g /= 255.0 @@ -170,18 +199,6 @@ module Paperclip ColorDiff::Color::RGB.new(*hsl_to_rgb(hue, saturation, light)) end - def palette_from_histogram(result, quantity) - frequencies = result.scan(/([0-9]+):/).flatten.map(&:to_f) - hex_values = result.scan(/\#([0-9A-Fa-f]{6,8})/).flatten - total_frequencies = frequencies.sum.to_f - - frequencies.map.with_index { |f, i| [f / total_frequencies, hex_values[i]] } - .sort_by { |r| -r[0] } - .reject { |r| r[1].size == 8 && r[1].end_with?('00') } - .map { |r| ColorDiff::Color::RGB.new(*r[1][0..5].scan(/../).map { |c| c.to_i(16) }) } - .slice(0, quantity) - end - def rgb_to_hex(rgb) format('#%02x%02x%02x', rgb.r, rgb.g, rgb.b) end diff --git a/lib/paperclip/lazy_thumbnail.rb b/lib/paperclip/lazy_thumbnail.rb index 10b14860c4a..e749a9b7672 100644 --- a/lib/paperclip/lazy_thumbnail.rb +++ b/lib/paperclip/lazy_thumbnail.rb @@ -1,24 +1,137 @@ # frozen_string_literal: true module Paperclip - class LazyThumbnail < Paperclip::Thumbnail + class LazyThumbnail < Paperclip::Processor + ALLOWED_FIELDS = %w( + width + height + bands + format + coding + interpretation + icc-profile-data + page-height + n-pages + loop + delay + ).freeze + + class PixelGeometryParser + def self.parse(current_geometry, pixels) + width = Math.sqrt(pixels * (current_geometry.width.to_f / current_geometry.height)).round.to_i + height = Math.sqrt(pixels * (current_geometry.height.to_f / current_geometry.width)).round.to_i + + Paperclip::Geometry.new(width, height) + end + end + + def initialize(file, options = {}, attachment = nil) + super + + @crop = options[:geometry].to_s[-1, 1] == '#' + @current_geometry = options.fetch(:file_geometry_parser, Geometry).from_file(@file) + @target_geometry = options[:pixels] ? PixelGeometryParser.parse(@current_geometry, options[:pixels]) : options.fetch(:string_geometry_parser, Geometry).parse(options[:geometry].to_s) + @format = options[:format] + @current_format = File.extname(@file.path) + @basename = File.basename(@file.path, @current_format) + + correct_current_format! + correct_target_geometry! + end + def make return File.open(@file.path) unless needs_convert? - if options[:geometry] - min_side = [@current_geometry.width, @current_geometry.height].min.to_i - options[:geometry] = "#{min_side}x#{min_side}#" if @target_geometry.square? && min_side < @target_geometry.width - elsif options[:pixels] - width = Math.sqrt(options[:pixels] * (@current_geometry.width.to_f / @current_geometry.height)).round.to_i - height = Math.sqrt(options[:pixels] * (@current_geometry.height.to_f / @current_geometry.width)).round.to_i - options[:geometry] = "#{width}x#{height}>" - end + dst = TempfileFactory.new.generate([@basename, @format ? ".#{@format}" : @current_format].join) - Paperclip::Thumbnail.make(file, options, attachment) + transformed_image.write_to_file(dst.path, **save_options) + + dst end private + def correct_target_geometry! + min_side = [@current_geometry.width, @current_geometry.height].min.to_i + @target_geometry = Paperclip::Geometry.new(min_side, min_side) if @target_geometry&.square? && min_side < @target_geometry.width + end + + def correct_current_format! + # If the attachment was uploaded through a base64 payload, the tempfile + # will not have a file extension. It could also have the wrong file extension, + # depending on what the uploaded file was named. We correct for this in the final + # file name, which is however not yet physically in place on the temp file, so we + # need to use it here. Mind that this only reliably works if this processor is + # the first in line and we're working with the original, unmodified file. + @current_format = File.extname(attachment.instance_read(:file_name)) + end + + def transformed_image + # libvips has some optimizations for resizing an image on load. If we don't need to + # resize the image, we have to load it a different way. + if @target_geometry.nil? + return Vips::Image.new_from_file(preserve_animation? ? "#{@file.path}[n=-1]" : @file.path, access: :sequential).copy.mutate do |mutable| + (mutable.get_fields - ALLOWED_FIELDS).each do |field| + mutable.remove!(field) + end + end + end + + # libvips thumbnail operation does not work correctly on animated GIFs. If we need to + # preserve the animation, we have to load all the frames and then manually crop + # them to then reassemble. + if preserve_animation? + original_image = Vips::Image.new_from_file("#{@file.path}[n=-1]", access: :sequential) + n_pages = original_image.get('n-pages') + + # The loaded image has each frame stacked on top of each other, therefore we must + # account for this when giving the resizing constraint, otherwise the width will + # always end up smaller than we want. + resized_image = original_image.thumbnail_image(@target_geometry.width, height: @target_geometry.height * n_pages, size: :down).mutate do |mutable| + (mutable.get_fields - ALLOWED_FIELDS).each do |field| + mutable.remove!(field) + end + end + + # If we don't need to crop the image, then we're already done. Otherwise, + # we need to manually crop each frame of the animation and reassemble them. + return resized_image unless @crop + + page_height = resized_image.get('page-height') + + frames = (0...n_pages).map do |i| + resized_image.crop(0, i * page_height, @target_geometry.width, @target_geometry.height) + end + + Vips::Image.arrayjoin(frames, across: 1).copy.mutate do |mutable| + mutable.set!('page-height', @target_geometry.height) + end + else + Vips::Image.thumbnail(@file.path, @target_geometry.width, height: @target_geometry.height, **thumbnail_options).mutate do |mutable| + (mutable.get_fields - ALLOWED_FIELDS).each do |field| + mutable.remove!(field) + end + end + end + end + + def thumbnail_options + @crop ? { crop: :centre } : { size: :down } + end + + def save_options + case @format + when 'jpg' + { Q: 90, interlace: true } + else + {} + end + end + + def preserve_animation? + @format == 'gif' || (@format.blank? && @current_format == '.gif') + end + def needs_convert? needs_different_geometry? || needs_different_format? || needs_metadata_stripping? end @@ -29,7 +142,7 @@ module Paperclip end def needs_different_format? - @format.present? && @current_format != @format + @format.present? && @current_format != ".#{@format}" end def needs_metadata_stripping? diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 1b9a13c38c2..5dd26142ab5 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -203,7 +203,7 @@ RSpec.describe MediaAttachment, :paperclip_processing do expect(media.type).to eq 'audio' expect(media.file.meta['original']['duration']).to be_within(0.05).of(0.235102) expect(media.thumbnail.present?).to be true - expect(media.file.meta['colors']['background']).to eq '#3088d4' + expect(media.file.meta['colors']['background']).to eq '#268cd9' expect(media.file_file_name).to_not eq 'boop.ogg' end end