RazrFalcon / tiny-skia

A tiny Skia subset ported to Rust
BSD 3-Clause "New" or "Revised" License
1.11k stars 69 forks source link

Support mipmap downscaling #13

Open RazrFalcon opened 3 years ago

RazrFalcon commented 3 years ago

Skia supports bilinear down-scaling using mipmap. It's not that easy to implement and Skia also caches them.

jermy commented 2 months ago

I'm using resvg for handling a variety of SVG files, and noticed that the quality of scaled down PNGs is definitely more blocky compared to other renderers, and I assume this is the missing/required feature?

I'll probably implement something simple for my usecase, but I'd be interested in the general design you had in mind for this. Is this a specific pipeline component, or an alternative path when in the high quality mode for either bilinear or bicubic? Is full mipmap generation/caching essential, or would just scaling down by some power of ½ quickly in simd then doing bilinear or bicubic on that provide the same results? I'd probably implement the latter, although mostly running via wasm so no simd support anyway.

RazrFalcon commented 2 months ago

No, it's a separate issue of an unknown origin: https://github.com/RazrFalcon/resvg/issues/653 I haven't looked into it yet.

although mostly running via wasm so no simd support anyway

We do support some WASM SIMD.

jermy commented 2 months ago

Ah, thank you. I somehow missed that one. My understanding is that it generates a set of mipmaps from the base (upper) size to the preferred lower size which calculated through SkMipmap::ComputeLevel giving lowerWeight. It runs SkMipmap::Build at some point to generate and cache the levels, then does the bilinear/bicubic sampling from there (via sample_level(&lower); in shaders/SkImageShader.cpp. Assuming it's working with RGBA8888, all the downsampling should be speedy integer arithmetic.

We do support some WASM SIMD.

Ah, apologies. My ideas of what's possible in WASM must be stuck somewhat in the past! I'll see what compiling with -Ctarget-feature=+simd128 gives.

jermy commented 1 week ago

For what it's worth, I finally pushed an implementation of mipmaps I did a couple of months ago on this branch . It's definitely not got all the features of the Skia implementation like sensible caching, but seems to generate almost identical renders in my testing.

RazrFalcon commented 1 week ago

Looks good to me. I'm interested in merging it.

Does it fixes the downscaling issue in resvg? Or is it just an optimization? Have you run any benchmarks? It appears to affect affect both bilinear and bicubic scalers.

How close the code to the Skia version? I understand that there are no caching, but are there any other changes? I'm curious how hard it would be to maintain it in the future.

jermy commented 1 week ago

It should address the downscaling quality in resvg, yes. I haven't benchmarked it, and sadly haven't added any tests. In this patch it will be invoked if you are downscaling less than half the picture size with either bilinear or bicubic scalers, using the next largest size as a source.

Skia does something a bit more interesting when SkMipmapMode == kLinear (source) that does a bilinear scale between the next larger and smaller mipmaps. I think this could be slightly higher quality than bilinear using the next largest mipmaps, but probably not that much different from bicubic. I'd have thought it will be a certain amount more work to implement having two sources of different sizes in the SIMD pipeline?

Apart from that sampling change and per-source caching, I think the main other thing Skia does differently is all the varied GPU-assisted generation of mipmaps. And obviously selection of mipmapmode as an independent setting from filtering/sampling so it can be enabled and disabled as desired.

Here are some output of running resvg with or without this patch on those files mentioned in RazrFalcon/resvg#653, where:

The smileys (source) have been zoomed up from 49x49 by 400% using feh, so I guess there's a small chance that softens edges. The logos (source) have been scaled to 100x100 then zoomed up 200%

Imlib2 Chromium Nearest Before After
smiley-0 smiley-1 smiley-2 smiley-3 smiley-4
instalogo-0 instalogo-1 instalogo-2 instalogo-3 instalogo-4

I was mostly looking for the "Chromium" and "After" to be consistent, and I don't think they're far off.

I don't really know what the future/maintenance would be like - I'm not much of a rust programmer, and somebody who is one could probably find a better way to handle a mipmap cache being against the source pixmap, in particular getting the right lifetimes on that when it's passed into the Blitter. I also switched to always using the mipmaps source with an index of 0 instead of a pixmap_src to avoid duplicating/copying/attempting-to-reference-with-a-suitable-lifetime that data, but I appreciate that could be confusing that it seems to be using a mipmap source when you're not scaling down.

It's probably a bit messy the way that it modifies the pipeline transform to scale the content back up again, but it was easier for me so I could keep that logic in one place in Pattern.push_stages(). I'm also sure the downsampling could be rewritten as an integer SIMD pipeline by someone more brave than I!

RazrFalcon commented 1 week ago

Thanks! You code looks fine to me. By maintenance I was mostly curious about how different it to Skia. The Rust code itself is fine.

Since you have already dived into it, do you have any idea why simply doing bilinear/bicubic doesn't work? My understanding was that mipmaps are just an optimization, that's why I have skipped them. But it appears like they are mandatory. Very strange. Again, I'm not a Skia expert either. Maybe Skia requires mipmaps because bilinear/bicubic are using a pretty small window?

As for mipmaps cache, I'm sure you can guess that doing a thread-safe cache would be a total pain in Rust. Even in Skia/C++ it's pretty convoluted with unique per-image ID generation and stuff. So even if this change slows down rendering it still worth it, since it's technically a bug fix.

jermy commented 1 week ago

Oh, right - that's relatively simple. If you do a bilinear scale - that is, take 4 corners of a square and interpolate them to a new pixel - then as soon as you scale to less than 50% of the original you are missing pixels from the source image.

At exactly 50% scale you create 1 pixel from 4 source pixels. Any less than that and you are effectively creating 1 pixel from 9 potential source pixels, but only sampling 4 of them; hence the loss of quality. By the time you get to 25%, you are creating 1 pixel from 16 and ignoring the values in 12 of those! Bicubic will fare a bit better because it takes 4x4 samples to produce one pixel, but below 25% will suffer the same fate.

There are some good notes about resampling by Jason Summers which I see is referenced a couple of times in the Skia source. In particular "Reducing" on the Image Resampling page that notes some of the issues.

The "correct" way to do downsampling is to create a rectangular region of the source image that maps to a destination pixel, then find an average pixel value by weighting the source pixels covered by the box by how much the box "overlaps" that pixel. There's an optimisation that Jason mentions of precalculating a weight lookup table, but either way it does have to deal with the literal edge case of how to weight pixels near the edge of the image.

Or there's mipmaps, traditionally used for quick rendering of distant textures in 3D engines, where you resize the image down in a series of really predictable/easy half-scales and use one of those as your source instead. As long as no less than 50% scale is being done from a mipmap source image, all of the pixels from your source will be appropriately used in the output. The other neat thing for libraries like Skia is that GPUs can typically generate mipmaps very quickly for 3D textures so the CPU doesn't have to do any work. Even in the CPU it's all integer addition and bitshifts so should be pretty fast.

RazrFalcon commented 1 week ago

Thanks! I really appreciate your explanation. I'm still learning 2D graphics basics.

Back to the Rust version, afair, Skia uses mipmaps only during bilinear scaling, so bicubic should work out of the box, but the resvg bug comes from bicubic, since resvg never uses bilinear. Or did Skia uses mipmaps for bicubic as well? I wrote this code 4 years ago...

Also, are you generating all scales at once? Shouldn't we generate only the one we need? Since we do not cache them anyway. Or do I misunderstood the code? I assume we have to generated the required mipmap right before a pipeline execution starts.

jermy commented 1 week ago

I've spent far too long looking through Skia code now, and not completely sure if it makes any sense. Yes, SkSamplingOptions() does only allow you to set either using bilinear via SkFilterMode::kLinear with an optional SkMipmapMode or a bicubic by providing a cubic filter with appropriate parameters like SkCubicResampler{1/3.0f, 1/3.0f} (which is Mitchell, or a pre-defined SkCubicResampler::Mitchell() or SkCubicResampler::CatmullRom()).

This has changed from previous code - you used to set a quality and it might switch from bicubic to bilinear. Or at least I thought it could. I can't find that now.

Since there is now no option to use mipmaps with bicubic it looks predictably rubbish when downsampling - on this Skia fiddle only the 3rd linear + mipmapped render looks any good and the 4th bicubic looks no better than nearest sampled one on the left. I guess when you use Skia for rendering SVGs you have to know which bits you are downsampling and which are upsampling and select different options? I gather they've been keen to move away from low/medium/high quality, but IMHO this seems like it just makes things tougher for a user who now has to care a bit more about whether they are upscaling or downscaling.

Probably mipmap+bicubic isn't going to get you anything better than mipmap+bilinear, and my patch could almost certainly set quality = FilterQuality::Bilinear; in the push_stages() code when it decides to use a mipmap, so as to avoid doing a more expensive bicubic sample. But I don't know what makes sense for tiny-skia, since I guess it depends whether you want to have control of this externally or not? It's probably not that logical that requesting bicubic could use that for scaling up or it will automatically switch to use mipmaps+bilinear if scaling down, but perhaps resvg knows enough about that that it could specify the right options in render_raster()?

Otherwise, to answer your other question:

Only the number of scales required are produced - compute_required_levels returns the number of mipmaps required, which could be zero. The call to self.mipmaps.build() actually creates the required number of levels that haven't already been generated and is called just before run() is called for the required pipeline.

RazrFalcon commented 1 week ago

Thanks again! My Skia knowledge stops at 2020, when I was writing tiny-skia. I have no idea how it have changed since.

From my perspective, I think tiny-skia should handle all those scaling shenanigans for the user, unlike with Skia. So if tiny-skia decides that bilinear+mipmap is better than bicubic - it should do it. And we can update documentation to reflect it. An average user of the library should not care about such edge cases.

Basically, PixmapPaint::quality becomes "a preferred filtering method".