interpolate-hcl uses non-standard D65 illuminant

[ChrisLoer]

The RGB -> HCL code in https://github.com/maplibre/maplibre-gl-js/blob/main/src/style-spec/util/color_spaces.ts forked out of d3-color in 2016. It uses the D65 illuminant in all of its intermediate color spaces. In 2018 d3-color was patched to convert from D65 to D50 (see discussion at d3/d3-color#42). The CSS spec recommends using D50 to convert to LAB (which is the step before converting to HCL).

The spirit of the Mapbox/MapLibre Style Spec is generally to follow CSS behavior where possible, so I expect this is the behavior we'd actually want. However, with the current behavior in place, migrating might be a bit trickier -- is it OK to change color behavior for existing maps? Is it OK for the behavior of MapLibre to diverge from Mapbox GL?

Here's an example of how the difference plays out, interpolating between ["#1518e4", "#A96011"] with the same input values to the interpolation -- the visual effect is to have less distinction as you traverse the color space between the two endpoints:

Using D50 (this is generated with Protomaps.js -- notice the wide range of purples)

Using D65 (this is generated with MapLibre GL JS -- notice how the values seem to cluster closer to red or blue)

I'm happy to work on a PR for this if we think it's a change that should go into MapLibre.

cc @HarelM @ibesora

[HarelM]

I think this was just solved in the maplibre style and there's an open PR here: #2376

[ChrisLoer]

Oh, cool! But I'm looking at https://github.com/kajkal/maplibre-gl-style-spec/blob/c34a895e6e3f138a404b1554a37d4ca3ab1a2a58/src/util/color_spaces.ts#L34-L42 and it looks to me like it's still using D65? There are different ways to do it, but I was expecting something like https://github.com/d3/d3-color/blob/958249d3a17aaff499d2a9fc9a0f7b8b8e8a47c8/src/lab.js#L5-L13.

I am not an expert in color spaces at all (in fact I'm color blind, I can barely even see these differences), so it's easy for me to get confused here.

[HarelM]

Cc @kajkal.
I don't have deep knowledge about color interpolation...

[kajkal]

As far as I'm concerned, I think you've hit the perfect moment when it comes to making changes with colors. Recently, I've been working on adding support for CSS Color 4 and, in the process, I've slightly improved the existing code related to color interpolation #94.
Personally, I believe that the use of color spaces other than sRGB in mapbox/maplibre was not a commonly used feature, because I refuse to believe that such interpolation results have not bothered anyone who actually used them in recent years.

When you're done you can add your bundle to the collection to easily illustrate the difference between the versions :)

[wipfli]

Nice demo!

[wipfli]

Did a lot of render tests break when you introduced the new interpolation?

[kajkal]

@wipfli Not too many, 3 in maplibre-style-spec and 4 in maplibre-gl-js (where there is a plan to move these tests to maplibre-style-spec #97). < These are unit and integration tests, I know nothing about visual/render tests :c

[ChrisLoer]

That is a great demo @kajkal ! It took me a while to figure out what's going wrong, but it looks to me like the totally broken interpolation you're seeing is only for style properties that use a color ramp texture (I think that's line-gradient and heatmaps). The reason is that the rgb->hcl->interpolate->back-to-rgb path can leave small negative values for some of the RGB values. This is obviously not correct (what does it even mean?) but I think on other code pathways it basically ends up being a rounding error. On the color ramp pathway, it gets clamped to an 8 bit uint, so it overflows to 255, causing those dramatic discontinuities.

At Felt, we definitely use interpolate-hcl (it's consistent with color interpolation in the rest of our app, and also I think it's just perceptually superior for interpolation). We don't run into this behavior because we don't use line gradients or heatmaps yet. A common case for us would be in a choropleth map.

Great to see this is an area of active development!

[ChrisLoer]

Aha, and I see that you added clamping on the way back to RGB that would prevent this: https://github.com/maplibre/maplibre-style-spec/pull/94/files#diff-1ea8753eaadd1cd8c60e59e66ccde779f029fbc78b12d363865d3b009351b271R91

OK, so the path forward here for me to try to do the d50 modification is probably to open a PR against maplibre-style-spec with the changes, then build a bundle with that pulled in so we can use it for comparison in your tool?

[kajkal]

I'm not the decision maker here but to me your plan sounds great 😅

When you're done you can add your bundle to the collection

Don't treat this part as some kind of requirement. I just thought it would help to visualize the changes and make sure that the results are visually satisfactory.

As for the bundle, you can easily make it locally by changing

maplibre-gl-js/package.json

Line 24 in e50a264

"@maplibre/maplibre-gl-style-spec": "^18.0.1",

to the location of the style repo with the generated dist folder:

"@maplibre/maplibre-gl-style-spec": "file:../maplibre-gl-style-spec",

at least that's how I did it.

[ChrisLoer]

Cool -- I drafted up an initial version of the changes and looking at the tests it looks very roughly right (but I think we probably want to visually benchmark against some internal tool like d3-color).

The change relies on being very trusting with importing new magic numbers, but is basically pretty simple: https://github.com/felt/maplibre-style-spec/pull/1/files

I had trouble getting the package built and incorporated into maplibre-gl the way you suggested. This error in the maplibre-style-spec tests might point to something to do with the typescript config?

  ● @maplibre/maplibre-gl-style-spec npm package › files build

    expect(received).toMatchInlineSnapshot(snapshot)

    Snapshot name: `@maplibre/maplibre-gl-style-spec npm package files build 1`

    - Snapshot  - 1
    + Received  + 0

    @@ -11,9 +11,8 @@
        "gl-style-validate.cjs.map",
        "gl-style-validate.mjs",
        "gl-style-validate.mjs.map",
        "index.cjs",
        "index.cjs.map",
    -   "index.d.ts",
        "index.mjs",
        "index.mjs.map",

Meanwhile on the maplibre-gl side, an npm install pointing to file:../maplibre-style-spec generates a lot of typescript compile errors, e.g.:

TSError: ⨯ Unable to compile TypeScript:
../maplibre-style-spec/dist/index.cjs:3:10 - error TS2304: Cannot find name 'define'.

3   typeof define === 'function' && define.amd ? define(['exports', 'color-string', '@mapbox/unitbezier'], factory) :
...

I haven't opened a PR against maplibre/maplibre-style-spec yet because I need to figure that stuff out. I'm OOO all next week so might not get a chance to wrap this up right away.

[kajkal]

In maplibre-gl-style-spec:
npm run build - to build .jss bundles
npm run generate-style-spec - idk to generate some more types or something
npm run generate-typings - to build index.d.ts bundle (the test you mentioned calls for it)
then your maplibre-gl-style-spec/dist folder should have all the necessary files.

Next in maplibre-gl-js:
npm run build-dist - to build everything possible

Sometimes my changes from maplibre-gl-style-spec did not append correctly to the final bundle maplibre-gl.js (or its easier-to-read equivalent maplibre-gl-dev.js). In this case, manually deleting the node_modules/@maplibre folder before build-dist helped. I was using pnpm so some magic caching was probably to blame.

[kajkal]

@wipfli

Did a lot of render tests break when you introduced the new interpolation?

I checked and not a single render test showed that something had changed with color interpolation :)

[ChrisLoer]

I ended up running the test by just swapping out the file in node_modules, and at first I thought nothing was happening (although I made an unminified build so I could easily verify the new code was in the bundle).

I put in debug log statements to look at the values being generated for the color ramp, and could see they were slightly different -- but I couldn't actually see the difference until I recorded a video going back and forth between the two (this is me recording switching between two tabs with d50 vs d65, it's d50 when "Example set 1" is highlighted). The recording is slightly lossy so it's not perfect, but you can see the difference in "interpolate-hcl" and "interpolate-lab" -- look in the part halfway between blue and white, and you can see with d65 it basically looks whiter all across the spectrum.

d50vsd65.mov

Honestly -- I can see the difference, and I think d50 is more correct, but this looks very subtle to me. I think @kajkal 's earlier fixes were way more important, and I'm not still sure if this is a big deal.

[kajkal]

@ChrisLoer I added your changes from felt/maplibre-style-spec#1 to the color demo bundle collection. I also added the option to compare changes using the maplibre-gl-compare plugin:
https://kajkal.github.io/maplibre-gl-style-spec-color-demo/swipe.html

The changes do indeed seem subtle, however, I think it would be worth switching to D50.
It would be nice to include these changes in v3.0.0 so that potentially breaking changes with colors can be made in a single release.

[ChrisLoer]

Oh wow, nice touch with the swipe interface! When you updated the color unit tests, did you follow a particular strategy (e.g. looking for testing specific colors), or did you just run them again and copy over new values?

[kajkal]

I assume you are asking about tests from color_spaces.test.ts. I was inspired by the color selection from the CSS Color 4 specification tests, but only for the input data, the output data was generated by the existing code.

[ChrisLoer]

OK, I put the PR up at maplibre/maplibre-style-spec#146!

[birkskyum]

Closed by maplibre/maplibre-style-spec#146 (change to style spec)
Closed by #2503 (update style spec in GL JS)