Skip to content

Conversation

@ze0987
Copy link
Contributor

@ze0987 ze0987 commented Mar 25, 2025

This should noticeably speed up previews and thumbnail generation when using magick. Also, the size of the thumbnail files should be smaller, especially when images have a landscape orientation.

Some numbers:

AVIF

Before:

Benchmark 1: taskset -c 19 magick -density 200 IMG_001.avif -flatten -resize 600x900^ -quality 75 -auto-orient JPG:IMG_001_th
  Time (mean ± σ):      4.148 s ±  0.002 s    [User: 3.953 s, System: 0.156 s]
  Range (min … max):    4.144 s …  4.151 s    10 runs

After:

Benchmark 1: taskset -c 19 magick IMG_001.avif -auto-orient -strip -sample 600x -flatten -quality 75 JPG:IMG_001_th
  Time (mean ± σ):     844.7 ms ±   2.0 ms    [User: 740.8 ms, System: 93.8 ms]
  Range (min … max):   841.8 ms … 848.3 ms    10 runs

HEIF

Before:

Benchmark 1: taskset -c 19 magick -density 200 IMG_001.heic -flatten -resize 600x900^ -quality 75 -auto-orient JPG:IMG_001_th
  Time (mean ± σ):      5.810 s ±  0.003 s    [User: 5.582 s, System: 0.174 s]
  Range (min … max):    5.806 s …  5.812 s    10 runs

After:

Benchmark 1: taskset -c 19 magick IMG_001.heic -auto-orient -strip -sample 600x -flatten -quality 75 JPG:IMG_001_th
  Time (mean ± σ):      2.518 s ±  0.002 s    [User: 2.380 s, System: 0.113 s]
  Range (min … max):    2.514 s …  2.520 s    10 runs

JPEG XL

Before:

Benchmark 1: taskset -c 19 magick -density 200 IMG_001.jxl -flatten -resize 600x900^ -quality 75 -auto-orient JPG:IMG_001_th
  Time (mean ± σ):      4.993 s ±  0.013 s    [User: 4.828 s, System: 0.119 s]
  Range (min … max):    4.956 s …  5.002 s    10 runs

After:

Benchmark 1: taskset -c 19 magick IMG_001.jxl -auto-orient -strip -sample 600x -flatten -quality 75 JPG:IMG_001_th
  Time (mean ± σ):      1.704 s ±  0.005 s    [User: 1.627 s, System: 0.060 s]
  Range (min … max):    1.691 s …  1.711 s    10 runs

⚠️ Breaking changes

New svg plugin as the previewer, preloader, and spotter for SVG files

Unlike typical image formats, SVG is a vector format.

This PR separates it from the magick and moves it into a new svg plugin, so that SVG-specific parameters do not affect magick's performance, and magick-specific parameters do not affect SVG's performance, thus improving the performance of both.

- { mime = "image/svg+xml", run = "magick" },
+ { mime = "image/svg+xml", run = "svg" },

This also makes it possible in the future to migrate the SVG backend from magick to other more specialized tools (such as resvg) without affecting the magick plugin – magick renders certain SVGs incorrectly and has suboptimal performance.

Disable the -flatten parameter for magick by default

The -flatten parameter is used to support multi-layer images, but it has a significant impact on performance, and multi-layer images are not common across all image formats.

A new --flatten has been introduced to enable it manually:

- { name = "*.xcf", run = "magick" }
+ { name = "*.xcf", run = "magick --flatten" }

@sxyazi
Copy link
Owner

sxyazi commented Mar 25, 2025

That's incredible!

Can you explain to me the reason for each parameter change?

@ze0987
Copy link
Contributor Author

ze0987 commented Mar 25, 2025

The order of some options is crucial, for example, -flatten at the beginning is slower than at the end (when working on a scaled-down image). -density unnecessarily increases the size of the thumbnail. We want -auto-orient before deleting metadata with -strip. -sample is much faster than -resize.

Edit. I was wrong about the -density. It is actually overridden by -resize or -sample , so it's completely useless.

@sxyazi
Copy link
Owner

sxyazi commented Mar 25, 2025

Moving -flatten to the end will break multi-level images.

You can download test.zip from #1684 (comment), and then try:

magick test.xcf -auto-orient -strip -sample 500x -flatten -quality 90 JPG:test.jpg

and

magick test.xcf -auto-orient -strip -flatten -sample 500x -quality 90 JPG:test.jpg

@ze0987
Copy link
Contributor Author

ze0987 commented Mar 25, 2025

I actually checked this during testing. I didn't notice that one layer was missing. Sorry about that.

-flatten before scaling is quite expensive. Can we use it conditionally? For example, for selected mime types or with option in yazi.toml disabled by default?

Benchmark 1: taskset -c 19 magick IMG_001.jxl -auto-orient -strip -sample 600x -quality 75 -flatten JPG:IMG_001_th
  Time (mean ± σ):      1.705 s ±  0.002 s    [User: 1.629 s, System: 0.060 s]
  Range (min … max):    1.702 s …  1.708 s    10 runs
Benchmark 1: taskset -c 19 magick IMG_001.jxl -auto-orient -strip -flatten -sample 600x -quality 75 JPG:IMG_001_th
  Time (mean ± σ):      2.881 s ±  0.003 s    [User: 2.762 s, System: 0.094 s]
  Range (min … max):    2.876 s …  2.886 s    10 runs

@sxyazi
Copy link
Owner

sxyazi commented Mar 25, 2025

Maybe add a --flatten parameter to enable it manually when the user needs it, such as:

{ name = "*.xcf", run = "magick --flatten" }

Then receive it with job.args.flatten in the preload() method.

So that we don't have to incur ongoing performance loss for all image formats, since images with multi-layer didn't seem very common

@ze0987
Copy link
Contributor Author

ze0987 commented Mar 25, 2025

Awesome. Unfortunately, I will need assistance with this, or some time to figure out how to implement it, since learning Lua is still only in my plans.

By the way, I noticed that rust previewer does not create thumbnails on hover when the preloader is disabled. Is this an intended behavior?

@ze0987 ze0987 marked this pull request as draft March 25, 2025 22:20
@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

I will need assistance with this

No worries, I'll take care of it

I noticed that rust previewer

What's rust previewer? You mean magick previewer or what?

@sxyazi sxyazi changed the title perf: refactor magick arguments perf: faster image preview with optimized magick arguments Mar 26, 2025
@ze0987
Copy link
Contributor Author

ze0987 commented Mar 26, 2025

No worries, I'll take care of it

Awesome. Thank you!

What's rust previewer? You mean magick previewer or what?

image if I'm not mistaken (?) It handles jpeg, webp, among others. With image preloaders disabled, please clear the cache, hover over any jpg/png/webp file, check the cache for the thumbnail.

prepend_preloaders = [
    { mime = "image/*", run = "noop" },
]

@ze0987
Copy link
Contributor Author

ze0987 commented Mar 26, 2025

I was wrong again. -density is not totally useless at all because it is negated by -sample. It can actually "break" things. Source svg: https://pl.wikipedia.org/wiki/Plik:NewTux.svg

w/o -density:
wodensity

with -density:
density

Edit. -density called after the file do not break testing file, but still, it's useless.

@sxyazi sxyazi changed the title perf: faster image preview with optimized magick arguments perf!: faster image preview with optimized magick arguments Mar 26, 2025
@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

image if I'm not mistaken (?) It handles jpeg, webp, among others. With image preloaders disabled, please clear the cache, hover over any jpg/png/webp file, check the cache for the thumbnail.

Yes, this is expected, image previewer uses the builtin image decoder, so there is no need for extra format conversion like that in magick, which would require creating an intermediate file. If the user has not enabled caching, the preview is displayed directly from the source file.

I was wrong again. -density is not totally useless at all because it is negated by -sample. It can actually "break" things. Source svg:

Should be fixed now in the latest commit – changed back to -resize from -sample for SVG.

@sxyazi sxyazi marked this pull request as ready for review March 26, 2025 13:05
Copy link
Owner

@sxyazi sxyazi left a comment

Choose a reason for hiding this comment

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

Thank you!

@sxyazi sxyazi merged commit ad09fb8 into sxyazi:main Mar 26, 2025
6 checks passed
@ze0987
Copy link
Contributor Author

ze0987 commented Mar 26, 2025

Correct me if I'm wrong, but -resize also negates -density. Anyway, this is probably a better solution for the svg previewer. It requires testing, though. The PNG could be considered for the output.

	local cmd = require("magick").with_env():args {
		"-background",
                "none",
                "-size", string.format("%dx%d>", rt.preview.max_width, rt.preview.max_height),
                tostring(job.file.url),
		"-quality", rt.preview.image_quality,
		string.format("JPG:%s", cache),
	}
Benchmark 1: taskset -c 19 magick -density 200 car.svg -resize "600x900>" -quality 75 JPG:thumbnail
  Time (mean ± σ):      5.997 s ±  0.075 s    [User: 4.657 s, System: 1.280 s]
  Range (min … max):    5.925 s …  6.131 s    10 runs

Output:
th_1

Benchmark 1: taskset -c 19 magick -background none -size "600x900>" car.svg -quality 75 JPG:thumbnail
  Time (mean ± σ):     973.9 ms ±   2.3 ms    [User: 653.8 ms, System: 308.9 ms]
  Range (min … max):   969.7 ms … 975.9 ms    10 runs

Output:
th2

Benchmark 1: taskset -c 19 magick -background none -size "600x900>" car.svg -quality 75 png:thumbnail
  Time (mean ± σ):      1.353 s ±  0.005 s    [User: 1.042 s, System: 0.297 s]
  Range (min … max):    1.346 s …  1.361 s    10 runs

Output:
th_3

Edit. Source file: https://dev.w3.org/SVG/tools/svgweb/samples/svg-files/car.svg

@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

In my tests, not adding -density causes the resulting image to be blurry.

Original SVG:

SVG_example_markup_grid

With -density:

magick -density 200 SVG_example_markup_grid.svg -strip -resize 1000x1000^ -quality 90 density.jpg

density

Without -density:

magick SVG_example_markup_grid.svg -strip -resize 1000x1000^ -quality 90 no-density.jpg

no-density

With -size

magick -size '1000x1000>' SVG_example_markup_grid.svg -strip -quality 90 size.jpg

size

@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

Oops, I passed two different -quality arguments - will re-test it using the same -quality value.

@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

Edited #2533 (comment) with the same -quality 90, but the result didn't change much - without -density, the image still appears blurry.

@ze0987
Copy link
Contributor Author

ze0987 commented Mar 26, 2025

Because you are up-scaling the bitmap with ^. Please consider -size with > before the svg file. Its much faster anyway.

@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

Because you are up-scaling the bitmap with ^

That's intentional, SVG is a vector format, which means it doesn't have a fixed width or height, and its initial canvas size can be very small, which makes it less ideal for previewing.

I'll test -size

@JustForFun88
Copy link

@sxyazi Maybe it would be better to switch to resvg for rendering SVGs? It’s similar to the image crate, but for the SVG format. It can render many more SVG images correctly than magick. Typst uses it as the default.

Also, rendering with resvg would be significantly faster.
If you're interested, I could prepare a PR sometime in the coming weeks (maybe a month or a bit more — just a bit busy with work right now 😅).

@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

I'll test -size

No luck, -size resizes the canvas not the actual vector. Updated #2533 (comment)

@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

Maybe it would be better to switch to resvg for rendering SVGs?

I haven't looked into resvg yet, not sure about the impact on binary size and build speed. It seems to have binary packages and perhaps can be invoked as a process like magick, but not sure if mainstream distributions have it, especially on Windows and Android.

@ze0987
Copy link
Contributor Author

ze0987 commented Mar 26, 2025

That's intentional,

I understand. I thought we desired the original size when the source is smaller than the thumbnail. In this case just the -size "%dx%d^" before job.file.url should do.

No luck, -size resizes the canvas not the actual vector. Updated #2533 (comment)

Strange, here it looks like this. magick -size '1000x1000>' image.svg -strip -quality 90 size.jpg

size

magick --version
Version: ImageMagick 7.1.1-46 Q16-HDRI x86_64 22747 https://imagemagick.org
Copyright: (C) 1999 ImageMagick Studio LLC
License: https://imagemagick.org/script/license.php
Features: Cipher DPC HDRI Modules OpenCL OpenMP(4.5) 
Delegates (built-in): bzlib cairo djvu fftw fontconfig freetype heic jbig jng jp2 jpeg jxl lcms lqr ltdl lzma openexr pangocairo png raqm raw rsvg tiff uhdr webp wmf x xml zip zlib zstd
Compiler: gcc (14.2)

Edit. Looks like we can drop ^ and > flags for -size. It makes no difference here.

@JustForFun88
Copy link

Maybe it would be better to switch to resvg for rendering SVGs?

I haven't looked into resvg yet, not sure about the impact on binary size and build speed. It seems to have binary packages and perhaps can be invoked as a process like magick, but not sure if mainstream distributions have it, especially on Windows and Android.

It is cross platform but not sure that they have some widget pakage.

In any case, I will try to prepare the PR and then we will see the binary size impact

@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

I thought we desired the original size when the source is smaller than the thumbnail.

Yes, for raster graphics formats we will respect their original size, but the vector format is an exception because its dimensions are the design dimensions rather than the used dimensions.

Do you think it would be more reasonable not to scale up — why? Perhaps we can discuss changing this behavior.

Strange, here it looks like this. magick -size '1000x1000>' image.svg -strip -quality 90 size.jpg

I revalidated that the parameters were correct and tested it, but the result is the same.

I also tried using just -size 1000x1000 without >, but the result is unchanged. Maybe it's an issue with magick using a different SVG backend.

❯ magick -version
Version: ImageMagick 7.1.1-46 Q16-HDRI aarch64 22747 https://imagemagick.org
Copyright: (C) 1999 ImageMagick Studio LLC
License: https://imagemagick.org/script/license.php
Features: Cipher DPC HDRI Modules OpenMP(5.0)
Delegates (built-in): bzlib fontconfig freetype gslib heic jng jp2 jpeg jxl lcms lqr ltdl lzma openexr png ps raw tiff webp xml zlib zstd
Compiler: clang (15.0.0)

@ze0987
Copy link
Contributor Author

ze0987 commented Mar 26, 2025

Perhaps we can discuss changing this behavior.

I don't have a strong opinion on the subject. Up-scaling probably makes more sense, especially when previewing very small files, for example 16x16 icons. Anyway, looks like ^ > flags are only for scaling. With -size we do not have much choice anyway if we care about the performance.

Maybe librsvg version makes the difference? Here, imagemagick was most likely compiled with v2:2.59.91.

@sxyazi
Copy link
Owner

sxyazi commented Mar 26, 2025

I installed ImageMagick with Homebrew, and seems it doesn't depend on librsvg at all. My system doesn't have librsvg, and I checked Homebrew's shared library directory but couldn't find it either.

@ze0987
Copy link
Contributor Author

ze0987 commented Mar 26, 2025

It'a a build dependency, but I'm only speculating.

Perhaps it would be sensible to pass the arguments via config file with safe-ish defaults? And similarly, regarding the scaling method for raster files. -sample is fast, but some may prefer better quality over performance. They may prefer -thumbnail or even -resize. What do you think?

Edit. Looks like --with-rsvg is optional. I'll recompile and report back.

Edit2. Bingo. Without --with-rsvg magick -size '1000x1000>' image.svg -strip -quality 90 size.jpg looks like this:

size

@ze0987 ze0987 deleted the imagemagick branch March 26, 2025 18:51
@ze0987
Copy link
Contributor Author

ze0987 commented Mar 29, 2025

A tip for those who are not satisfied with the quality of -sample and would like to go back to -resize.
-resize is OpenCL accelerated, so if you meet the requirements (homebrew users are out of luck again) you can export the environment variable MAGICK_OCL_DEVICE=true.

MAGICK_OCL_DEVICE=false

AVIF

Benchmark 1: taskset -c 19 magick IMG_001.avif -strip -resize "600x900>" -quality 95 JPG:IMG_001_th
  Time (mean ± σ):      2.063 s ±  0.003 s    [User: 1.944 s, System: 0.098 s]
  Range (min … max):    2.057 s …  2.067 s    10 runs

HEIF

Benchmark 1: taskset -c 19 magick IMG_001.heic -strip -resize "600x900>" -quality 95 JPG:IMG_001_th
  Time (mean ± σ):      3.738 s ±  0.004 s    [User: 3.586 s, System: 0.118 s]
  Range (min … max):    3.731 s …  3.743 s    10 runs

JPEG XL

Benchmark 1: taskset -c 19 magick IMG_001.jxl -strip -resize "600x900>" -quality 95 JPG:IMG_001_th
  Time (mean ± σ):      2.926 s ±  0.019 s    [User: 2.820 s, System: 0.073 s]
  Range (min … max):    2.885 s …  2.946 s    10 runs

MAGICK_OCL_DEVICE=true @ Intel UHD 770

AVIF

Benchmark 1: taskset -c 19 magick IMG_001.avif -strip -resize "600x900>" -quality 95 JPG:IMG_001_th
  Time (mean ± σ):     995.3 ms ±   6.0 ms    [User: 795.2 ms, System: 183.5 ms]
  Range (min … max):   988.7 ms … 1004.1 ms    10 runs

HEIF

Benchmark 1: taskset -c 19 magick IMG_001.heic -strip -resize "600x900>" -quality 95 JPG:IMG_001_th
  Time (mean ± σ):      2.681 s ±  0.009 s    [User: 2.430 s, System: 0.211 s]
  Range (min … max):    2.667 s …  2.698 s    10 runs

JPEG XL

Benchmark 1: taskset -c 19 magick IMG_001.jxl -strip -resize "600x900>" -quality 95 JPG:IMG_001_th
  Time (mean ± σ):      1.842 s ±  0.017 s    [User: 1.659 s, System: 0.156 s]
  Range (min … max):    1.813 s …  1.868 s    10 runs

@sxyazi
Copy link
Owner

sxyazi commented Apr 1, 2025

Perhaps it would be sensible to pass the arguments via config file with safe-ish defaults? And similarly, regarding the scaling method for raster files. -sample is fast, but some may prefer better quality over performance. They may prefer -thumbnail or even -resize. What do you think?

Yeah, if someone needs them, we can add it as a previewer parameter, but of course, that would need to be justified — like obvious visual differences between -sample and the rest of two.

-resize is OpenCL accelerated, so if you meet the requirements (homebrew users are out of luck again) you can export the environment variable MAGICK_OCL_DEVICE=true.

Great find! Since the current SVG previewer uses -resize, I think we can add that environment variable to it - would you like to raise a PR for that?

I just tested it, and it seems that for ImageMagick builds that don't support OpenCL acceleration, it silently ignores it, so it doesn't seem to break anything.

@ze0987
Copy link
Contributor Author

ze0987 commented Apr 1, 2025

Here's an example when -sample performs poorly (no AA).

-resize
resize

-sample
sample

Personally, I think this quality is perfectly fine for this job, but there will certainly be people who disagree. An alternative approach: use -resize when preview.image_quality is greater than 95, which would mean that the user expects the highest quality.

I believe Yazi should require rsvg-convert (librsvg), or an alternative, if imagemagick is built without librsvg and render at preview.max_width x preview.max_height (not scale a bitmap). This is probably the only solution that will deliver satisfactory results. And it's fast. The SVG preview is currently kinda broken for everyone.

main + imagemagick bulid against librsvg:

resize_librsvg.mp4

main + imagemagick without rsvg:

resize_nolibrvg.mp4

"-size", string.format("%dx%d", rt.preview.max_width, rt.preview.max_height), tostring(job.file.url), "-strip", "-quality", rt.preview.image_quality, "JPG:" .. tostring(cache), + imagemagick bulid against librsvg

size_librsvg.mp4

@sxyazi
Copy link
Owner

sxyazi commented Apr 1, 2025

use -resize when preview.image_quality is greater than 95

image_quality won't be 95, its available range is 50-90.

I believe Yazi should require rsvg-convert (librsvg), or an alternative

Yeah, I plan to explore other ImageMagick alternatives in the future for SVG rendering

@uncenter uncenter mentioned this pull request Apr 8, 2025
13 tasks
Lemi0002 pushed a commit to Lemi0002/yazi that referenced this pull request Apr 10, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants