Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
loading TCL / Tk symbols dynamically#6442
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes from1 commit
Commits
Show all changes
14 commits Select commitHold shift + click to select a range
edc80a3 WIP: loading TCL / Tk symbols dynamically
matthew-brett64f01dd RF: refactor C code to Michael D's commments
matthew-brett89535be NF: add Windows support for TCL dynamic loading
matthew-brettfc5d060 RF: make dynamic loading of TCL the one true way
matthew-brett1c85968 RF: disable build-time link against TCL/Tk
matthew-brett830d7a3 RF: find TCL / Tk symbols correctly on Windows
matthew-brett4c5a3ab RF: short-circuit the Tk function tests
matthew-brett94bb457 WIP: Add excerpts from TCL / Tk header
matthew-brett3f5407a RF: check, edit, refactor Tcl / tk mini header
matthew-brett573aeb5 RF: remove Tcl / Tk header / lib discovery
matthew-brett626a1d2 RF: remove build-time check of Tkinter
matthew-brett0c2c5a0 RF: force TkAgg / Agg extensions with messages
matthew-brettde99a76 BF: fix goofy double "installing" message
matthew-brettaa275c6 TST: add test of tkagg backend import
matthew-brettFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
NextNext commit
WIP: loading TCL / Tk symbols dynamically
This is an attempt to load the symbols we need from the Tkinter.tkintermodule at run time, rather than by linking at build time.It is one way of building a manylinux wheel that can build on a basicManylinux docker image, and then run with the TCL / Tk library installedon the installing user's machine. It would also make the situationbetter on OSX, where we have to build against ActiveState TCL forcompatibility with Python.org Python, but we would like to allowrun-time TCL from e.g. homebrew.I have tested this on Debian Jessie Python 2.7 and 3.5, and on OSX 10.9with Python 2.7.Questions:* Would y'all consider carrying something like this approach in the matplotlib source, but not enabled by default, to help building binary wheels?* Do you have any better suggestions about how to do this?* My C fu is weak; is there a way of collecting the typedefs I need from the TCL / Tk headers rather than copying them into the _tkagg.cpp source file (typdefs starting around line 52)?* My fu for Python C extension modules is also weak; did I configure exceptions and handle references correctly?
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
commitedc80a38e76e6cf61a49a4311b0c5c3101835a44
There are no files selected for viewing
159 changes: 141 additions & 18 deletionssrc/_tkagg.cpp
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -44,6 +44,38 @@ typedef struct | ||
| Tcl_Interp *interp; | ||
| } TkappObject; | ||
| #define DYNAMIC_TKINTER | ||
| #ifdef DYNAMIC_TKINTER | ||
| // Load TCL / Tk symbols from tkinter extension module at run-time. | ||
| // Typedefs, global vars for TCL / Tk library functions. | ||
| typedef Tcl_Command (*tcl_cc)(Tcl_Interp *interp, | ||
| const char *cmdName, Tcl_CmdProc *proc, | ||
| ClientData clientData, | ||
| Tcl_CmdDeleteProc *deleteProc); | ||
| static tcl_cc TCL_CREATE_COMMAND; | ||
| typedef void (*tcl_app_res) (Tcl_Interp *interp, ...); | ||
| static tcl_app_res TCL_APPEND_RESULT; | ||
| typedef Tk_Window (*tk_mw) (Tcl_Interp *interp); | ||
| static tk_mw TK_MAIN_WINDOW; | ||
| typedef Tk_PhotoHandle (*tk_fp) (Tcl_Interp *interp, const char *imageName); | ||
| static tk_fp TK_FIND_PHOTO; | ||
| typedef void (*tk_ppb_nc) (Tk_PhotoHandle handle, | ||
| Tk_PhotoImageBlock *blockPtr, int x, int y, | ||
| int width, int height); | ||
| static tk_ppb_nc TK_PHOTO_PUTBLOCK; | ||
| typedef void (*tk_pb) (Tk_PhotoHandle handle); | ||
| static tk_pb TK_PHOTO_BLANK; | ||
| #else | ||
| // Build-time linking against system TCL / Tk functions. | ||
| #define TCL_CREATE_COMMAND Tcl_CreateCommand | ||
| #define TCL_APPEND_RESULT Tcl_AppendResult | ||
| #define TK_MAIN_WINDOW Tk_MainWindow | ||
| #define TK_FIND_PHOTO Tk_FindPhoto | ||
| #define TK_PHOTO_PUTBLOCK Tk_PhotoPutBlock | ||
| #define TK_PHOTO_BLANK Tk_PhotoBlank | ||
| #endif | ||
| static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int argc, char **argv) | ||
| { | ||
| Tk_PhotoHandle photo; | ||
| @@ -61,25 +93,25 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int argc, | ||
| long mode; | ||
| long nval; | ||
| if (TK_MAIN_WINDOW(interp) == NULL) { | ||
| // Will throw a _tkinter.TclError with "this isn't a Tk application" | ||
| return TCL_ERROR; | ||
| } | ||
| if (argc != 5) { | ||
| TCL_APPEND_RESULT(interp, "usage: ", argv[0], " destPhoto srcImage", (char *)NULL); | ||
| return TCL_ERROR; | ||
| } | ||
| /* get Tcl PhotoImage handle */ | ||
| photo =TK_FIND_PHOTO(interp, argv[1]); | ||
| if (photo == NULL) { | ||
| 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; | ||
| @@ -88,7 +120,7 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int argc, | ||
| try { | ||
| buffer = numpy::array_view<uint8_t, 3>(bufferobj); | ||
| } catch (...) { | ||
| TCL_APPEND_RESULT(interp, "buffer is of wrong type", (char *)NULL); | ||
| PyErr_Clear(); | ||
| return TCL_ERROR; | ||
| } | ||
| @@ -99,13 +131,13 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int argc, | ||
| /* get array mode (0=mono, 1=rgb, 2=rgba) */ | ||
| mode = atol(argv[3]); | ||
| if ((mode != 0) && (mode != 1) && (mode != 2)) { | ||
| TCL_APPEND_RESULT(interp, "illegal image mode", (char *)NULL); | ||
| return TCL_ERROR; | ||
| } | ||
| /* 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; | ||
| @@ -126,7 +158,7 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int argc, | ||
| destbuffer = new agg::int8u[deststride * destheight]; | ||
| if (destbuffer == NULL) { | ||
| TCL_APPEND_RESULT(interp, "could not allocate memory", (char *)NULL); | ||
| return TCL_ERROR; | ||
| } | ||
| @@ -167,7 +199,7 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int argc, | ||
| block.pitch = deststride; | ||
| block.pixelPtr = destbuffer; | ||
| TK_PHOTO_PUTBLOCK(photo, &block, destx, desty, destwidth, destheight); | ||
| delete[] destbuffer; | ||
| } else { | ||
| @@ -177,9 +209,9 @@ static int PyAggImagePhoto(ClientData clientdata, Tcl_Interp *interp, int argc, | ||
| block.pixelPtr = buffer.data(); | ||
| /* Clear current contents */ | ||
| TK_PHOTO_BLANK(photo); | ||
| /* Copy opaque block to photo image, and leave the rest to TK */ | ||
| TK_PHOTO_PUTBLOCK(photo, &block, 0, 0, block.width, block.height); | ||
| } | ||
| return TCL_OK; | ||
| @@ -216,11 +248,11 @@ static PyObject *_tkinit(PyObject *self, PyObject *args) | ||
| /* This will bomb if interp is invalid... */ | ||
| TCL_CREATE_COMMAND(interp, | ||
| "PyAggImagePhoto", | ||
| (Tcl_CmdProc *)PyAggImagePhoto, | ||
| (ClientData)0, | ||
| (Tcl_CmdDeleteProc *)NULL); | ||
| Py_INCREF(Py_None); | ||
| return Py_None; | ||
| @@ -232,6 +264,90 @@ static PyMethodDef functions[] = { | ||
| { NULL, NULL } /* sentinel */ | ||
| }; | ||
| #ifdef DYNAMIC_TKINTER | ||
| // Functions to fill global TCL / Tk function pointers from tkinter module. | ||
| #include <dlfcn.h> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This include fails when using Visual C++ for Python 2.7: https://ci.appveyor.com/project/mdboom/matplotlib/build/1.0.1545/job/724qv5hcfngqa2r9#L1033 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Actually more recent versions of Visual C++ also fail, e.g. in the Python 3.5 Windows build on AppVeyor: https://ci.appveyor.com/project/mdboom/matplotlib/build/1.0.1545/job/rk6safsaid63i7ch#L1053 | ||
| #if PY3K | ||
| #define TKINTER_PKG "tkinter" | ||
| #define TKINTER_MOD "_tkinter" | ||
| // From module __file__ attribute to char *string for dlopen. | ||
| #define FNAME2CHAR(s) (PyBytes_AsString(PyUnicode_EncodeFSDefault(s))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This isn't exception proof -- | ||
| #else | ||
| #define TKINTER_PKG "Tkinter" | ||
| #define TKINTER_MOD "tkinter" | ||
| // From module __file__ attribute to char *string for dlopen | ||
| #define FNAME2CHAR(s) (PyString_AsString(s)) | ||
| #endif | ||
| void *_dfunc(void *lib_handle, const char *func_name) | ||
| { | ||
| // Load function, unless there has been a previous error. If so, then | ||
| // return NULL. If there is an error loading the function, return NULL | ||
| // and set error flag. | ||
| static int have_error = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This is fine, but is a sort of unidiomatic pattern. Rather than using a static variable | ||
| void *func = NULL; | ||
| if (have_error == 0) { | ||
| // reset errors | ||
| dlerror(); | ||
| func = dlsym(lib_handle, func_name); | ||
| const char *error = dlerror(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I think this only needs to be called if | ||
| if (error != NULL) { | ||
| PyErr_SetString(PyExc_RuntimeError, error); | ||
| have_error = 1; | ||
| } | ||
| } | ||
| return func; | ||
| } | ||
| int _func_loader(void *tkinter_lib) | ||
| { | ||
| // Fill global function pointers from dynamic lib. | ||
| // Return 0 fur success; 1 otherwise. | ||
| TCL_CREATE_COMMAND = (tcl_cc) _dfunc(tkinter_lib, "Tcl_CreateCommand"); | ||
| TCL_APPEND_RESULT = (tcl_app_res) _dfunc(tkinter_lib, "Tcl_AppendResult"); | ||
| TK_MAIN_WINDOW = (tk_mw) _dfunc(tkinter_lib, "Tk_MainWindow"); | ||
| TK_FIND_PHOTO = (tk_fp) _dfunc(tkinter_lib, "Tk_FindPhoto"); | ||
| TK_PHOTO_PUTBLOCK = (tk_ppb_nc) _dfunc(tkinter_lib, "Tk_PhotoPutBlock_NoComposite"); | ||
| TK_PHOTO_BLANK = (tk_pb) _dfunc(tkinter_lib, "Tk_PhotoBlank"); | ||
| return (TK_PHOTO_BLANK == NULL); | ||
| } | ||
| int load_tkinter_funcs(void) | ||
| { | ||
| // Load tkinter global funcs from tkinter compiled module. | ||
| // Return 0 for success, non-zero for failure. | ||
| int ret = -1; | ||
| PyObject *pModule, *pSubmodule, *pString; | ||
| pModule = PyImport_ImportModule(TKINTER_PKG); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. It's a matter of personal preference, but I generally like to avoid the excessive nesting by:
at the bottom | ||
| if (pModule != NULL) { | ||
| pSubmodule = PyObject_GetAttrString(pModule, TKINTER_MOD); | ||
| if (pSubmodule != NULL) { | ||
| pString = PyObject_GetAttrString(pSubmodule, "__file__"); | ||
| if (pString != NULL) { | ||
| char *tkinter_libname = FNAME2CHAR(pString); | ||
| void *tkinter_lib = dlopen(tkinter_libname, RTLD_LAZY); | ||
| if (tkinter_lib == NULL) { | ||
| PyErr_SetString(PyExc_RuntimeError, | ||
| "Cannot dlopen tkinter module file"); | ||
| } else { | ||
| ret = _func_loader(tkinter_lib); | ||
| // dlclose probably safe because tkinter has been | ||
| // imported. | ||
| dlclose(tkinter_lib); | ||
| } | ||
| Py_DECREF(pString); | ||
| } | ||
| Py_DECREF(pSubmodule); | ||
| } | ||
| Py_DECREF(pModule); | ||
| } | ||
| return ret; | ||
| } | ||
| #endif | ||
| #if PY3K | ||
| static PyModuleDef _tkagg_module = { PyModuleDef_HEAD_INIT, "_tkagg", "", -1, functions, | ||
| NULL, NULL, NULL, NULL }; | ||
| @@ -244,13 +360,20 @@ PyMODINIT_FUNC PyInit__tkagg(void) | ||
| import_array(); | ||
| #ifdef DYNAMIC_TKINTER | ||
| return (load_tkinter_funcs() == 0) ? m : NULL; | ||
| #else | ||
| return m | ||
| #endif | ||
| } | ||
| #else | ||
| PyMODINIT_FUNC init_tkagg(void) | ||
| { | ||
| import_array(); | ||
| Py_InitModule("_tkagg", functions); | ||
| #ifdef DYNAMIC_TKINTER | ||
| load_tkinter_funcs(); | ||
| #endif | ||
| } | ||
| #endif | ||
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.