10000 COMPAT: use tkagg backend on PyPy by mattip · Pull Request #9356 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

COMPAT: use tkagg backend on PyPy #9356

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Jan 12, 2018
Merged

COMPAT: use tkagg backend on PyPy #9356

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b4e2b8a
simple plot works
mattip Oct 10, 2017
68492fb
flesh out bbox handling, fix mpl_PyFile_Dup for PyPy which is like py…
mattip Oct 16, 2017
8e80a20
change interface in PyAggImagePhoto to avoid id()
mattip Nov 3, 2017
512e4d8
search for cffi c-extension module to find c-functions
mattip Nov 5, 2017
4c5dfaa
pep8
mattip Nov 5, 2017
c2a7143
fixes from review
mattip Nov 23, 2017
09576fa
silence warnings, fix build on old MSC compilers
mattip Nov 24, 2017
596345e
Cleanup embedding_in_tk, remove embedding_in_tk2.
anntzer Oct 3, 2017
1712678
Merge branch 'tkagg-anim' into tkagg-cffi
mattip Dec 8, 2017
b40923d
use agg:int8u not uint8_t
mattip Dec 12, 2017
43d9754
Merge branch 'master' into tkagg-cffi
10000 mattip Jan 11, 2018
270f05d
add whatsnew entry
mattip Jan 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions lib/matplotlib/backends/tkagg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,24 @@ def blit(photoimage, aggimage, bbox=None, colormode=1):

if bbox is not None:
bbox_array = bbox.__array__()
# x1, x2, y1, y2
bboxptr = (bbox_array[0, 0], bbox_array[1, 0], bbox_array[0, 1], bbox_array[1, 1])
else:
bbox_array = None
bboxptr = 0
data = np.asarray(aggimage)
dataptr = (data.ctypes.data, data.shape[0], data.shape[1])
try:
tk.call(
"PyAggImagePhoto", photoimage,
id(data), colormode, id(bbox_array))
dataptr, colormode, bboxptr)
except Tk.TclError:
try:
try:
_tkagg.tkinit(tk.interpaddr(), 1)
except AttributeError:
_tkagg.tkinit(id(tk), 0)
tk.call("PyAggImagePhoto", photoimage,
id(data), colormode, id(bbox_array))
except (ImportError, AttributeError, Tk.TclError):
raise
if hasattr(tk, 'interpaddr'):
_tkagg.tkinit(tk.interpaddr(), 1)
else:
# very old python?
_tkagg.tkinit(tk, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

interpaddr was added in 1998(! python/cpython@9d1b7ae) so I cannot actually imagine a case where this is triggered today (but that can go to another PR of course)

tk.call("PyAggImagePhoto", photoimage,
dataptr, colormode, bboxptr)

def test(aggimage):
import time
Expand Down
2 changes: 0 additions & 2 deletions setupext.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1513,13 +1513,11 @@ def runtime_check(self):

def get_extension(self):
sources = [
'src/py_converters.cpp',
'src/_tkagg.cpp'
]

ext = make_extension('matplotlib.backends._tkagg', sources)
self.add_flags(ext)
Numpy().add_flags(ext)
LibAgg().add_flags(ext, add_sources=False)
return ext

Expand Down
110 changes: 53 additions & 57 deletions src/_tkagg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,15 @@
#include <cstdio>
#include <sstream>

#include "py_converters.h"

// Include our own excerpts from the Tcl / Tk headers
#include "_tkmini.h"

#if defined(_MSC_VER)
# define SIZE_T_FORMAT "%Iu"
# define IMG_FORMAT "%Iu %d %d"
#else
# define SIZE_T_FORMAT "%zu"
# define IMG_FORMAT "%zu %d %d"
#endif
# define BBOX_FORMAT "%f %f %f %f"
Copy link
Contributor

Choose a reason for hiding this comment

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

extra space?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in cc7b73e


typedef struct
{
Expand All @@ -44,16 +43,15 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int
{
Tk_PhotoHandle photo;
Tk_PhotoImageBlock block;
PyObject *bufferobj;

// vars for blitting
PyObject *bboxo;

size_t aggl, bboxl;
size_t pdata;
int wdata, hdata;
float x1, x2, y1, y2;
bool has_bbox;
uint8_t *destbuffer;
uint8_t *destbuffer, *buffer;
int destx, desty, destwidth, destheight, deststride;
//unsigned long tmp_ptr;

long mode;
long nval;
Expand All @@ -73,24 +71,14 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int
TCL_APPEND_RESULT(interp, "destination photo must exist", (char *)NULL);
return TCL_ERROR;
}
/* get array (or object that can be converted to array) pointer */
if (sscanf(argv[2], SIZE_T_FORMAT, &aggl) != 1) {
TCL_APPEND_RESULT(interp, "error casting pointer", (char *)NULL);
return TCL_ERROR;
}
bufferobj = (PyObject *)aggl;

numpy::array_view<uint8_t, 3> buffer;
try {
buffer = numpy::array_view<uint8_t, 3>(bufferobj);
} catch (...) {
TCL_APPEND_RESULT(interp, "buffer is of wrong type", (char *)NULL);
PyErr_Clear();
/* get buffer from str which is "ptr height width" */
if (sscanf(argv[2], IMG_FORMAT, &pdata, &hdata, &wdata) != 3) {
TCL_APPEND_RESULT(interp,
"error reading data, expected ptr height width",
(char *)NULL);
return TCL_ERROR;
}
int srcheight = buffer.dim(0);

/* XXX insert aggRenderer type check */
buffer = (uint8_t*)pdata;

/* get array mode (0=mono, 1=rgb, 2=rgba) */
mode = atol(argv[3]);
Expand All @@ -100,39 +88,33 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int
}

/* check for bbox/blitting */
if (sscanf(argv[4], SIZE_T_FORMAT, &bboxl) != 1) {
TCL_APPEND_RESULT(interp, "error casting pointer", (char *)NULL);
return TCL_ERROR;
}
bboxo = (PyObject *)bboxl;

if (bboxo != NULL && bboxo != Py_None) {
agg::rect_d rect;
if (!convert_rect(bboxo, &rect)) {
return TCL_ERROR;
}

if (sscanf(argv[4], BBOX_FORMAT, &x1, &x2, &y1, &y2) == 4) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should probably error out cleanly if an invalid string is passed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yesish, these are basically "private" functions parsing a call from python, but I will add more checking code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in cc7b73e

has_bbox = true;
}
else {
has_bbox = false;
}

destx = (int)rect.x1;
desty = srcheight - (int)rect.y2;
destwidth = (int)(rect.x2 - rect.x1);
destheight = (int)(rect.y2 - rect.y1);
if (has_bbox) {
int srcstride = wdata * 4;
destx = x1;
desty = hdata - y2;
destwidth = x2 - x1;
destheight = y2 - y1;
deststride = 4 * destwidth;

10000 destbuffer = new agg::int8u[deststride * destheight];
destbuffer = new uint8_t[deststride * destheight];
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason for the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

now it no longer uses #include py_converters.h which supplied the agg:int8u

Copy link
Member

Choose a reason for hiding this comment

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

Why change away from the agg types here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously there was a conversion from a PyArrayObject * (in other words ndarray) to a data pointer, width, height, etc via py_converters. Now I pass that information directly from python. I no longer need the "py_converters.cpp" and ""py_converters.h" since there is no use of ndarray in the c++ code. Should I leave the include for this one type definition?

Copy link
Member

Choose a reason for hiding this comment

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

I am concerned about the corner case where unint8_t and agg:int8u are different underlying types. Not sure how likely that actually is, hence leaning towards being conservative about changing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what different types you think these can be.
I would also bet that there must be parts of the code where we assume that this is interconvertible with np.uint8, or that this fits in a single byte... doesn't leave you a lot of options!

if (destbuffer == NULL) {
TCL_APPEND_RESULT(interp, "could not allocate memory", (char *)NULL);
return TCL_ERROR;
}

for (int i = 0; i < destheight; ++i) {
memcpy(destbuffer + (deststride * i),
&buffer(i + desty, destx, 0),
&buffer[(i + desty) * srcstride + (destx * 4)],
deststride);
}
} else {
has_bbox = false;
destbuffer = NULL;
destx = desty = destwidth = destheight = deststride = 0;
}
Expand Down Expand Up @@ -168,10 +150,10 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int
delete[] destbuffer;

} else {
block.width = buffer.dim(1);
block.height = buffer.dim(0);
block.width = wdata;
block.height = hdata;
block.pitch = (int)block.width * nval;
block.pixelPtr = buffer.data();
block.pixelPtr = buffer;

/* Clear current contents */
TK_PHOTO_BLANK(photo);
Expand Down Expand Up @@ -199,7 +181,7 @@ static PyObject *_tkinit(PyObject *self, PyObject *args)
} else {
/* Do it the hard way. This will break if the TkappObject
layout changes */
app = (TkappObject *)PyLong_AsVoidPtr(arg);
app = (TkappObject *)arg;
interp = app->interp;
}

Expand Down Expand Up @@ -338,7 +320,7 @@ int load_tkinter_funcs(void)
* tkinter uses these symbols, and the symbols are therefore visible in the
* tkinter dynamic library (module).
*/
#if PY3K
#if PY_MAJOR_VERSION >= 3
#define TKINTER_PKG "tkinter"
#define TKINTER_MOD "_tkinter"
// From module __file__ attribute to char *string for dlopen.
Expand Down Expand Up @@ -432,13 +414,31 @@ int load_tkinter_funcs(void)
}
tkinter_lib = dlopen(tkinter_libname, RTLD_LAZY);
if (tkinter_lib == NULL) {
PyErr_SetString(PyExc_RuntimeError,
"Cannot dlopen tkinter module file");
goto exit;
/* Perhaps it is a cffi module, like in PyPy? */
pString = PyObject_GetAttrString(pSubmodule, "tklib_cffi");
if (pString == NULL) {
goto fail;
}
pString = PyObject_GetAttrString(pString, "__file__");
if (pString == NULL) {
goto fail;
}
tkinter_libname = fname2char(pString);
if (tkinter_libname == NULL) {
goto fail;
}
tkinter_lib = dlopen(tkinter_libname, RTLD_LAZY);
}
if (tkinter_lib == NULL) {
goto fail;
}
ret = _func_loader(tkinter_lib);
// dlclose probably safe because tkinter has been imported.
dlclose(tkinter_lib);
goto exit;
fail:
PyErr_SetString(PyExc_RuntimeError,
"Cannot dlopen tkinter module file");
exit:
Py_XDECREF(pModule);
Py_XDECREF(pSubmodule);
Expand All @@ -447,7 +447,7 @@ int load_tkinter_funcs(void)
}
#endif // end not Windows

#if PY3K
#if PY_MAJOR_VERSION >= 3
static PyModuleDef _tkagg_module = { PyModuleDef_HEAD_INIT, "_tkagg", "", -1, functions,
NULL, NULL, NULL, NULL };

Expand All @@ -457,15 +457,11 @@ PyMODINIT_FUNC PyInit__tkagg(void)

m = PyModule_Create(&_tkagg_module);

import_array();

return (load_tkinter_funcs() == 0) ? m : NULL;
}
#else
PyMODINIT_FUNC init_tkagg(void)
{
import_array();

Py_InitModule("_tkagg", functions);

load_tkinter_funcs();
Expand Down
2 changes: 1 addition & 1 deletion src/file_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ extern "C" {
/*
* PyFile_* compatibility
*/
#if PY3K
#if defined(PY3K) | defined(PYPY_VERSION)

/*
* Get a FILE* handle to the file represented by the Python object
Expand Down
0