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

Use non-xxxVF paths in t2cstr when flattening and outputting CFF2 #1588

Merged
merged 1 commit into from
Jan 2, 2023

Conversation

skef
Copy link
Collaborator

@skef skef commented Dec 21, 2022

Description

handleBlend() in shared/t2cstr.c "handles" flattening by adjusting values on the stack. Because it doesn't copy the blendValues into the opEntry in the flatten mode, you do wind up with numBlends equaling zero in that entry but you wind up with the default in value rather than the blended value.

Perhaps handleBlend() should also adjust the opEntry->value in the flatten case, but I'm reluctant to do that because it seems like the value may be cross-referenced in other places. And while this would fix our stem calculation problems it appears that the curveto case is more complicated and would need additional work.

Anyway, there's no real point to using the blend when we're flattening.

So instead this PR just avoids using the blends entirely when calling out downstream (moveto, lineto, curveto, stemto) in the flattening case. Instead of calling the xxxxVF variant we just call the normal "flat" one. (It also skips the "preparatory" work in t2decode().) That will work fine with CFF2 -- in fact cffwrite does that optimization when there are no blends anyway.

I was going to add a CFF2 instancing test for this but it turns out we don't (appear to?) have any instancing tests right now. So I filed #1587 instead, as what should go into those tests is more than I want to think about while we're sorting through VF hinting.

Checklist:

  • I have followed the Contribution Guidelines
  • I have added test code and data to prove that my code functions correctly
  • I have verified that new and existing tests pass locally with my changes
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

@skef
Copy link
Collaborator Author

skef commented Dec 21, 2022

Given the nature of the failures I'm going to assume this is the same windows problem we currently have in develop branch rather than something caused by the (very specific) changes in this PR.

Copy link
Collaborator

@josh-hadley josh-hadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and I agree that the Windows CI failure is not related to this code at all and should not block this.

@skef skef merged commit 3d74655 into adobe-type-tools:develop Jan 2, 2023
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 this pull request may close these issues.

2 participants