Skip to content

[TEM-13161] не оптимизируем оригинальные изображения если они меньше 4096 пикселей#104

Merged
ngoral merged 21 commits intoinsales:masterfrom
akachalova:master
Dec 18, 2025
Merged

Conversation

@akachalova
Copy link
Copy Markdown

No description provided.

Comment on lines +72 to +73
else
FileUtils.cp(src, dst)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Если сделать в стиле guard condition, то будет более понятно и diff короче

anastasiya.kachalova added 4 commits December 10, 2025 15:34
crop_geometry = cropping(dst, ratio, scale)
else
scale_geometry = dst.to_s
needs_scaling = (width > dst.width) || (height > dst.height)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я бы тут вынесла в отдельный метод

    def needs_scaling?(dst)
      # TODO: проверять другие трансформации
      case dst.to_s
      when /\d+x\d+>/
        dst.width < width || dst.height < height
      when /\d+x\d+</
        dst.width > width || dst.height > height
      else
        true
      end
    end

всё-таки от того, как строка выглядит, тоже зависит, надо ли ресайз делать, ну и плюс это метод geometry, а не .процессора

else
scale_geometry = dst.to_s
needs_scaling = (width > dst.width) || (height > dst.height)
scale_geometry = dst.to_s if needs_scaling
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А ещё, чтобы не ломать то, что уже и так есть, может, где-то нужно всегда или ещё хз что, я бы в transformation_to добавила аргумент lazy: false и, соответственно, его передавать в true из конструктора класса Thumbnail.
И тогда тут можно сделать что-то типа такого

        scale_geometry = !lazy || needs_scaling?(dst) ? dst.to_s : nil

dst = Tempfile.new(["#{@basename}-thumb-", ext])
dst.binmode

if !needs_scaling? && !crop? && !convert_options? && !@current_geometry.auto_orient
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут как будто получается очень узкий кейс. Оно может и неплохо, но я бы лучше в tranformation_command проверяла бы, что нам надо добавлять, а что нет. Может быть, и пустую строку там возвращать, почему нет

# Defines the geometry of an image.
class Geometry
attr_accessor :height, :width, :modifier
attr_accessor :height, :width, :modifier, :lazy
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я вообще имела в виду к tranformation_to добавить. Так-то геометрия ленивой быть не может, а транформация как будто да...)

@ngoral ngoral merged commit 24dc082 into insales:master Dec 18, 2025
1 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants