Add metrics to transformations filter #238
Conversation
| if self.options.BSB: | ||
| ctx.metrics["bottomMargin"] = self.options.BSB | ||
| if self.options.VerticalOrigin: | ||
| ctx.metrics["verticalOrigin"] = self.options.VerticalOrigin |
There was a problem hiding this comment.
why don't you just use the metrics values defined in self.options? The context is only for stuff that may vary from glyph to another, but this options should be global for all the glyphs processed by this filter instance.
There was a problem hiding this comment.
stuff that may vary from glyph to another
sorry, I meant stuff that may vary from one font to another.
I see, you're mapping the Glyphs.app-specific option names (e.g. LSB, RSB) to defcon Glyph-specific attribute names (e.g. "leftMargin", "rightMargin").
I think this should be moved to start method, which is run only once at filter initialization, since it is not dependent on the font you're currenty running the filter onto.
moyogo
force-pushed
the
transformations-metrics
branch
from
5bc1c7d to
9bbdebb
Compare
| else: | ||
| logger.warning( | ||
| "Cannot add %i to undefined %s in %s", | ||
| value, attr, glyph.name |
There was a problem hiding this comment.
minor suggestion: you don’t need to increment value +=current_value, just setattr(glyph, attr, current_value+value)
| ["c", "d"]) | ||
| def test_verticalOrigin_undefined(self, font, name): | ||
| value = +14 | ||
| with CapturingLogHandler(logger, level="WARNING") as captor: |
There was a problem hiding this comment.
Since we are doing pytest-style tests you could use the caplog fixture
|
We need to change the way filter strings are parsed in glyphLib. |
|
@moyogo so, about https://github.com/googlei18n/ufo2ft/pull/238/files#diff-866da416f2f593dfda48548e538d877aR86 I see you are using leftMargin, righMargin, etc. Now, I think this is a bit problematic, for two reasons, The glyphs we are operating on are parent-less, they are copies of the original without a reference to their parent font. For components, that means they cannot get their own "bounds" by themselves without requiring the glyph set they are within. Besides, in ufoLib2 the glyph objects don't have a parent (they never have, not just when they are duplicated out of a font). If you look at the outlineCompiler, you'll see we don't use leftMargin and rightMargin attributes to build the htmx; but we make the bounding boxes ourselves at the beginning for the whole glyphSet and compute the margins accordingly Maybe in set_context method of Transformations filter, we should compute the bounding boxes the same way that the outlineCompiler does (we could turn that method in a function and move it to the ufo2ft.util module so both outlineCompiler and the Transformations filter can use it) Setting the leftMargin basically means redrawing the glyph's outline but shifted horizontally, plus modifying the advance at the same time (see how defcon implements that). You see, I'm reluctant to add a parent attribute to the ufoLib2 objects. I don't like these recursive cross-references between containers and contained objects. I'd like to keep them as pure data classes without too much built-in intelligence |
|
this PR is stale. Is there still interest in this? |
|
hm, I guess this depends on googlefonts/glyphsLib#366 being fixed first, right @moyogo? |
|
Yes #366 should be fixed first. |
Add LSB, RSB arguments but also TSB, BSB and VerticalOrigin.