FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Rewrite the tk C blitting code by anntzer · Pull Request #10680 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Rewrite the tk C blitting code - #10680

Merged
QuLogic merged 1 commit into
matplotlib:masterfrom
anntzer:newtk
Apr 4, 2018
ConversationCommits (1)ChecksFiles changed
Merged

Rewrite the tk C blitting code#10680
QuLogic merged 1 commit into
matplotlib:masterfrom
anntzer:newtk

Conversation

anntzer commented Mar 5, 2018

Copy link
Copy Markdown
Contributor

... to use the standard Python C-API rather than the tk API.

We do not need to support colormodes other than 2 (i.e. RGBA)
(previously there was 0 = grayscale, 1 = RGB) so it was simpler to just
deprecate the previous function and move everything to a new private one
(matplotlib.backends._backend_tk.blit).

Followup to #10486 (comment).

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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

anntzer added the GUI: tk label Mar 5, 2018
anntzer added this to the v3.0 milestone Mar 5, 2018
QuLogic reviewed Mar 5, 2018
View reviewed changes
Comment thread src/_tkagg.cpp Outdated
int height, width;
unsigned char *data_ptr;
int x1, x2, y1, y2;
if (!PyArg_ParseTuple(args, "O(iin)(iiii)",

QuLogic Mar 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Add a function name here?

Comment thread src/_tkagg.cpp Outdated
}
PyObject *py_photo_name;
char const *photo_name;
Tk_PhotoHandle photo;

QuLogic Mar 5, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

and these too?
(edit: oops, should go below the next)

Comment thread src/_tkagg.cpp Outdated
&x1, &x2, &y1, &y2)) {
goto exit;
}
PyObject *py_tk, *py_interpaddr;

QuLogic Mar 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Probably should all have defaults?

anntzer commented Mar 5, 2018

Copy link
Copy Markdown
Contributor Author

all comments handled

QuLogic reviewed Mar 5, 2018
View reviewed changes
Comment thread src/_tkagg.cpp Outdated
block.height = y2 - y1;
block.pitch = 4 * width;
block.pixelSize = 4;
block.offset[0] = 0;

QuLogic Mar 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you use this to avoid the buf.take copy?

anntzer Mar 6, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

done

anntzer mentioned this pull request Apr 1, 2018
Merged
6 tasks

tacaswell commented Apr 1, 2018

Copy link
Copy Markdown
Member

@mattip Can you verify that this still works with pypy?

tacaswell approved these changes Apr 1, 2018
View reviewed changes

tacaswell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Modulo confirmation that this still works on pypy

anntzer commented Apr 1, 2018

Copy link
Copy Markdown
Contributor Author

Moved some more work to Python's side, avoiding any need for refcounting at all in the C extension.

mattip commented Apr 1, 2018

Copy link
Copy Markdown
Contributor

works fine on PyPy just like before the PR (with the nightly builds, not the latest release)

Thanks for pinging me

QuLogic approved these changes Apr 2, 2018
View reviewed changes
Comment thread src/_tkagg.cpp Outdated
return Py_None;
}

static PyObject *mpl_tk_blit(PyObject *self, PyObject *args) {

QuLogic Apr 2, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Opening function braces go on a new line.

anntzer Apr 2, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

yup, fixed

tacaswell commented Apr 2, 2018

Copy link
Copy Markdown
Member

@mattip 2.2.2 does not work on pypy?

Rewrite the tk C blitting code
691c7e6 
... to use the standard Python C-API rather than the tk API.

We do not need to support colormodes other than 2 (i.e. RGBA)
(previously there was 0 = grayscale, 1 = RGB) so it was simpler to just
deprecate the previous function and move everything to a new private one
(matplotlib.backends._backend_tk.blit).
QuLogic merged commit b828756 into matplotlib:master Apr 4, 2018
anntzer deleted the newtk branch April 4, 2018 04:30
anntzer mentioned this pull request Jun 20, 2019
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tacaswell tacaswell approved these changes
QuLogic QuLogic approved these changes

Assignees

No one assigned

Labels

GUI: tk

Projects

None yet

Milestone

v3.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL