Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle sRGB encoding/decoding correctly #54

Open
leroycep opened this issue Nov 4, 2024 · 3 comments
Open

Handle sRGB encoding/decoding correctly #54

leroycep opened this issue Nov 4, 2024 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@leroycep
Copy link

leroycep commented Nov 4, 2024

Hi there,

I've been writing my own software compositing library (targeting real time rendering for wayland applications). Along the way, I discovered that I was handling sRGB encoding/decoding incorrectly. I did a cursory glance at your code, and it seems like eyou are making the same mistake? You can checkout the color module I've created, and I highly recommend the article What Every Coder Should Know About Gamma.

@vancluever
Copy link
Owner

@leroycep thanks for the feedback! BTW it doesn't look like the link to your module made it in correctly to the issue.

WRT to handling encoding correctly per se, do you have any 3rd party prior art with regards to what other 2D libraries or compositing libraries handle? Just looking at this I'm guessing that this is where the issue lies (particularly in downsampling or compositing operations), but at this point I'm more concerned with conformance versus correctness. Like, for example, if pixman is handling this in a more correct way gamma-wise, that's a dead ringer we need to update the operations, otherwise maybe it's up to the consumer to make sure they understand what to do to get gamma correctness.

PS: Our compositing operations right now are based off of the operations in the SVG compositing specification. That document doesn't necessarily mention sRGB, but chapter 14 of the SVG 1.1 second edition specification does, mind you.

@leroycep
Copy link
Author

leroycep commented Nov 4, 2024

Oops, updated my first comment to fix the link.

This may just a problem with documentation. In src/pixel.zig you have the following:

    pub fn fromClamped(r: f64, g: f64, b: f64) RGB {
        return .{
            .r = @intFromFloat(255 * math.clamp(r, 0, 1)),
            .g = @intFromFloat(255 * math.clamp(g, 0, 1)),
            .b = @intFromFloat(255 * math.clamp(b, 0, 1)),
        };
    }

This is converting f32 linear values into u8 linear values; which isn't a problem per say, but it must be kept in mind that most formats (PNG, the format that monitors use) don't use linear u8 values, they use sRGB encoded values.

Here is the function I implemented for encoding linear floating point numbers into sRGB encoded floating point numbers:

    /// Converts a color component from a linear 0..1 space to a compressed 8-bit encoding.
    ///
    /// > [!warn] Alpha is not a color component! It is generally linear even in 8-bit encodings.
    pub fn linearToSRGBFloat(comptime F: type, component_linear: F) F {
        if (component_linear <= 0.0031308) {
            // lower end of the sRGB encoding is linear
            return component_linear * 12.92;
        } else if (component_linear < 1.0) {
            // higher end of value range is exponential
            return std.math.pow(F, 1.055 * component_linear, 1.0 / 2.4) - 0.055;
        } else {
            return 1.0;
        }
    }

(uhh, just noticed that the comment is wrong. I extracted the floating point version from another function that implemented both the encoding and turning it into an integer).

@vancluever
Copy link
Owner

@leroycep thanks, did some research on this really quick and here's what I came up with:

  • Cairo doesn't seem to do sRGB, or else it's not obvious - in cairo-color.c you can see the basic conversion, this is the ultimate result of the cairo_set_source_rgba calls et al.

    • Additionally, image surfaces are set up as PIXMAN_a8r8g8b8 (link); and translating back from the pixman sRGB formats are invalid (link).
  • This doesn't necessarily mean that I want to just support linear RGB, especially given that we do offer the rudimentary PNG export functionality and if sRGB is expected there, that obviously makes sense. I took a quick look and it looks like tiny-skia supports sRGB too, which they seem to have pulled in through Skia itself, so there you go; it should be noted however that linear is the default in tiny-skia.

Given all this I'm slotting this in after we're done line-dash and other v0.5.0 milestone issues as I'm currently focusing on some library API issues and then line-dash, as that's the last missing piece of basic drawing functionality remaining. Hopefully with how we've structured things this should not be too hard to implement.

Thanks again for the heads up!

@vancluever vancluever added the enhancement New feature or request label Nov 5, 2024
@vancluever vancluever added this to the v0.6.0 milestone Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants