Tuesday, November 29, 2022

Going inside Cairo to add color management

Before going further you might want to read the previous blog post. Also, this:

I don't really have prior experience with color management, Cairo internals or the like. I did not even look at the existing patchsets for this. They are fairly old so they might have bitrotted and debugging that is not particularly fun. This is more of a "the fun is in the doing" kind of thing. What follows is just a description of things tried, I don't know if any of it would be feasible for real world use.

Basically this is an experiment. Sometimes experiments fail. That is totally fine.

Main goals

There are two things that I personally care about: creating fully color managed PDFs (in grayscale and CMYK) and making the image backend support images in colorspaces other than sRGB (or, more specifically, "uncalibrated RGB which most of the time is sRGB but sometimes isn't"). The first of these two is simpler as you don't need to actually do any graphics manipulations, just specify and serialize the color data out to the PDF file. Rendering it is the PDF viewer's job. So that's what we are going to focus on.

Color specification

Colors in Cairo are specified with the following struct:

struct _cairo_color {
    double red;
    double green;
    double blue;
    double alpha;

    unsigned short red_short;
    unsigned short green_short;
    unsigned short blue_short;
    unsigned short alpha_short;
};

As you can probably tell it is tied very tightly to the fact that internally everything works with (uncalibrated) RGB, the latter four elements are used for premultiplied alpha computations. It also has a depressingly amusing comment above it:

Fortunately this struct is never exposed to the end users. If it were it could never be changed without breaking backwards compatibility. Somehow we need to change this so that it supports other color models. This struct is used a lot throughout the code base so changing it has the potential to break many things. The minimally invasive change I could come up with was the following:

struct _comac_color {
    comac_colorspace_t colorspace;
    union {
        struct _comac_rgb_color rgb;
        struct _comac_gray_color gray;
        struct _comac_cmyk_color cmyk;
    } c;
};

The definition of rbg color is the original color struct. With this change every usage of this struct inside the code base becomes a compile error which is is exactly what we want. At every point the code is changed so that it first asserts that colorspace is RGB and then accesses the rgb part of the union. In theory this change should be a no-op and unless someone has done memcpy/memset magic, it is also a no-op in practice. After some debugging (surprisingly little, in fact) this change seemed to work just fine.

Color conversion

Ideally you'd want to use LittleCMS but making it a hard dependency seems a bit suspicious. There are also use cases where people would like to use other color management engines and even select between them at run time. So a callback functions it is:

typedef void (*comac_color_convert_cb) (comac_colorspace_t,
                                        const double *,
                                        comac_colorspace_t,
                                        double *,
                                        comac_rendering_intent_t,
                                        void *);

This only converts a single color element. A better version would probably need to take a count so it could do batch operations. I had to put this in the surface struct, since they are standalone objects that can be manipulated without a drawing context.

Generating a CMYK PDF stream is actually fairly simple for solid colors. There are only two color setting operators, one for stroking and one for nonstroking color (there may be others in e.g. gradient batch definitions, I did not do an exhaustive search). That code needs to be changed to convert the color to match the format of the output PDF and then serialize it out.

CMYK PDF output

With these changes creating simple CMYK PDF files becomes fairly straightforward. All you need to do as the end user is to specify color management details on surface creation:

comac_pdf_surface_create2 (
    "cmyktest.pdf",
    COMAC_COLORSPACE_CMYK,
    COMAC_RENDERING_INTENT_RELATIVE_COLORIMETRIC,
    comac_default_color_convert_func,
    NULL,
    595.28,
    841.89);

and then enjoy the CMYK goodness:

What next?

Probably handling images with ICC color profiles.

Saturday, November 26, 2022

Experimenting on how to add CMYK and color management to Cairo

Cairo is an amazing piece of tech that powers a lot of stuff, like all of GTK. Unfortunately it is not without its problems. The biggest one being that it was designed almost 20 years ago with the main use case of dealing with "good old" 8 bit uncalibrated RGB images. There has been a lot of interest in adding native support for things like CMYK documents, linear RGB, color calibration, wide gamuts and all of that good stuff. Sadly it has not come to be.

The reasons are mostly the same as always. The project is sadly understaffed and there does not seem to be a corporate sponsor to really drive the development forward. What makes things extra difficult is that Cairo supports a lot of different platforms like Postscript, Win32, Quartz and SVG. So if someone wants to add new features in Cairo, not only do they need to understand how color math works and how to do C, they would also need to handle all the various backends. That is a rare combination of skills. In any case the patchset needed to make all that happen would be enormous and thus hard to get reviewed and merged.

As an exercise I thought I'd see if I could change the landscape somewhat to make it easier to experiment with the code base. Out of this came this project repo called Comac (for, obviously, COlor MAnaged Cairo). The creation process was fairly straightforward:

  • Take the latest Cairo trunk
  • Delete every backend except image and PDF
  • Rename all files and symbols from cairo-something to comac-something
  • Add minimal code for generating grayscale and CMYK PDFs
  • Create a demo app that creates test PDFs
The output files are very simple, but the whole thing actually works. Both Okular and Acrobat Reader will happily display the documents without errors. Thus people who are interested in color work can now actually look into it without having to understand how Xlib works. Since Comac does not need to follow all of Cairo's backwards compatibility guarantees, experimenters can play a bit more fast & loose.

Want to contribute?

Merge requests welcome, I guess? I have not thought too deeply about governance and all that. If there is a lot of interest, this could be moved to its own top level project rather than living in my Github pages. Just note that the goal is not to fork things and create yet another graphics library. In the best possible case this project is only used to discover a good API and an idea on how it could best be implemented. These changes could then be put into Cairo core.

Monday, November 14, 2022

If you don't tolerate it in new code you should not tolerate it in old code either

Let's assume that you are working on a code base and notice that it has some minor issue. For argument's sake we'll say that it has some self written functionality and that the language's standard library has added identical functionality recently. Let's further assume that that said implementation behaves exactly the same as the self written one. At this point you might decide to clean up the code base, make it use the stdlib implementation and delete the custom code. This seems like a nice cleanup so you then file merge request to get the thing changed.

It might be accepted and merged without problems. On the other hand it might be blocked, either temporarily or permanently. There are several valid reasons for not merging the change. For example:

  1. That part of the code does not have sufficient tests so we don't know if the change is safe.
  2. A release is coming up soon, so we are minimizing all changes that might cause regressions, no matter how minor. The merge can only be done after the release is out.
  3. We need to support old platform X, whose stdlib does not have that functionality.
  4. It's not immediately obvious that the two implementations behave identically (for example, because the old implementation has load-bearing bugs) so a change might introduce bugs.

Getting blocked like this is a bit unfortunate, but these things happen. The important thing, however, is that all these are solid technical reason for not doing the cleanup (or at least not do it immediately). Things get bad when you get blocked by some other reason, such by your reviewer asking "why are you even doing this at all". This is a reasonable question, so let's examine it in detail.

Suppose that instead of submitting a cleanup commit you are instead submitting a piece of completely new functionality. In this MR you have chosen to reimplement a piece of standard library code (for no actual gain, just because you were not aware of its existence). The review comment that you should get is "You are reimplementing stdlib functionality, delete that code here and use the standard library instead". This is a valid review comment and something that should be heeded.

The weird thing here is that this is in a way the exact same change, but it is either not acceptable or absolutely necessary depending on whether parts of the code are already inside your repo or not. This is weird and should not be the case, but human beings are strangely irrational and their "value functions" are highly asymmetric. This can lead to lots of review fighting if one person really wants to fix the issue whereas some other one does not see the value in it. The only real solution is to have a policy on this, specifically that submitting a change that fixes an issue that would be unacceptable in new code is, by itself, a sufficient reason to do the work but not to merge it without technical review. This shifts the discussion from "should this be done at all" to "what are the technical risks and merits of this change", which is the way reviews should be done.

I should emphasize that this does not mean that you should go and immediately spend 100% of your time fixing all these existing issues in your code base. You could, but probably it is not worth it. What this guideline merely says that if you happen to come across an issue like this and if you feel like fixing it then you can do it and reviewers can't block you merely by "I personally don't like this" line of reasoning.

A nastier version

Suppose again you are a person who cares about the quality of your work. That you want to write code that is of sufficiently good quality and actually care about things like usability, code understandability and long term maintainability. For you people there exists a blocker comment that is much worse than the one above. In fact is the single biggest red flag for working conditions I have ever encountered:

But we already have [functionality X] and it works.

This question does have a grain of truth in it, existing code does have value. Sadly it is most commonly used as a convenient way to hardblock the change without needing to actually think about it or having to explain your reasoning.

Several times I've had to make some change into existing code and hours and days of debugging later found that the existing self written code has several bugs that a stdlib implementation definitely would not have. After reporting my findings and suggesting a cleanup the proposal has been knocked out with reasoning above. As a consultant I have to remain calm and respect the client's decision but the answer I have wanted to shout every time is: "No it's not! It's completely broken. I just spent [a large amount of time] fixing it because it was buggy and looking at the code makes me suspect that there are a ton of similar bugs in it. Your entire premise is wrong and thus every conclusion drawn from it is incorrect."

And, to reiterate, even this is not a reason by itself to actually go and change the code. It might get changed, it might not. The point is the prevailing attitude around minor fixups and how it matches your personal desire of getting things done. If you are a "fixing things proactively makes sense" kind of person you are probably not going to be happy in a "let's hide under the bed and pretend everything is fine" kind of organization and vice versa.