[skrifa] floating point COLR #697

dfrg · 2023-11-09T03:42:57Z

Internal interface for color outline loading in full floating point.

This copies ColrInstance & co from read-fonts to skrifa and updates it to process all values/deltas in floating point. Additionally, adds methods to ItemVariationStore to compute scalars/deltas in floating point. It's f32 all the way down.

This doesn't remove the code in read-fonts and also contains a module for printing the paint graph so it's not ready to merge.

Printing the graph for a glyph:

use skrifa::prelude::*;
use skrifa::raw::TableProvider;

let font_data = std::fs::read("/path/to/colrv1font.ttf").unwrap();
let font = FontRef::new(&font_data).unwrap();
let colr = font.colr().unwrap();
let coords = [NormalizedCoord::from_f32(-0.6021); 64];
skrifa::scale::print_color_glyph(&colr, GlyphId::new(94), LocationRef::new(&coords));

For the COLRv1 test font, glyph 94 (radial_horizontal_gradient_extend_mode_pad) with all coords set to -0.6021 loads as follows with skrifa maintaining fractional values from deltas:

freetype:

Glyph 2
    RadialGradient p0: (-202.000000,-102.000000) r0: -502.000000 p1: (98.000000,-102.000000) r1: -402.000000 extend: Pad stops: (-1.204224 3 1.000000), (-0.704224 9 1.000000), (-0.204224 0 1.000000)

skrifa:

Glyph GID_2
    RadialGradient p0: (-202.11182,-102.11182) r0: -502.11182 p1: (97.88818,-102.11182) r1: -402.11182 extend: Pad stops: (-1.2042236 3 1), (-0.70422363 9 1), (-0.20422363 0 1)

Internal interface for color outline loading in full floating point.

drott · 2023-11-14T18:09:57Z

Thanks for this - the fact that FreeType returns rounded values is the bug in FreeType at the moment that @tursunova is in the process of fixing - so it's a bit hard to tell how war the fixed computation would be off. I am a bit concerned about the code duplication - if it's COLR related, could we perhaps shift read_fonts to do this in floats right away? I don't think we have a huge legacy there from FreeType (rather, we have bugs that we need to fix, like https://bugs.chromium.org/p/chromium/issues/detail?id=1473020

dfrg · 2023-11-14T18:24:37Z

I am also curious to see how the issue will be resolved in FreeType and how big the diffs might be.

re duplication: if we go this route, my intent was to just remove this functionality from read-fonts. I think your new ColorPainter code does a better job of providing a nice semantic model of a paint graph. For people wanting low level access, that would still be available from our codegen'd types in read-fonts.

- remove fixed point ColrInstance code from read-fonts - remove color paint debug printing module from skrifa

drott · 2023-11-15T13:48:22Z

skrifa/src/scale/colr/instance.rs

@@ -0,0 +1,653 @@
+//! COLR table instance.


What about putting this in skrifa/src/colr/instance.rs? Or skrifa/src/color/instance.rs - My thinking is: We're not really scaling here, mostly applying variations. And then ColorPainter can go in the same directory when it is ready.

This seems reasonable but the details are a bit tricky. I’m thinking forward to adding bounding box computation which will require the scaling code. As of now, the “metadata level” types don’t have any dependencies on the scale module and I find that to be a clean separation. I suppose we can maintain that by just taking a closure for computing per glyph bounding boxes. Let me put together a quick proposal for this API.

I just arrived at the same question - when looking at bounding box computation. Taking transforms into account we'd arrive at implementing a full matrix transform library - which we already have in Skia. So I don't think we need to necessarily replicate that now. My approach now is a basic BoundsPainter that does mostly nothing for the fills, just keeps track of a matrix, and retrieves the tight contours of path along the way. The ingredients already exist.

#702 is a proposal for this that tries to fit it into the current design for metadata access. I see your point about the matrix library, but it seems like ColorPainter needs the struct regardless as a parameter for transform callbacks. I believe your code already has the constructors for translate/rotate/skew, so we'd really only need to add mat * mat and mat * point operators to make this work.

drott · 2023-11-15T13:49:30Z

skrifa/src/scale/colr/instance.rs

+}
+
+#[derive(Copy, Clone)]
+enum DeltaTy {


Does Ty mean Type? Is that a convention we're using. Can we spell it out? DeltaType?

Oops, yes. Since type is a reserved word in rust, this is fairly idiomatic and I follow it by habit but I’m happy to spell it out.

drott · 2023-11-15T13:51:40Z

skrifa/src/scale/colr/mod.rs

@@ -0,0 +1,6 @@
+mod instance;


In line with the comment above, move this to src/colr/mod.rs?

drott · 2023-11-15T13:52:27Z

read-fonts/src/tables/variations.rs

+    ///
+    /// This requires a type parameter specifying the type of target value for the
+    /// delta.
+    pub fn compute_delta_f32<T: ItemDeltaTarget>(


Is it possible to add some standalone file level tests for this?

We don't have a reference for correctness here so I added a sanity test to ensure that the floating point results are within a reasonable margin to the delta produced by the FT compatible computation. Specifically, it ensures that either the rounded or truncated floating point delta matches the FT compatible delta. We have to account for both because fractional accumulation can push us to either side of the 0.5 boundary in some cases.

- renamed DeltaTy -> DeltaType - add sanity test for floating point delta computation

Expose a color outline API in top level metadata rather than in the scale module.

- add version() method to ColorOutline - remove maxp lookup and change iterator to use max glyph in COLR table - remove glyph_id from ColorOutline and change iterator to return (GlyphId, ColorOutline) tuple

As requested, this is the floating point refactor extracted from #697. * add methods to IVS to compute scalars/deltas in f32. * move `ColrInstance` from read-fonts to a private module in skrifa and update it to produce results in f32.

drott · 2023-12-11T17:06:24Z

Can we close this, is this superseded by #725?

dfrg · 2023-12-11T17:43:05Z

Yep!

[skrifa] floating point COLR

415f9c4

Internal interface for color outline loading in full floating point.

dfrg added 2 commits November 14, 2023 14:11

cleanup to make ready for review

ff6677f

- remove fixed point ColrInstance code from read-fonts - remove color paint debug printing module from skrifa

fix doc comment links

484b00d

dfrg marked this pull request as ready for review November 14, 2023 19:17

drott reviewed Nov 15, 2023

View reviewed changes

dfrg added 2 commits November 15, 2023 12:16

partial review feedback

8ca7838

- renamed DeltaTy -> DeltaType - add sanity test for floating point delta computation

Merge branch 'main' into colr-float

9767283

dfrg mentioned this pull request Nov 15, 2023

[skrifa] public color API #702

Merged

dfrg added 3 commits November 28, 2023 10:41

[skrifa] public color API

9671285

Expose a color outline API in top level metadata rather than in the scale module.

review feedback

fdf5a13

- add version() method to ColorOutline - remove maxp lookup and change iterator to use max glyph in COLR table - remove glyph_id from ColorOutline and change iterator to return (GlyphId, ColorOutline) tuple

add test for iterator and version

2c9308a

drott mentioned this pull request Nov 28, 2023

[skrifa] Callback-based COLRv1 implementation #722

Merged

8 tasks

dfrg mentioned this pull request Nov 29, 2023

COLR in floating point (IVS/resolve only) #725

Merged

dfrg closed this Dec 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[skrifa] floating point COLR #697

[skrifa] floating point COLR #697

dfrg commented Nov 9, 2023

drott commented Nov 14, 2023

dfrg commented Nov 14, 2023

drott Nov 15, 2023

dfrg Nov 15, 2023

drott Nov 15, 2023

dfrg Nov 15, 2023

drott Nov 15, 2023

dfrg Nov 15, 2023

dfrg Nov 15, 2023

drott Nov 15, 2023

drott Nov 15, 2023

dfrg Nov 15, 2023

dfrg Nov 15, 2023

drott commented Dec 11, 2023 •

edited

Loading

dfrg commented Dec 11, 2023

[skrifa] floating point COLR #697

[skrifa] floating point COLR #697

Conversation

dfrg commented Nov 9, 2023

drott commented Nov 14, 2023

dfrg commented Nov 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drott commented Dec 11, 2023 • edited Loading

dfrg commented Dec 11, 2023

drott commented Dec 11, 2023 •

edited

Loading