diff --git a/Gemfile b/Gemfile index 999df40c..3f23fef6 100644 --- a/Gemfile +++ b/Gemfile @@ -8,7 +8,6 @@ gem 'bcrypt' gem 'sinatra-flash', require: 'sinatra/flash' gem 'sinatra-xsendfile', require: 'sinatra/xsendfile' gem 'puma', '5.6.5', require: nil -gem 'rmagick', require: nil gem 'sidekiq', '~> 5.2.0' gem 'mail' gem 'net-smtp' @@ -56,6 +55,7 @@ gem 'maxmind-db' gem 'json', '>= 2.3.0' gem 'nokogiri' gem 'rss' +gem 'webp-ffi' gem 'rszr' group :development, :test do diff --git a/Gemfile.lock b/Gemfile.lock index d8710dd1..10fdb0d0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -225,7 +225,6 @@ GEM netrc (~> 0.8) rexml (3.2.5) rinku (2.0.6) - rmagick (5.0.0) rss (0.2.9) rexml rszr (1.3.0) @@ -295,6 +294,9 @@ GEM addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) + webp-ffi (0.3.1) + ffi (>= 1.9.0) + ffi-compiler (>= 0.1.2) webrick (1.7.0) websocket-driver (0.7.5) websocket-extensions (>= 0.1.0) @@ -361,7 +363,6 @@ DEPENDENCIES redis-namespace rest-client rinku - rmagick rss rszr sanitize @@ -382,6 +383,7 @@ DEPENDENCIES tilt timecop webmock + webp-ffi will_paginate xmlrpc zipruby diff --git a/models/site.rb b/models/site.rb index ae408fcc..855dc622 100644 --- a/models/site.rb +++ b/models/site.rb @@ -1554,7 +1554,7 @@ class Site < Sequel::Model end def screenshot_path(path, resolution) - File.join base_screenshots_path, "#{path}.#{resolution}.jpg" + File.join base_screenshots_path, "#{path}.#{resolution}.webp" end def base_screenshots_path(name=username) @@ -1568,14 +1568,14 @@ class Site < Sequel::Model end def screenshot_exists?(path, resolution) - File.exist? File.join(base_screenshots_path, "#{path}.#{resolution}.jpg") + File.exist? File.join(base_screenshots_path, "#{path}.#{resolution}.webp") end def screenshot_url(path, resolution) path[0] = '' if path[0] == '/' out = '' out = 'https://neocities.org' if ENV['RACK_ENV'] == 'development' - out+"#{base_screenshots_url}/#{path}.#{resolution}.jpg" + out+"#{base_screenshots_url}/#{path}.#{resolution}.webp" end def base_thumbnails_path(name=username) @@ -1584,8 +1584,7 @@ class Site < Sequel::Model end def thumbnail_path(path, resolution) - ext = File.extname(path).gsub('.', '').match(LOSSY_IMAGE_REGEX) ? 'jpg' : 'png' - File.join base_thumbnails_path, "#{path}.#{resolution}.#{ext}" + File.join base_thumbnails_path, "#{path}.#{resolution}.webp" end def thumbnail_exists?(path, resolution) @@ -1598,8 +1597,7 @@ class Site < Sequel::Model def thumbnail_url(path, resolution) path[0] = '' if path[0] == '/' - ext = File.extname(path).gsub('.', '').match(LOSSY_IMAGE_REGEX) ? 'jpg' : 'png' - "#{THUMBNAILS_URL_ROOT}/#{sharding_dir}/#{values[:username]}/#{path}.#{resolution}.#{ext}" + "#{THUMBNAILS_URL_ROOT}/#{sharding_dir}/#{values[:username]}/#{path}.#{resolution}.webp" end def to_rss diff --git a/tests/files/img/screenshot.png b/tests/files/img/screenshot.png new file mode 100644 index 00000000..ba79519a Binary files /dev/null and b/tests/files/img/screenshot.png differ diff --git a/tests/site_file_tests.rb b/tests/site_file_tests.rb index a52ab44b..1c9b7547 100644 --- a/tests/site_file_tests.rb +++ b/tests/site_file_tests.rb @@ -446,7 +446,7 @@ describe 'site_files' do Site::THUMBNAIL_RESOLUTIONS.each do |resolution| _(File.exists?(@site.thumbnail_path('derpie/derptest/test.jpg', resolution))).must_equal true _(@site.thumbnail_url('derpie/derptest/test.jpg', resolution)).must_equal( - File.join "#{Site::THUMBNAILS_URL_ROOT}", Site.sharding_dir(@site.username), @site.username, "/derpie/derptest/test.jpg.#{resolution}.jpg" + File.join "#{Site::THUMBNAILS_URL_ROOT}", Site.sharding_dir(@site.username), @site.username, "/derpie/derptest/test.jpg.#{resolution}.webp" ) end end diff --git a/tests/workers/screenshot_worker_tests.rb b/tests/workers/screenshot_worker_tests.rb index b0259070..713cc717 100644 --- a/tests/workers/screenshot_worker_tests.rb +++ b/tests/workers/screenshot_worker_tests.rb @@ -14,14 +14,14 @@ describe ScreenshotWorker do stub_request(:get, "#{base_host}/?url=#{site.uri}/#{path}&wait_time=#{ScreenshotWorker::PAGE_WAIT_TIME}"). with(basic_auth: [uri.user, uri.password]). - to_return(status: 200, headers: {}, body: File.read('tests/files/img/test.jpg')) + to_return(status: 200, headers: {}, body: File.read('tests/files/img/screenshot.png')) ScreenshotWorker.new.perform site.username, path Site::SCREENSHOT_RESOLUTIONS.each do |r| - _(File.exists?(File.join(Site::SCREENSHOTS_ROOT, Site.sharding_dir(site.username), site.username, "#{path}.#{r}.jpg"))).must_equal true + _(File.exists?(File.join(Site::SCREENSHOTS_ROOT, Site.sharding_dir(site.username), site.username, "#{path}.#{r}.webp"))).must_equal true _(site.screenshot_url(path, r)).must_equal( - File.join(Site::SCREENSHOTS_URL_ROOT, Site.sharding_dir(site.username), site.username, "#{path}.#{r}.jpg") + File.join(Site::SCREENSHOTS_URL_ROOT, Site.sharding_dir(site.username), site.username, "#{path}.#{r}.webp") ) end end diff --git a/workers/screenshot_worker.rb b/workers/screenshot_worker.rb index d2433c27..880d3aef 100644 --- a/workers/screenshot_worker.rb +++ b/workers/screenshot_worker.rb @@ -1,4 +1,3 @@ -require 'rmagick' require 'securerandom' require 'open3' @@ -10,7 +9,6 @@ class ScreenshotWorker sidekiq_options queue: :screenshots, retry: 2, backtrace: true def perform(username, path) - site = Site[username: username] return if site.nil? || site.is_deleted @@ -37,14 +35,6 @@ class ScreenshotWorker path = "/#{path}" unless path[0] == '/' path_for_screenshot = path - - if path.match(Site::HTML_REGEX) - path = path.match(/(.+)#{Site::HTML_REGEX}/).captures.first - end - - if path.match(/(.+)index?/) - path = path.match(/(.+)index?/).captures.first - end uri = Addressable::URI.parse $config['screenshot_urls'].sample api_user, api_password = uri.user, uri.password @@ -54,37 +44,30 @@ class ScreenshotWorker ) begin - base_image_tmpfile_path = "/tmp/#{SecureRandom.uuid}.jpg" + base_image_tmpfile_path = "/tmp/#{SecureRandom.uuid}.png" File.write base_image_tmpfile_path, HTTP.basic_auth(user: api_user, pass: api_password).get(uri).to_s - image = Rszr::Image.load base_image_tmpfile_path user_screenshots_path = File.join SCREENSHOTS_PATH, Site.sharding_dir(username), username screenshot_path = File.join user_screenshots_path, File.dirname(path_for_screenshot) - FileUtils.mkdir_p screenshot_path unless Dir.exist?(screenshot_path) Site::SCREENSHOT_RESOLUTIONS.each do |res| width, height = res.split('x').collect {|r| r.to_i} + full_screenshot_path = File.join(user_screenshots_path, "#{path_for_screenshot}.#{res}.webp") + + opts = {quality: 90, resize_w: width, resize_h: height} + if width == height - new_img = image.resize(width, height, crop: :n) - else - new_img = image.resize width, height + opts.merge! crop_x: 160, crop_y: 0, crop_w: 960, crop_h: 960 end - full_screenshot_path = File.join(user_screenshots_path, "#{path_for_screenshot}.#{res}.jpg") - tmpfile_path = "/tmp/#{SecureRandom.uuid}.jpg" - - begin - new_img.save tmpfile_path, quality: 92 - $image_optim.optimize_image! tmpfile_path - File.open(full_screenshot_path, 'wb') {|file| file.write File.read(tmpfile_path)} - ensure - FileUtils.rm tmpfile_path - end + WebP.encode base_image_tmpfile_path, full_screenshot_path, opts end true + rescue => e + raise e ensure FileUtils.rm base_image_tmpfile_path end diff --git a/workers/thumbnail_worker.rb b/workers/thumbnail_worker.rb index 1aa311a1..1436a4c9 100644 --- a/workers/thumbnail_worker.rb +++ b/workers/thumbnail_worker.rb @@ -1,8 +1,6 @@ -require 'rmagick' - class ThumbnailWorker THUMBNAILS_PATH = Site::THUMBNAILS_ROOT - MAXIMUM_IMAGE_SIZE = 2_000_000 # 2MB + MAXIMUM_IMAGE_SIZE = 10_000_000 # 10MB include Sidekiq::Worker sidekiq_options queue: :thumbnails, retry: 3, backtrace: true @@ -14,43 +12,33 @@ class ThumbnailWorker site_file_path = site.files_path(path) return unless File.exist?(site_file_path) - # Large images jam up ImageMagick and eat a ton of memory, so we skip for now. + # Large images can eat a ton of memory, so we skip for now. return if File.size(site_file_path) > MAXIMUM_IMAGE_SIZE - img_list = Magick::ImageList.new - - begin - img_list.from_blob File.read(site_file_path) - rescue Errno::ENOENT => e # Not found, skip - return - rescue Magick::ImageMagickError => e - GC.start full_mark: true, immediate_sweep: true - puts "thumbnail fail: #{site_file_path} #{e.inspect}" - return - end - - img = img_list.first - user_thumbnails_path = site.base_thumbnails_path FileUtils.mkdir_p user_thumbnails_path FileUtils.mkdir_p File.join(user_thumbnails_path, File.dirname(path)) Site::THUMBNAIL_RESOLUTIONS.each do |res| - resimg = img.resize_to_fit(*res.split('x').collect {|r| r.to_i}) + width, height = res.split('x').collect {|r| r.to_i} format = File.extname(path).gsub('.', '') + full_thumbnail_path = File.join(user_thumbnails_path, "#{path}.#{res}.webp") - save_ext = format.match(Site::LOSSY_IMAGE_REGEX) ? 'jpg' : 'png' + image = Rszr::Image.load site_file_path + if image.width > image.height + image.resize! width, :auto + else + image.resize! :auto, height + end - full_thumbnail_path = File.join(user_thumbnails_path, "#{path}.#{res}.#{save_ext}") + begin + tmpfile = "/tmp/#{SecureRandom.uuid}.png" + image.save(tmpfile) - resimg.write(full_thumbnail_path) { |i| - i.quality = 75 - } - resimg.destroy! - #$image_optim.optimize_image! full_thumbnail_path + WebP.encode tmpfile, full_thumbnail_path, quality: 70 + ensure + FileUtils.rm tmpfile + end end - - img.destroy! - GC.start full_mark: true, immediate_sweep: true end end