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

"friendly names" #48

Closed
PeterConstable opened this issue Sep 28, 2020 · 7 comments · Fixed by #49
Closed

"friendly names" #48

PeterConstable opened this issue Sep 28, 2020 · 7 comments · Fixed by #49
Milestone

Comments

@PeterConstable
Copy link
Collaborator

+1 to the friendly names.

The name PaintGlyph for format 6 doesn't clearly indicate what is distinctive about it vs. PaintColrGlyph.

The latter provides a color composition (an entire tree) for re-use. It's functionality has a lot of overlap with Transform (format 4); it's just that ColrGlyph uses a GID while the Transform uses a paint offset, and Transform includes a 2x3 while ColrGlyph doesn't. These are more similar than ColorGlyph is to Glyph.

PaintGlyph provides a geometry and fill, and it's the only way to put a fill into a geometry. But "PaintGlyph" doesn't really indicate that.

PaintColrGlyph reasonably indicates its functionality since it references a composition by the base glyph ID.

But perhaps "PaintGlyph" could be replaced with something like "PaintGlyphWithFill" or PaintOutlineWithFill?

@rsheeter
Copy link
Contributor

PaintBaseGlyph? PaintSimpleGlyph?

@behdad
Copy link
Collaborator

behdad commented Sep 28, 2020

The latter provides a color composition (an entire tree) for re-use. It's functionality has a lot of overlap with Transform (format 4); it's just that ColrGlyph uses a GID while the Transform uses a paint offset,

There's exactly ZERO overlap. Indeed, they are expected to be chained together mooooooost of the time.

The ColrGlyph allows "group" compositing. (Random anecdote: that's also what MERG table is about.)

and Transform includes a 2x3 while ColrGlyph doesn't. These are more similar than ColorGlyph is to Glyph.

So what's the overlap again?!!!!

PaintGlyph provides a geometry and fill, and it's the only way to put a fill into a geometry. But "PaintGlyph" doesn't really indicate that.

PaintFill is fine with me.

@behdad
Copy link
Collaborator

behdad commented Sep 28, 2020

Thinking more, grouping can be done now that we have a full DAG.

So PaintColrGlyph really just allows that: reuse by gid. Is not necessary, but definitely good to have.

@behdad
Copy link
Collaborator

behdad commented Sep 28, 2020

Also specially since we are using Offset16 in many places, not Offset32. So there's a limit to how much reuse can happen through offsets.

@behdad
Copy link
Collaborator

behdad commented Sep 28, 2020

Let me review and see if I can recommend some of the offsets to be elevated to Offset32.

@behdad
Copy link
Collaborator

behdad commented Sep 28, 2020

Let me review and see if I can recommend some of the offsets to be elevated to Offset32.

I suggest upgrading all Offset16's to Offset32. I have changed my position on this since it was designed (last week?)

This is particularly concerning:

// Glyph root
// NOTE: uint8 size saves bytes in most cases and does not
// preclude use of large layer counts via PaintComposite.
typedef ArrayOf<Offset16<Paint>, uint8> LayerV1List;

@rsheeter
Copy link
Contributor

Oh you are right, thank you, will fix.

@davelab6 davelab6 added this to the COLRv1 milestone Oct 1, 2020
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