COLR in floating point (IVS/resolve only) #725
Conversation
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.
|
Scratch my earlier comment on the offsets, I was confused by the test diffs after rebasing. #722 is now rebased on top of this. |
| impl<'a> ColorStops<'a> { | ||
| /// Returns an iterator yielding resolved color stops with variation deltas | ||
| /// applied. | ||
| pub fn resolve( |
There was a problem hiding this comment.
I think given that I see some differences in the offsets in my results before and after this change, it would be useful to add a few tests for this part, just to be able to verify this one level lower down.
There was a problem hiding this comment.
Fixed the intermediate 2.14 overflow and updated the IVS floating point test to check 2.14 and 16.16 deltas.
|
Since #722 is already rebased on this, would you like me to address your comments in a follow up after that lands or fix them here? Either works for me. |
I'd say let's fix them here first, shouldn't be hard to rebase again. |
There actually is an issue here. Some of the intermediate 2.14 deltas in the test font overflow a signed 16-bit integer so they were wrapping negative. The values are such that your stop offset normalization will correct them. I’m not sure if this is in spec but I have a patch that avoids the overflow. |
- update resolve_paint comment to specify that we're returning values in floating point - make use of UfWord delta type for radial gradient radii - avoid overflow of intermediate deltas for 2.14 values and update floating point IVS test to make sure they work
|
I noticed that my variable clipbox test fails with this (including the latest 2257849 commit). Clip boxes are 16bit FWORD types with the same size of a delta - represented in float, I think the first shifted |
|
I’ll dig into this to make sure but I suspect it is working as intended. We’re no longer rounding/truncating after applying deltas to FWORD values and I think this is desired behavior for other properties (specifically, gradient parameters). If we want to special case clip boxes by rounding (or better, floor/ceil the min/max values), that seems reasonable to me. We can also add a |
* Maybe better API for floating point IVS Make use of the type system to appropriately apply floating point deltas to target values.
|
FYI, rebased #722 on top of this and the JSON colrv1 graph dump tests still pass. I did not observe any differences in values. |
As requested, this is the floating point refactor extracted from #697.
ColrInstancefrom read-fonts to a private module in skrifa and update it to produce results in f32.@drott I believe this captures what you requested as a separate, clean PR.