Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compute COLRv1 ClipBoxes to work around a bug in Chrome #847

Closed
anthrotype opened this issue Jan 17, 2023 · 12 comments
Closed

compute COLRv1 ClipBoxes to work around a bug in Chrome #847

anthrotype opened this issue Jan 17, 2023 · 12 comments

Comments

@anthrotype
Copy link
Member

COLRv1 fonts built from .glyphs sources using fontmake do not explicitly define clipBoxes, in fact the ClipList is optional and should be computed on the fly when omitted says the COLRv1 spec. However, Chrome 108 has a bug whereby some gradients are not being rendered if the font doesn't contain some ClipBoxes.

Details are in https://bugs.chromium.org/p/chromium/issues/detail?id=1404678
This first surfaced in fonttools/fonttools#2938
Also related: fonttools/fontbakery#4020

Here's an example of a font that is built that way: https://fonts.google.com/specimen/Blaka+Ink

COLRv1 fonts built using nanoemoji (from .svg inputs, e.g. Noto Color Emoji) won't have this issue because nanoemoji always precomputes explicit clip boxes, never omits them.

I think glyphsLib should also always compute them. They don't take that much space as things gets reused, it will make rendering a bit faster as renderer doesn't have to traverse the graph to compute on the fly, and will also work around this Chrome bug.

Current code lives in https://github.com/googlefonts/glyphsLib/blob/main/Lib/glyphsLib/builder/color_layers.py

@drott
Copy link

drott commented Jan 17, 2023

However, Chrome 108 has a bug whereby some gradients are not being rendered if the font doesn't contain some ClipBoxes.

I would describe the bug differently. The bug is not about ClipBoxes but about reading ColorLines at the end of the table. Placing ClipBoxes there happens to fix the bug because it has the side effect of making the bounds checks in reading the ColorLine pass. The bug itself is not more or less reason to add ClipBoxes.

We previously discussed the difficulty of computing maximum ClipBoxes or suitable variable ClipBoxes with PaintVarRotate and PaintVarTransform.

@anthrotype
Copy link
Member Author

Thanks for the clarification Dominik, I didn't want to go into the details here, just linked to the upstream bug which explains that -- but you're right the bug is not really about clipboxes, and their presence just happens to workaround it.

But yeah, you make a valid point about computing variable glyphs max bounds being hard when there are complex rotation/transforms...

@anthrotype
Copy link
Member Author

... but since the COLRv1 glyphs we generate from .glyphs source only contains a very simpe list of PaintGlyphs without any fancy transformations it is safe and easy to compute bounding boxes for them. I have code locally to do that, just need to polish and add some tests. It only adds a bunch of KB to Blaka Ink's COLR table so I think it's still worth it.

@khaledhosny
Copy link
Collaborator

Last I checked, calculating glyph bounds in glyphsLib was so slow, do your changes affect build time significantly?

@anthrotype
Copy link
Member Author

Last I checked, calculating glyph bounds in glyphsLib was so slow, do your changes affect build time significantly?

no, it's relatively quick (runtime increases 0.1~0.2 seconds for BlakaInk with 600+ color glyphs) since I am not actually using the existing code for GSLayer.bounds, the latter needs bezier derivatives to computes the tightest bounds for the curves, whereas i'm only interested in computing the control points bounds; I am simply iterating over all control points in each layer and taking the min/max, while also taking into account component transforms.

here's the WIP PR: #853

anthrotype added a commit that referenced this issue Jan 24, 2023
@schriftgestalt
Copy link
Collaborator

In Glyphs, the layer has a fastBounds method that returns the bounds of the control points.

@anthrotype
Copy link
Member Author

anthrotype commented Jan 25, 2023

@schriftgestalt thanks, I can move my code to a new GSLayer.fastBounds property (or is it a method call i.e. layer.fastBounds()?).

However, I tried calling it from within the macro window using version 3.1.2 (3151) though I get AttributeError. Is it only available in the beta?

print(Glyphs.font.selectedLayers[0].fastBounds)

Traceback (most recent call last):
  File "<macro panel>", line 1
AttributeError: 'NSKVONotifying_GSLayer' object has no attribute 'fastBounds'

@anthrotype
Copy link
Member Author

in reply to myself, I updated to cutting-edge version and there's a fastBounds() method (not property) on GSLayer returning a rect.

@schriftgestalt
Copy link
Collaborator

ObjectiveC doesn’t really have properties. And the the pyobjc bridge doesn’t help much with them either.
So to have a property in python, I need to manually add that to the wrapper.

And as layer.bounds are a property, I’ll do the same for fastBounds.

@anthrotype
Copy link
Member Author

ok, I'll make it a property (though also I don't mind it being a method).

can you please comment on what the logic is to resolve GSComponent.componentLayer when inside a color layer? #853 (comment)

@anthrotype
Copy link
Member Author

anthrotype commented Jan 30, 2023

i'm going to rework this on top of #854

EDIT: actually, I changed my mind, and implemented the functionality in fonttools

@anthrotype
Copy link
Member Author

Chrome has since fixed the bug in stable version 111 released a couple weeks ago.
And the newly released ufo2ft 2.31.0 will compute clipboxes for us so glyphsLib needs to do nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants