RazrFalcon / tiny-skia

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

Color spaces for blending #71

Closed lunabunn closed 1 year ago

lunabunn commented 1 year ago

Thank you for the great lib. I have a question about how blending works in tiny_skia.

Knowing nothing about Skia, my understanding after reading #5 is that Skia blends colors in srgb (!) by default, and tiny-skia inherits this default. If my understanding here is incorrect, please feel free to correct me and/or close the issue. But assuming it to be correct...

I think this is a bad default that is almost always wrong, but I understand the choice given Skia compatibility. There should, however, still be an option for linear blending, perhaps in Paint/PixmapPaint. Otherwise, it is impossible to get "correct" blending with tiny_skia.

Perhaps it would make sense to always use HQ when blending linearly, as otherwise there may be a degradation in color information.

Am I correct in thinking that this is a feature tiny_skia currently lacks? If so, is it planned? Would you consider merging a PR if someone were to open one?

RazrFalcon commented 1 year ago

tiny-skia works exactly the same as Skia. So yes, sRGB. I know that some people don't like it. But this is the "correct" way for a "classic" 2D library like this.

To support linear color spaces we would have to support color spaces in general, which would take forever to implement.

Basically, this feature is blocked by a lack of a pure Rust color management library. And I don't plan to support just the LinearRGB case. And even after that it would take a while to add a proper color management to tiny-skia.

Ralith commented 1 year ago

There are some excellent pure-rust color management libraries out there, e.g. kolor. It doesn't seem necessary to couple support for linear blending in the color space tiny-skia already uses with support for using different (and more niche) color spaces entirely, though. Correct blending works the same way in any RGB color space.

RazrFalcon commented 1 year ago

I wouldn't call it "excellent". There are no ICC support. No SIMD. Barely any tests. Not good. A good library would be a mix of CGColorSpace + Accelerate on macOS. Or at least a port of Skia's skcms.

Correct blending works the same way in any RGB color space.

Define "correct". tiny-skia uses the same one Skia uses.

jrmuizel commented 1 year ago

qcms supports ICC and has SIMD. It's what we use for color management in Firefox

RazrFalcon commented 1 year ago

@jrmuizel Yes, your library is probably the best so far. I've seen it before. But afaik it's 8bit only?

lunabunn commented 1 year ago

But this is the "correct" way for a "classic" 2D library like this.

Define "correct". tiny-skia uses the same one Skia uses.

There is a well-defined "correct" result for blending A over B, which for (gamma-compressed) sRGB is the result you get by first expanding into linear sRGB and then doing linear blending. What tiny-skia does could've been correct if it assumed all colors were linear and ignored the existence of gamma correction (which is indeed what many "classic" 2D libraries do), but it's not. RgbaU8 basically implies gamma-compressed sRGB + A, and you have confirmed that in the past (#5).

Skia's default behavior of using linear blending for non-linear colors (assuming that's even what it does, as that seems strange) is not correct. tiny-skia's replicating that behavior may be expected but is also not correct.

jrmuizel commented 1 year ago

Yes, it's 8bit only, but I'd accept a PR to add higher bit depth support.

RazrFalcon commented 1 year ago

@lunabunn I understand. But tiny-skia works just like Skia. It's that simple. It's not a bug.

Will it support LinearRGB and other color spaces? Maybe. But definitely not in the next couple of years. Unless someone has a lot of time to figure it all out. It's in a maintenance mode at best.