Skip to content

cairo: speed up the cairo append_path() slow path #13040

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

Closed
wants to merge 1 commit into from

Conversation

lazka
Copy link
Contributor

@lazka lazka commented Dec 22, 2018

PR Summary

Using the Gtk3Cairo backend with wire3d_animation_sgskip.py:

Before: 9.15 fps
After: 10.93 fps

Converting the code to int makes the comparisons faster.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Using the Gtk3Cairo backend with wire3d_animation_sgskip.py:

Before: 9.15 fps
After: 10.93 fps

Converting the code to int makes the comparisons faster.
@lazka
Copy link
Contributor Author

lazka commented Dec 22, 2018

This in combination with #13039 makes the slow path faster than the fast one and the fast one could be removed.

lazka added a commit to lazka/matplotlib that referenced this pull request Dec 23, 2018
With the performance improvements in matplotlib#13040 and matplotlib#13039 the old slow path
is now faster than the previously fast one. And it also works with
pycairo.
@timhoffm
Copy link
Member

timhoffm commented Dec 23, 2018

This will be affected by the type changes proposed in #13039 (comment). I suggest to get #13039 in first and then revisit how this code here should handle types.

@tacaswell tacaswell added this to the v3.1 milestone Dec 24, 2018
@tacaswell
Copy link
Member

#13039 is in which made the change the other way (making everything int8) do we still need this?

@lazka
Copy link
Contributor Author

lazka commented Dec 25, 2018

Yeah, this is no longer needed.

@lazka lazka closed this Dec 25, 2018
@QuLogic QuLogic removed this from the v3.1 milestone Dec 25, 2018
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.

4 participants