diff --git a/.github/workflows/reusable-workflow-rspec.yml b/.github/workflows/reusable-workflow-rspec.yml index d598b6e679..d5f32ad169 100644 --- a/.github/workflows/reusable-workflow-rspec.yml +++ b/.github/workflows/reusable-workflow-rspec.yml @@ -66,6 +66,10 @@ jobs: # Dependencies + - name: Install libvips package for image processing + run: | + sudo apt-get install libvips-dev + - name: Bundle install run: | bundle config path vendor/bundle diff --git a/.github/workflows/storybook.yml b/.github/workflows/storybook.yml index 9afd3c2bb1..2307cbd9a5 100644 --- a/.github/workflows/storybook.yml +++ b/.github/workflows/storybook.yml @@ -89,6 +89,10 @@ jobs: # Dependencies + - name: Install libvips package for image processing + run: | + sudo apt-get install libvips-dev + - name: Bundle install run: | bundle config path vendor/bundle diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2aa83dd91a..9a6d54424d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -141,6 +141,10 @@ jobs: # Dependencies + - name: Install libvips package for image processing + run: | + sudo apt-get install libvips-dev + - name: Install audiowaveform package if: ${{ matrix.install-audiowaveform == true}} run: | diff --git a/app/models/pageflow/image_file.rb b/app/models/pageflow/image_file.rb index cf61b36538..b49e419f7b 100644 --- a/app/models/pageflow/image_file.rb +++ b/app/models/pageflow/image_file.rb @@ -2,6 +2,9 @@ module Pageflow class ImageFile < ApplicationRecord include UploadableFile include ImageAndTextTrackProcessingStateMachine + include OutputSource + + before_post_process :set_output_presences # used in paperclip initializer to interpolate the storage path # needs to be "processed_attachments" for images for legacy reasons @@ -35,19 +38,19 @@ def attachment_styles(attachment) panorama_format = File.extname(attachment.original_filename) == '.png' ? :PNG : :JPG Pageflow - .config.thumbnail_styles + .config.thumbnail_styles.transform_values { |options| options.merge(style_defaults) } .merge( print: {geometry: '300x300>', - format: :JPG, + **style_defaults, convert_options: '-quality 10 -interlace Plane'}, medium: {geometry: '1024x1024>', - format: :JPG, + **style_defaults, convert_options: '-quality 70 -interlace Plane'}, large: {geometry: '1920x1920>', - format: :JPG, + **style_defaults, convert_options: '-quality 70 -interlace Plane'}, ultra: {geometry: '3840x3840>', - format: :JPG, + **style_defaults, convert_options: '-quality 90 -interlace Plane'}, panorama_medium: {geometry: ImageFile.scale_down_to_cover(1024, 1024), format: panorama_format, @@ -82,5 +85,17 @@ def save_image_dimensions self.height = geo.height rescue Paperclip::Errors::NotIdentifiedByImageMagickError end + + def style_defaults + if output_present?(:webp) + {format: :webp, processors: [:pageflow_webp]} + else + {format: :JPG} + end + end + + def set_output_presences + self.output_presences = {webp: true} if entry&.feature_state('webp_images') + end end end diff --git a/app/models/pageflow/image_file_url_templates.rb b/app/models/pageflow/image_file_url_templates.rb index c75e4efcb6..eb8e25e732 100644 --- a/app/models/pageflow/image_file_url_templates.rb +++ b/app/models/pageflow/image_file_url_templates.rb @@ -2,12 +2,18 @@ module Pageflow class ImageFileUrlTemplates def call styles.each_with_object({}) do |style, result| - result[style] = UrlTemplate.from_attachment(example_file.attachment, style) + result[style] = replace_extension_with_placeholder( + UrlTemplate.from_attachment(example_file.attachment, style) + ) end end private + def replace_extension_with_placeholder(url) + url.gsub(/.JPG$/, '.:processed_extension') + end + def styles example_file.attachment_styles(example_file.attachment).keys + [:original] end diff --git a/app/views/pageflow/image_files/_image_file.json.jbuilder b/app/views/pageflow/image_files/_image_file.json.jbuilder index 351238253e..d66de3f4e8 100644 --- a/app/views/pageflow/image_files/_image_file.json.jbuilder +++ b/app/views/pageflow/image_files/_image_file.json.jbuilder @@ -1,2 +1,3 @@ json.call(image_file, :width, :height) +json.processed_extension image_file.output_present?(:webp) ? 'webp' : 'JPG' json.created_at image_file.created_at.try(:utc).try(:iso8601, 0) diff --git a/config/initializers/features.rb b/config/initializers/features.rb index 564349e370..920ca11713 100644 --- a/config/initializers/features.rb +++ b/config/initializers/features.rb @@ -1,4 +1,5 @@ Pageflow.configure do |config| + config.features.register('webp_images') config.features.register('highdef_video_encoding') config.features.register('force_fullhd_video_quality') config.features.register('selectable_themes') diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index f8fd2044a0..e818add533 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -1,5 +1,6 @@ require 'pageflow/paperclip_processors/vtt' require 'pageflow/paperclip_processors/audio_waveform' +require 'pageflow/paperclip_processors/webp' require 'pageflow/paperclip_processors/noop' Paperclip.interpolates(:pageflow_s3_root) do |_attachment, _style| @@ -33,6 +34,9 @@ end Paperclip.configure do |config| + config.register_processor(:pageflow_webp, + Pageflow::PaperclipProcessors::Webp) + config.register_processor(:pageflow_vtt, Pageflow::PaperclipProcessors::Vtt) diff --git a/config/locales/new/webp_images.de.yml b/config/locales/new/webp_images.de.yml new file mode 100644 index 0000000000..512506518a --- /dev/null +++ b/config/locales/new/webp_images.de.yml @@ -0,0 +1,4 @@ +de: + pageflow: + webp_images: + feature_name: "webp Bilder" diff --git a/config/locales/new/webp_images.en.yml b/config/locales/new/webp_images.en.yml new file mode 100644 index 0000000000..13e47a2fa6 --- /dev/null +++ b/config/locales/new/webp_images.en.yml @@ -0,0 +1,4 @@ +en: + pageflow: + webp_images: + feature_name: "webp images" diff --git a/db/migrate/20231024062501_add_output_presences_to_image_files.rb b/db/migrate/20231024062501_add_output_presences_to_image_files.rb new file mode 100644 index 0000000000..9333bbaf96 --- /dev/null +++ b/db/migrate/20231024062501_add_output_presences_to_image_files.rb @@ -0,0 +1,5 @@ +class AddOutputPresencesToImageFiles < ActiveRecord::Migration[5.2] + def change + add_column :pageflow_image_files, :output_presences, :text + end +end diff --git a/entry_types/paged/packages/pageflow-paged-react/src/files/__spec__/selectors-spec.js b/entry_types/paged/packages/pageflow-paged-react/src/files/__spec__/selectors-spec.js index 756d0a01bd..bc4b5a2d7a 100644 --- a/entry_types/paged/packages/pageflow-paged-react/src/files/__spec__/selectors-spec.js +++ b/entry_types/paged/packages/pageflow-paged-react/src/files/__spec__/selectors-spec.js @@ -79,6 +79,16 @@ describe('file', () => { expect(result).toHaveProperty('urls.high', 'http://example.com/my-video.mp4'); }); + it('interpolates processed extension into file url template', () => { + const files = {'image_files': [{id: 2004, perma_id: 31, processed_extension: 'webp', basename: 'image'}]}; + const fileUrlTemplates = {'image_files': {'medium': 'http://example.com/:basename.:processed_extension'}}; + const state = sample({files, fileUrlTemplates}); + + const result = file('imageFiles', {id: 31})(state); + + expect(result).toHaveProperty('urls.medium', 'http://example.com/image.webp'); + }); + it('interpolates hls qualities into video file url template', () => { const files = {'video_files': [{ id: 2004, @@ -231,10 +241,12 @@ describe('fileExists', () => { function sample({ files, fileUrlTemplates = { + image_files: {}, video_files: {}, text_track_files: {}, }, modelTypes = { + image_files: 'Pageflow::ImageFile', video_files: 'Pageflow::VideoFile', text_track_files: 'Pageflow::TextTrackFile' } diff --git a/entry_types/paged/packages/pageflow-paged-react/src/files/expandUrls.js b/entry_types/paged/packages/pageflow-paged-react/src/files/expandUrls.js index a1be287627..66f42772d7 100644 --- a/entry_types/paged/packages/pageflow-paged-react/src/files/expandUrls.js +++ b/entry_types/paged/packages/pageflow-paged-react/src/files/expandUrls.js @@ -38,6 +38,7 @@ function getFileUrl(collectionName, file, quality, urlTemplates) { return template .replace(':id_partition', idPartition(file.id)) .replace(':basename', file.basename) + .replace(':processed_extension', file.processedExtension) .replace(':pageflow_hls_qualities', () => hlsQualities(file)); } } diff --git a/entry_types/paged/packages/pageflow-paged-react/src/files/index.js b/entry_types/paged/packages/pageflow-paged-react/src/files/index.js index a5a5ce4a2a..3df766f3ba 100644 --- a/entry_types/paged/packages/pageflow-paged-react/src/files/index.js +++ b/entry_types/paged/packages/pageflow-paged-react/src/files/index.js @@ -17,7 +17,8 @@ export default { idAttribute: 'perma_id', attributes: [ - 'id', 'perma_id', 'basename', 'variants', 'is_ready', + 'id', 'perma_id', 'basename', 'processed_extension', + 'variants', 'is_ready', 'parent_file_id', 'parent_file_model_type', 'width', 'height', 'duration_in_ms', 'rights', 'created_at' ], diff --git a/entry_types/scrolled/package/spec/entryState/useFile-spec.js b/entry_types/scrolled/package/spec/entryState/useFile-spec.js index 65596136b0..ef40918c01 100644 --- a/entry_types/scrolled/package/spec/entryState/useFile-spec.js +++ b/entry_types/scrolled/package/spec/entryState/useFile-spec.js @@ -13,7 +13,8 @@ describe('useFile', () => { seed: { fileUrlTemplates: { imageFiles: { - large: '/image_files/:id_partition/large.jpg' + original: '/image_files/:id_partition/original/:basename.:extension', + large: '/image_files/:id_partition/large/:basename.:processed_extension', } }, fileModelTypes: { @@ -24,7 +25,8 @@ describe('useFile', () => { id: 100, permaId: 1, basename: 'image', - extension: 'jpg', + extension: 'svg', + processedExtension: 'webp', rights: 'author', configuration: { some: 'value' @@ -41,12 +43,13 @@ describe('useFile', () => { id: 100, permaId: 1, modelType: 'Pageflow::ImageFile', - extension: 'jpg', + extension: 'svg', configuration: { some: 'value' }, urls: { - large: '/image_files/000/000/100/large.jpg' + original: '/image_files/000/000/100/original/image.svg', + large: '/image_files/000/000/100/large/image.webp', } }); }); @@ -58,7 +61,8 @@ describe('useFile', () => { seed: { fileUrlTemplates: { imageFiles: { - large: '/image_files/:id_partition/large.jpg' + original: '/image_files/:id_partition/original/:basename.:extension', + large: '/image_files/:id_partition/large/:basename.:processed_extension', } }, fileModelTypes: { @@ -75,7 +79,8 @@ describe('useFile', () => { id: 100, perma_id: 1, basename: 'image', - extension: 'jpg', + extension: 'svg', + processed_extension: 'webp', rights: 'author', configuration: { some: 'value' @@ -95,12 +100,13 @@ describe('useFile', () => { permaId: 1, modelType: 'Pageflow::ImageFile', basename: 'image', - extension: 'jpg', + extension: 'svg', configuration: { some: 'value' }, urls: { - large: '/image_files/000/000/100/large.jpg' + original: '/image_files/000/000/100/original/image.svg', + large: '/image_files/000/000/100/large/image.webp', } }); }); @@ -165,50 +171,6 @@ describe('useFile', () => { }); }); - it('interpolates file basename and extension', () => { - const {result} = renderHookInEntry( - () => useFile({collectionName: 'imageFiles', permaId: 1}), - { - seed: { - fileUrlTemplates: { - imageFiles: { - original: '/image_files/:id_partition/:basename.:extension' - } - }, - fileModelTypes: { - imageFiles: 'Pageflow::ImageFile' - }, - imageFiles: [ - { - id: 100, - permaId: 1, - basename: 'image', - extension: 'svg', - rights: 'author', - configuration: { - some: 'value' - } - } - ] - } - } - ); - - const file = result.current; - - expect(file).toMatchObject({ - id: 100, - permaId: 1, - modelType: 'Pageflow::ImageFile', - configuration: { - some: 'value' - }, - urls: { - original: '/image_files/000/000/100/image.svg' - } - }); - }); - it('interpolates hls qualities into video file url templates', () => { const {result} = renderHookInEntry( () => useFile({collectionName: 'videoFiles', permaId: 1}), diff --git a/entry_types/scrolled/package/src/entryState/extendFile.js b/entry_types/scrolled/package/src/entryState/extendFile.js index 9ef136a9c9..c439bd4499 100644 --- a/entry_types/scrolled/package/src/entryState/extendFile.js +++ b/entry_types/scrolled/package/src/entryState/extendFile.js @@ -67,6 +67,7 @@ function getFileUrl(collectionName, file, quality, urlTemplates) { .replace(':id_partition', idPartition(file.id)) .replace(':basename', file.basename) .replace(':extension', file.extension) + .replace(':processed_extension', file.processedExtension) .replace(':pageflow_hls_qualities', () => hlsQualities(file)); } } diff --git a/entry_types/scrolled/package/src/entryState/watchCollections.js b/entry_types/scrolled/package/src/entryState/watchCollections.js index 04995453b5..3bc17a57fb 100644 --- a/entry_types/scrolled/package/src/entryState/watchCollections.js +++ b/entry_types/scrolled/package/src/entryState/watchCollections.js @@ -51,6 +51,7 @@ export function watchCollections(entry, {dispatch}) { name: camelize(collectionName), attributes: ['id', {permaId: 'perma_id'}, 'width', 'height', 'basename', 'extension', 'rights', + {processedExtension: 'processed_extension'}, {isReady: 'is_ready'}, {variants: variants => variants && variants.map(variant => camelize(variant))}, {durationInMs: 'duration_in_ms'}, diff --git a/lib/pageflow/paperclip_processors/webp.rb b/lib/pageflow/paperclip_processors/webp.rb new file mode 100644 index 0000000000..f691a9a804 --- /dev/null +++ b/lib/pageflow/paperclip_processors/webp.rb @@ -0,0 +1,63 @@ +require 'vips' + +module Pageflow + module PaperclipProcessors + # @api private + class Webp < Paperclip::Processor + ANIMATED_FORMATS = %w[.gif].freeze + + def initialize(file, options = {}, attachment = nil) + super + + geometry = options[:geometry].to_s + @should_crop = geometry[-1, 1] == '#' + + @target_geometry = Paperclip::Geometry.parse(geometry) + @whiny = options.fetch(:whiny, true) + + @current_format = File.extname(file.path) + @basename = File.basename(@file.path, @current_format) + end + + def make + source = @file + filename = [@basename, '.webp'].join + destination = Paperclip::TempfileFactory.new.generate(filename) + + begin + thumbnail = Vips::Image.thumbnail( + ANIMATED_FORMATS.include?(@current_format) ? "#{source.path}[n=-1]" : source.path, + width, + size: :down, + height: height, + crop: crop + ) + thumbnail.webpsave(destination.path) + rescue Vips::Error => e + if @whiny + message = "There was an error processing the thumbnail for #{@basename}:\n" + e.message + raise Paperclip::Error, message + end + end + + destination + end + + private + + def crop + return unless @should_crop + + @options[:crop] || :centre + end + + def width + @target_geometry.width + end + + def height + @target_geometry.height + end + end + end +end diff --git a/pageflow.gemspec b/pageflow.gemspec index 45f394840d..a869b7223e 100644 --- a/pageflow.gemspec +++ b/pageflow.gemspec @@ -74,6 +74,9 @@ Gem::Specification.new do |s| s.add_dependency 'paperclip', '~> 6.1' end + # Image processing + s.add_dependency 'ruby-vips', '~> 2.2' + # MySQL/Postgres advisory locks s.add_dependency 'with_advisory_lock', '~> 4.6' diff --git a/spec/helpers/pageflow/files_helper_spec.rb b/spec/helpers/pageflow/files_helper_spec.rb index a052cf95e2..376cd4d6d7 100644 --- a/spec/helpers/pageflow/files_helper_spec.rb +++ b/spec/helpers/pageflow/files_helper_spec.rb @@ -157,6 +157,28 @@ def render(helper, entry) expect(variants).to include('peakData') end end + + describe 'for image files' do + it 'includes JPG processed extension' do + entry = create(:published_entry) + create(:image_file, used_in: entry.revision) + + result = render(helper, entry) + processed_extension = json_get(result, path: ['image_files', 0, 'processed_extension']) + + expect(processed_extension).to include('JPG') + end + + it 'includes webp processed extension based based on output presence' do + entry = create(:published_entry) + create(:image_file, used_in: entry.revision, output_presences: {webp: true}) + + result = render(helper, entry) + processed_extension = json_get(result, path: ['image_files', 0, 'processed_extension']) + + expect(processed_extension).to include('webp') + end + end end end end diff --git a/spec/models/pageflow/image_file_spec.rb b/spec/models/pageflow/image_file_spec.rb index 48432eac0e..12fdb9cc29 100644 --- a/spec/models/pageflow/image_file_spec.rb +++ b/spec/models/pageflow/image_file_spec.rb @@ -17,12 +17,14 @@ module Pageflow describe 'attachment_styles' do it 'includes Pageflow.config.thumbnail_styles' do - Pageflow.config.thumbnail_styles[:square] = '100x100' + Pageflow.config.thumbnail_styles[:square] = { + geometry: '100x100', format: :JPG + } image_file = build(:image_file, :uploading, file_name: 'image.jpg') styles = image_file.attachment_styles(image_file.attachment) - expect(styles[:square]).to eq('100x100') + expect(styles[:square]).to eq(geometry: '100x100', format: :JPG) end it 'turns png file into jpg for non panorama styles' do @@ -68,6 +70,30 @@ module Pageflow expect(styles[:panorama_large].processor_options[:geometry]).to eq('100%') expect(styles[:panorama_medium].processor_options[:geometry]).to eq('100%') end + + describe 'with webp outputs' do + it 'turns jpg file into webp files' do + Pageflow.config.thumbnail_styles[:square] = { + geometry: '100x100', format: :JPG + } + + image_file = build(:image_file, + :uploading, + file_name: 'image.jpg', + output_presences: { + webp: true + }) + + styles = image_file.attachment_styles(image_file.attachment) + + expect(styles[:medium][:format]).to eq(:webp) + expect(styles[:large][:format]).to eq(:webp) + expect(styles[:ultra][:format]).to eq(:webp) + expect(styles[:square][:format]).to eq(:webp) + + expect(styles[:medium][:processors]).to eq([:pageflow_webp]) + end + end end describe 'basename' do @@ -85,5 +111,33 @@ module Pageflow expect(image_file.thumbnail_url(:medium)).to match(/placeholder/) end end + + describe '#reprocess!' do + it 'does not set output presences by default' do + entry = create(:entry) + image_file = create(:image_file, :uploaded, entry: entry) + + image_file.attachment.reprocess! + + expect(image_file.reload.present_outputs).to eq([]) + end + + it 'sets output presences based on entry feature flag' do + entry = create(:entry, with_feature: 'webp_images') + image_file = create(:image_file, :uploaded, entry: entry) + + image_file.attachment.reprocess! + + expect(image_file.reload.present_outputs).to eq([:webp]) + end + + it 'does not fail if entry is missing' do + image_file = create(:image_file, :uploaded, entry: nil) + + image_file.attachment.reprocess! + + expect(image_file.reload.present_outputs).to eq([]) + end + end end end diff --git a/spec/models/pageflow/image_file_url_templates_spec.rb b/spec/models/pageflow/image_file_url_templates_spec.rb index fddb58fcec..247cfc3a3a 100644 --- a/spec/models/pageflow/image_file_url_templates_spec.rb +++ b/spec/models/pageflow/image_file_url_templates_spec.rb @@ -7,10 +7,10 @@ module Pageflow expect(result[:large]) .to include('pageflow/image_files/attachment_on_s3s/' \ - ':id_partition/large/:basename.JPG') + ':id_partition/large/:basename.:processed_extension') end - it 'includes original URL with extname placeholder' do + it 'includes original URL' do result = ImageFileUrlTemplates.new.call expect(result[:original]) diff --git a/spec/pageflow/entry_export_import/revision_serialization_spec.rb b/spec/pageflow/entry_export_import/revision_serialization_spec.rb index ef0297eb88..2c2580b64e 100644 --- a/spec/pageflow/entry_export_import/revision_serialization_spec.rb +++ b/spec/pageflow/entry_export_import/revision_serialization_spec.rb @@ -289,7 +289,8 @@ module EntryExportImport 'entry_id', 'uploader_id', 'state', - 'updated_at')) + 'updated_at', + 'output_presences')) expect(imported_revision.file_usages.first) .to have_attributes(exported_file_usage.attributes.except('id', 'file_id', diff --git a/spec/pageflow/paperclip_processors/webp_spec.rb b/spec/pageflow/paperclip_processors/webp_spec.rb new file mode 100644 index 0000000000..a7826d5726 --- /dev/null +++ b/spec/pageflow/paperclip_processors/webp_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +module Pageflow + module PaperclipProcessors + describe Webp do + describe '#make' do + it 'shrinks jpg correctly' do + processor = Webp.new(fixture_jpg, geometry: '600x300>') + + result = processor.make + new_image = Vips::Image.new_from_file(result.path) + + expect(result.path).to end_with('.webp') + expect(new_image.width).to eq(450) + expect(new_image.height).to eq(300) + end + + it 'does not shrink if already smaller' do + processor = Webp.new(fixture_jpg, geometry: '2000x2000>') + + result = processor.make + new_image = Vips::Image.new_from_file(result.path) + + expect(result.path).to end_with('.webp') + expect(new_image.width).to eq(1200) + expect(new_image.height).to eq(800) + end + + it 'crops them correctly' do + processor = Webp.new(fixture_jpg, geometry: '600x300#') + + result = processor.make + new_image = Vips::Image.new_from_file(result.path) + + expect(result.path).to end_with('.webp') + expect(new_image.width).to eq(600) + expect(new_image.height).to eq(300) + end + + it 'handles gif files' do + processor = Webp.new(fixture_gif, geometry: '600x300>') + + result = processor.make + new_image = Vips::Image.new_from_file(result.path) + + expect(result.path).to end_with('.webp') + expect(new_image.width).to eq(1) + expect(new_image.height).to eq(1) + end + + def fixture_jpg + File.open(Engine.root.join('spec', 'fixtures', 'image.jpg')) + end + + def fixture_gif + File.open(Engine.root.join('spec', 'fixtures', 'image.gif')) + end + end + end + end +end diff --git a/spec/support/pageflow/support/config/paperclip.rb b/spec/support/pageflow/support/config/paperclip.rb index 272844b935..e1ecb41286 100644 --- a/spec/support/pageflow/support/config/paperclip.rb +++ b/spec/support/pageflow/support/config/paperclip.rb @@ -13,7 +13,7 @@ config.before(:each) do |example| unless example.metadata[:unstub_paperclip] || example.metadata[:js] - allow_any_instance_of(Paperclip::Attachment).to receive(:post_process) + allow_any_instance_of(Paperclip::Attachment).to receive(:post_process_styles) allow(Paperclip).to receive(:run).and_return('100x100') end end