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

64bit and fixes #271

Merged
merged 27 commits into from Mar 27, 2018
Merged

64bit and fixes #271

merged 27 commits into from Mar 27, 2018

Conversation

miguelsousa
Copy link
Member

This is the revival of PR #161

* master: (26 commits)
  Resolve merge conflict with main branch.
  [buildCFF2VF] REsolve merge conflict with main branch
  [buildCFF2VF'. Fix bug with builder.buildVarData() which  blocks building a CFF2 VarStore
  [buildCFF2VF] Added work-around for bug in fonttools
  [buildCFF2VF] Fix support for CFF2 fonts with multiple FontDicts
  [buildCFF2VF] Remove debug line
  [buildCFF2VF] Added work-around for bug in fonttools
  [buildCFF2VF] Fix  bug in building from name-keyed CFF masters
  [blendCFF2VF] Assign a value of False rather than 0 to boolean fields
  blendCFF2VF] Fix PEP8 conflicts: line length, white-space.
  [buildCFF2VF] Fix Codacy error for unused operator.
  [buildCFF2VF] Fix flake8 error reports
  [buildCFF2VF] Fix support for CFF2 fonts with multiple FontDicts
  [travis] Replace bare Slack token
  [makeotfexe] Remove code for preventing idDelta overflow in cmap format 4.
  makeotfexe] Fix issue #242, Unnecessary truncation of the Format 4.
  [checkoutlinesufo] Change --version option to print only the tool's version number
  [checkoutlinesufo] Fix __doc__ string. Move tool description to the top.
  Update Codacy badge links
  Correct change made in c55de1b
  ...
A mistake in the format specifier for reading Unicode values from the GOADB file made the Unicode assignment not work at all.

Comparing output between the 32 bit and 64 bit makeotfexe led to finding a very old bug with un-initialized variables when filling mark to base records. Anchor records were not fully zeroed when created, resulting in a failure to match two records that should match. Old fonts are correct, they just sometimes have more anchor records than needed.
@adobe-type-tools adobe-type-tools deleted a comment Mar 14, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 14, 2018
@miguelsousa
Copy link
Member Author

@readroberts why re-indenting whole files when fixing the affected lines would have been simpler and would not introduce a bunch of unnecessary changes?

This PR is messed up too much for my taste.

@readroberts
Copy link
Contributor

@msousa I choose to re-indent the entire file because there are many inconsistencies in indentation and spacing throughout the file. If I am fixing one spot, why not make them all consistent? I note that the only changes in the last commit are whitespace. I don't care that much about the issue - if you really don't want the entire file re-indented, I can revert the change, let me know, and I will fix only the spots that recently had tabs converted to spaces by Xcode.

@miguelsousa
Copy link
Member Author

@readroberts changing just the lines I marked would have been a lot better IMO. Please undo the changes by rewriting the commits instead of reverting.

@adobe-type-tools adobe-type-tools deleted a comment Mar 20, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 20, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 20, 2018
…r a variable. Set build architecture back to 32 bits.
…nvironment.

Add references to <stdint.h>, and make function declaration and definition use the same types.
@adobe-type-tools adobe-type-tools deleted a comment Mar 23, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 23, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 23, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 23, 2018
Fixed operator precedence issue by adding parents to DF_LEVEL in feat.h, as pointed out by Chris Chapman
Fixed several  printf format warnings about signed/unsigned, long/int differences between arguments and specifiers.
@adobe-type-tools adobe-type-tools deleted a comment Mar 23, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 23, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 23, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 27, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 27, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 27, 2018
@adobe-type-tools adobe-type-tools deleted a comment from miguelsousa Mar 27, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 27, 2018
@readroberts
Copy link
Contributor

I have now installed cppCheck - required fixing some stuff with Xcode. I recently installed High Sierra, but I had some old stuff that interfered with Xcode 9. cppCheck shows many warnings about makeotf - over 500, with '--enable=warning --enable=style' - but this commit fixes all the issues with the change from 32 to 64 bit. I propose that this PR should be merged with main branch now, and that the many cppCheck non-consequential style and printf format warnings should be pursued as a separate project.

@adobe-type-tools adobe-type-tools deleted a comment Mar 27, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 27, 2018
@adobe-type-tools adobe-type-tools deleted a comment from readroberts Mar 27, 2018
Copy link
Contributor

@cjchapman cjchapman left a comment

Choose a reason for hiding this comment

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

(changing this back to "request changes" pending discussion with Miguel)

@adobe-type-tools adobe-type-tools deleted a comment Mar 27, 2018
@adobe-type-tools adobe-type-tools deleted a comment Mar 27, 2018
@cjchapman cjchapman merged commit 56a27ca into master Mar 27, 2018
@cjchapman cjchapman deleted the issuu-64bit-andfixes branch March 27, 2018 23:51
@miguelsousa miguelsousa mentioned this pull request Apr 23, 2018
schriftgestalt pushed a commit to schriftgestalt/afdko that referenced this pull request May 18, 2019
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.

None yet

4 participants