GNOME Bugzilla – Bug 694604
Non-introspected signals do not marshal foreign types
Last modified: 2014-05-06 03:52:03 UTC
pygobject currently does not seem to marshal foreign types in signal handlers which are not introspectable. One example where I hit this issue is the "draw" event of the cairooverlay gst element (see http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-good-plugins/html/gst-plugins-good-plugins-cairooverlay.html). Is it not possible to also marshal foreign types in signals which are not introspected? At least for cairo.Context, it seems possible since there is a gobject type (CAIRO_GOBJECT_TYPE_CONTEXT) used in signals?
Can you provide a minimal example which demonstrates the problem? This might also be related to bug 694233.
This might also be related to bug 666862
Created attachment 237338 [details] Simple example exposing the issue I've attached a simple example exposing the issue mentioned.
Thanks. Verified. It seems like this could be possible but very difficult. I am planning an attempt to unify marshaling code paths post 3.8 and will keep this in mind. It should also be considered if support is added for custom marshalers described in bug 686261. I guess the GstCairoOverlay has no introspection typelib and this is why it is a problem?
Indeed. This holds generally for any software which uses a similar dynamic plugin type of system, although I guess it will not occur a lot. That said, it would be nice to have good support for gstreamer in pygobject. Thanks for looking into it.
Created attachment 237351 [details] ctypes hack You could hack it with ctypes, but this might not be portable and could break in the future. We might eventually expose a pointer attribute for boxed types as a capsule which might work better.
Yes I thought about that, but it seemed a bit fragile to take the context id + offset as a pointer to the boxed cairo_t*. In the end I went with a small C extension which does the same, but then just uses pygobject and pycairo.
(In reply to comment #6) Thanks for this hack Simon, works for me with py(2)cairo and cairooverlay element from GStreamer 1.2.3. Portability/fragility concerns noted.
*** Bug 726856 has been marked as a duplicate of this bug. ***
Looking into this a bit deeper, it turns out to be fairly trivial to fix. Notes on a potential approach: * Add a dependency to cairo-gobject [1] in the PyGObject _gi_cairo module [2]. * In the module init of pygi-foreign-cairo.c [3], register a GValue marshaler using "pyg_register_gtype_custom" [4] with simple converters, similar to what it is already doing for foreign marshaling. [1] http://cgit.freedesktop.org/cairo/tree/util/cairo-gobject [2] https://git.gnome.org/browse/pygobject/tree/gi/Makefile.am?id=3.11.92#n138 [3] https://git.gnome.org/browse/pygobject/tree/gi/pygi-foreign-cairo.c [4] https://git.gnome.org/browse/pygobject/tree/gi/pygtype.c?id=3.11.92#n631
Created attachment 272720 [details] [review] Move pygi foreign API into pygi-foreign-api.h Move only APIs necessary for registering foreign marshalers into pygi-foreign-api.h Remove "_real" from unused foreign APIs and add necessary includes to the rest of pygobject for calling these APIs directly. This is needed to avoid compilation problems when including pygobject.h in a foreign marshaling plugin which conflicts with pygobject-private.h, causing an inability to use the pygobject API from foreign marshaling plugins.
Created attachment 272721 [details] [review] Add cairo marshaling support for non-introspected signals Add link dependency of cairo-gobject to _gi_cairo_la needed for retrieving the GTypes of cairo classes. Always initialize foreign marshaling list at gi import time. This allows direct importing of gi._gi_cairo needed to bootstrap new GValue marshalers. Add GValue marshalers for cairo Context, Surface, FontFace, ScaledFont, and Pattern classes. Add override for CairoGObject (gi.repository.cairo) which bootstraps the cairo foreign marshaling plugin needed to install the GValue marshalers. Move all cairo related unittests into new tests/test_cairo.py file and add non-introspected signal marshaling tests.
Created attachment 272722 [details] [review] Add cairo marshaling support for non-introspected signals (added missed file)
Review of attachment 272720 [details] [review]: ::: gi/pygi-foreign-cairo.c @@ +32,3 @@ #endif +#include <cairo-gobject.h> This is a mistake that should go in the next patch. @@ +36,2 @@ +/* Limit PyGObject includes to externally usable APIs since _gi_cairo is + * built as a separate shared library. Clarify this comment.
Review of attachment 272722 [details] [review]: (self review) ::: gi/overrides/cairo.py @@ +40,3 @@ +ScaledFont = CairoGObject.ScaledFont +Surface = CairoGObject.Surface +image_surface_create = CairoGObject.image_surface_create Not sure we need to do all this, I think importing gi._gi_cairo should load the necessary GTypes... ::: tests/test_cairo.py @@ +7,3 @@ +try: + import cairo + from gi.repository import cairo as CairoGObject It is unfortunate if this is really needed. Perhaps a new technique needs to be looked at which can bootstrap the foreign lib automatically... ::: tests/test_everything.py @@ -48,3 @@ class TestEverything(unittest.TestCase): - def test_cairo_context(self): Moving these tests should go in a separate patch applied before this one.
Review of attachment 272722 [details] [review]: ::: tests/test_cairo.py @@ +7,3 @@ +try: + import cairo + from gi.repository import cairo as CairoGObject As noted in g_irepository_find_by_gtype [1] it is not possible to deduce the GI namespace of a GType if the GI repository is not loaded. In this particular case, we have a signal without introspection information passing a CairoGObjectContext to PyGI without gi.repository.cairo being loaded. I think a cleaner approach would be to use the "gi.require_foreign" API from bug 707735 which loads the appropriate GI repository. So instead of: import cairo from gi.repository import cairo as CairoGObject We would have: import cairo gi.require_foreign('cairo') At which point we don't need to cairo overrides hack. [1] https://developer.gnome.org/gi/stable/GIRepository.html#g-irepository-find-by-gtype
Comment on attachment 272720 [details] [review] Move pygi foreign API into pygi-foreign-api.h Attachment 272720 [details] pushed as def4714 - Move pygi foreign API into pygi-foreign-api.h
The following fix has been pushed: 31ecd93 Initialize the foreign API at PyGI load time
Created attachment 275929 [details] [review] Initialize the foreign API at PyGI load time Initialize the foreign struct list at gi._gi module load time. This ensures we always have a valid (non-null) list of foreign marshalers outside of the context of marshaling.
The following fix has been pushed: 4ee91a4 tests: Move cairo tests into test_cairo.py
Created attachment 275930 [details] [review] tests: Move cairo tests into test_cairo.py Move cairo related tests from test_everything.py into test_cairo.py
The following fix has been pushed: de827d0 Add cairo marshaling support for non-introspected signals Notes: Beyond the added tests which round trip non-introspected cairo objects through signals, the following has also been verified to bootstrap cairo GI for non-introspected foreign marshaling: gi.require_foreign('cairo') or from gi.repository import cairo as CairoGObject gi.require_foreign is preferred.
The following fix has been pushed: de827d0 Add cairo marshaling support for non-introspected signals
Created attachment 275933 [details] [review] Add cairo marshaling support for non-introspected signals Add link dependency of cairo-gobject to _gi_cairo_la needed for retrieving the GTypes of cairo classes. Add GValue marshalers for cairo Context, Surface, FontFace, ScaledFont, and Pattern classes.