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

Link against system FreeType #556

Closed
wants to merge 1 commit into from
Closed

Conversation

jwiggins
Copy link
Member

Closes #553

This is just the change to the build script. Another PR will remove the vendored code since that's a very big change.

setup.py Outdated Show resolved Hide resolved
@jwiggins jwiggins force-pushed the deps/system-freetype branch 2 times, most recently from 0029cfd to 10e766d Compare January 29, 2021 10:11
setup.py Outdated
Comment on lines 230 to 237
if sys.platform == 'win32':
# XXX: Note that Windows has used FreeType for many years. This would
# be a change in behavior.
define_macros += [('KIVA_USE_WIN32', None)]
agg_include_dirs += [os.path.join(agg_dir, 'font_win32_tt')]
kiva_agg_sources += [
os.path.join(agg_dir, 'font_win32_tt', 'agg_font_win32_tt.cpp'),
]
Copy link
Member Author

Choose a reason for hiding this comment

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

It's important to check that this doesn't change things too much on Windows. If it does, we'll have to figure out how to link against an external FreeType on Windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using the Win32 font rendering appears to break text rotation, according to @rahulporuri's testing

@jwiggins jwiggins force-pushed the deps/system-freetype branch 2 times, most recently from c697235 to 49df452 Compare January 29, 2021 10:50
@rahulporuri
Copy link
Contributor

Looks like this is no deal on Windows. Apart from CI, I just tried testing this locally and I saw the same issue.

@jwiggins
Copy link
Member Author

The problem was that KIVA_USE_FREETYPE was being forcibly defined in the code. I've fixed that now and it should build

@jwiggins jwiggins force-pushed the deps/system-freetype branch 2 times, most recently from 457419c to d5e058f Compare January 29, 2021 11:32
@jwiggins
Copy link
Member Author

OK. That took a few more tries than expected, but I've now shaken out the code rot in the KIVA_USE_WIN32 code paths. CI should pass for real this time.

@jwiggins
Copy link
Member Author

jwiggins commented Feb 1, 2021

Using the Win32 font renderer in Agg has bug related to affine transformations, so it's best to stick with the FreeType renderer. Unfortunately, that means a lot more complexity when compiling on Windows if we don't want to include a vendored copy of FreeType.

I'm going to close this for now and we can revisit when we're not trying to get a release out the door.

@jwiggins jwiggins closed this Feb 1, 2021
@jwiggins jwiggins deleted the deps/system-freetype branch February 1, 2021 13:29
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.

Remove vendored FreeType from Kiva Agg backend
2 participants