Gradient Part 1 - Color rework by Dherse · Pull Request #2171 · typst/typst

Dherse · 2023-09-17T14:26:06Z

Tracking issue: #2282

This PR does some fundamental changes to the way colors work, as well as introducing several breaking changes. Following is the list of changes in no particular order:

Colors are now all with 64-bit precision: this avoids some f64 -> f32 -> f64 precision issues (when converting from a color to an array) and allows for maximum precision when converting between color spaces
Added the following color spaces: HSV, HSL, Oklab, and linear-RGB (non gamma-corrected RGB)
Added the following PostScript functions in PDF export: Oklab to linRGB, HSV to sRGB, HSL to sRGB
Added the following ICC profiles in PDF export: sRGB and Grayscale (this should fix colors looking different based on the reader), they both fallback to DeviceRGB and DeviceGray with the correct D65 white point
New dependency on palette, a pure-rust crate that provides tons of color conversion utilities
Added the following color constructs: oklab, linear-rgb, hsl, and hsv
Added the following method to color: components which replaces the old to-rgba, to-cmyk, and to-luma. This is a breaking change. These return an array of the color components in the target color space (i.e luma, cmyk, etc.).
Color space conversion can now be done using the constructors i.e they take color components or a color: rgb(hsv(10deg, 10%, 40%)) converts an HSV color into an RGB color.
lighten, darken are now based on the perceived lightness value and handled directly inside of palette.
Added the following method to color: rotate which allows the rotation of the hue by a given angle
Mix now allows more color spaces for mixing of colors and keeps the color in the mixing space: this change avoids chained inaccuracies when converting from colors back and forth. This is a breaking change.
PDF export now encodes colors in their target color space, using ICC profiles and/or PostScript function for correct color conversions (writing the PostScript code was... difficult)
The default colors Black, White, and Gray are now based on a LumaColor instead of being RGB, this should save a small amount of PDF file size, which may be quite large over an entire document
Added relevant tests for all methods and for changes
Changed PDF export to only include the necessary color spaces instead of always containing them all, slight file size optimization
Added dedicated SVG color export for the following spaces: sRGB, linear-RGB, oklab, and HSL since those are natively supported by the SVG spec. The other color space fallback to a hex sRGB value.
Added saturate and desaturate methods on colors, they increase/decrease the saturation based on a percent value.

Color precision

It was quite hard to find a good balance for color precision, in the end, the solution that worked best was f64 colors simply because typst's floats are also f64s and it solves some issues when comparing numbers. However, in general, I would highly recommend against comparing color values purely based on the numerical values as colors are converted lossily

ICC profiles

The ICC profiles are tiny (especially once delfated) accounting for at most ~500 bytes

PostScript function

Have been tested to be correct (although automated testing using GhostScript might be desirable), are written with comments. The comments, line breaks, etc. are stripped before being deflated for size optimization

Gradients

This is the first PR towards gradient, and part of a series of 5 PRs (Colors, Layout & Export, Shape gradients, Text gradients, Fine control of gradients) that will allow gradients to be added to typst.

Printing

On my printer (a cheapo Brother), all color spaces print just fine.

Dherse · 2023-09-17T14:33:18Z

As an example of color precision issues, here is the falling test at the time of writing:

It works just fine on my machine which is a x64 AMD CPU, why it fails I do not know. I will edit tests to work at a purposefully lower precision

freundTech · 2023-09-17T14:36:39Z

Do you think it would make sense to put all the color constructors into a new module color instead of having them as top level functions?

There's quite a few of them (oklab, rgb, linear-rgb, cmyk, hsl, and hsv)

Dherse · 2023-09-17T14:39:25Z

I actually agree with you that the most important ones should be at the top level (imo that would be rgb, oklab, cmyk, and luma) relegating "second-class" constructors (so linear-rgb, hsv, and hsl) inside of the module. The reasoning is that they are terrible color spaces to use directly, and are more intended to be used inside of the gradients (some gradients work really well using just hue rotation).

Dherse · 2023-09-17T15:56:34Z

Before merging: there is a mistake in the HSL PostScript code, some ranges are mixed up, I'll fix it later today

Dherse · 2023-09-17T17:31:25Z

Okay, I have fixed all the PostScript code and it should be pretty much ready for review!

istudyatuni · 2023-09-17T18:25:07Z

Suggestion: instead of having many as- functions, add to color components() method (or similar)

Dherse · 2023-09-17T19:39:22Z

Suggestion: instead of having many as- functions, add to color components() method (or similar)

Initially I wanted to call them rgba etc. but perhaps simply components would work well

Dherse · 2023-09-17T22:03:17Z

Suggestion: instead of having many as- functions, add to color components() method (or similar)

@istudyatuni I ended up implementing this change as I thought it was a much better implementation. To obtain the same effect as the as, the color conversion is now explicit luma(40).to-rgba().components() and also allows the user to select whether they care about the alpha channel or not!

reknih · 2023-09-18T12:53:56Z

lighten and darken should be perceptual for all color spaces.

Dherse · 2023-09-18T12:57:07Z

lighten and darken should be perceptual for all color spaces.

What I meant by that is that I didn't change it for RGB, etc. and I am not sure whether those implementation were perceptual *

Dherse · 2023-09-18T14:38:35Z

Following some discussion on Discord, I removed the to-<color-space>() methods, instead allowing the constructors to perform that conversion like rgb(hsl(10deg, 20%, 30%)) to convert from HSL to RGB.

laurmaedje

Wow! Looks really good overall, I just have some minor nits + a hope (that will probably soon be shattered) that generics could drastically reduce the amount of code in color.rs: The whole file has quite a lot of methods for conversion / modification and dispatch to subcolors. However, palette has a quite powerful trait-based interface from the looks of it. Could we build on that to prevent this n^2 code size (n = number of color types). E.g. by bounding the ColorExt trait on Desaturate + FromColor<HsvColor> + ...?

crates/typst-library/src/lib.rs

crates/typst/Cargo.toml

crates/typst/src/export/pdf/color.rs

crates/typst/src/geom/color.rs

Co-authored-by: Laurenz <laurmaedje@gmail.com>

Dherse · 2023-09-18T21:34:10Z

@laurmaedje in tests, I added a special test-repr method to avoid having to write test(repr(a), repr(b)) all the time for colors, it makes tests a lot more understandable.

Dherse · 2023-09-18T22:16:02Z

Regarding improving things using trait, I have tried A L-O-T on an uncommitted version, and the best I could achieve was a 100-ish line reduction. The main reason why I am not using the palette structs directly is because they're not Hash nor Eq. And these are required for Value. The biggest gains are just by replacing all of the length match self in the main Color struct with a macro to delegate which does help quite a bit.

crates/typst/src/export/pdf/color.rs

crates/typst/src/geom/color.rs

tests/src/tests.rs

crates/typst/src/geom/color.rs

laurmaedje · 2023-09-19T08:52:43Z

Also, now that I see them, the top level icc and post-script folders don't seem right in typst. I would be okay with one of two options:

Move both in typst/assets (I know I said PS code isn't an assets, but whatever)
Move them into src/export/pdf/{icc|postscript}
I think postscript without a hyphen is okay too.

laurmaedje · 2023-09-19T13:48:49Z

Thank you! I'm really happy with the result. :)

Dherse · 2023-09-19T13:54:22Z

Thank you! I'm really happy with the result. :)

@laurmaedje I am really glad this big chunk of gradients is merged 🎉

Dherse added 4 commits September 17, 2023 15:03

Working colors

e3cf88b

Fixed precision

82486ce

fmt, clippy

4488858

cargo fmt

16bb649

Fixed test precision

61f532a

Dherse added 3 commits September 17, 2023 16:40

Fixed issue in color encoding for PDF

b2ec52e

cargo fmt

11d68d9

Cleanup of PDF color conversion

5f55891

Dherse added 4 commits September 17, 2023 18:00

Fixed oklab, HSV and HSL fallbacks

2d26939

Removed debug println

d28fd3f

Fixed HSL & HSV ordering

8cf706a

Avoid division by zero

e42f317

Added dedicated SVG color encoding

fa07b98

Removed as- methods, replaced with components

45b11be

Dherse added 4 commits September 18, 2023 00:11

Added comments

cf98af9

Replaced STD dependency with libm for WASM compat

f87ca9d

Fixed spelling in comment

b396cda

Added saturate and desaturate

fe6d08b

Moved color conversion methods to constructors

9d96840

laurmaedje reviewed Sep 18, 2023

View reviewed changes

Dherse and others added 5 commits September 18, 2023 10:47

Update crates/typst/Cargo.toml

0c8dd99

Co-authored-by: Laurenz <laurmaedje@gmail.com>

Fixed most code review items

113a17b

Fixed all outstanding issues

f8b2e4a

cargo fmt & clippy

6670f53

finished clippy

72646a3

Fixed ICC d65 bug

c54376b

laurmaedje reviewed Sep 19, 2023

View reviewed changes

Dherse and others added 5 commits September 19, 2023 11:31

Fixed code review items

b1c9b13

Replaced with unscanny

0829deb

Cleanup color.rs

dcdf972

Fixed hsv/hsl repr

e223f07

Tidy up a bit

1cf7102

laurmaedje merged commit 163c2e1 into typst:main Sep 19, 2023

Zheoni mentioned this pull request Sep 28, 2023

Wrong colours in PDF export in some viewers #2257

Closed

1 task

This was referenced Sep 29, 2023

Gradient Part 1a - Color fixes #2262

Merged

Gradient Part 2 - Linear gradients #2279

Merged

🌈 Gradient - Tracking issue #2282

Open

Dherse deleted the colors branch October 10, 2023 12:49

Dherse mentioned this pull request Nov 14, 2023

Fix luma → cmyk conversion #2661

Merged

Uh oh!

Conversation

Dherse commented Sep 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Color precision

ICC profiles

PostScript function

Gradients

Printing

Uh oh!

Dherse commented Sep 17, 2023

Uh oh!

freundTech commented Sep 17, 2023

Uh oh!

Dherse commented Sep 17, 2023

Uh oh!

Dherse commented Sep 17, 2023

Uh oh!

Dherse commented Sep 17, 2023

Uh oh!

istudyatuni commented Sep 17, 2023

Uh oh!

Dherse commented Sep 17, 2023

Uh oh!

Dherse commented Sep 17, 2023

Uh oh!

reknih commented Sep 18, 2023

Uh oh!

Dherse commented Sep 18, 2023

Uh oh!

Dherse commented Sep 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laurmaedje left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Dherse commented Sep 18, 2023

Uh oh!

Dherse commented Sep 18, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

laurmaedje commented Sep 19, 2023

Uh oh!

laurmaedje commented Sep 19, 2023

Uh oh!

Dherse commented Sep 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Dherse commented Sep 17, 2023 •

edited

Loading

Dherse commented Sep 18, 2023 •

edited

Loading

laurmaedje left a comment •

edited

Loading