NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.4k

Fix wxcairo byteorder.#10429

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

Jump to bottom

Merged

tacaswell merged 3 commits intomatplotlib:masterfromanntzer:wxcairobyteorder

Feb 19, 2018

Merged

Fix wxcairo byteorder.#10429

tacaswell merged 3 commits intomatplotlib:masterfromanntzer:wxcairobyteorder

Feb 19, 2018

Conversation

Copy link

Contributor

anntzer commentedFeb 12, 2018•
edited by DietmarSchwertberger
Loading

PR Summary

I'm still not sure what I'm supposed to do with premultiplication (https://wxpython.org/Phoenix/docs/html/wx.Bitmap.html#wx.Bitmap.FromBufferRGBA is not very clear...) but at least this should get the byteorders correct).
See#10395 (comment).

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

Fix wxcairo byteorder.

fff747a

anntzer added the GUI: wx label

Feb 12, 2018

tacaswell added this to thev2.2.0 milestone

Feb 12, 2018

Copy link

Contributor

DietmarSchwertberger commentedFeb 14, 2018•
edited
Loading

I can only try this next weekend.
I would be nice to have a byte re-ordering function as C extension. That would look much nicer.

If the bitmap has the right size already, I would suggest to re-use the buffer of the bitmap.
Would it be possible to make Cairo render into the bitmap buffer and then re-order this in place? That would be even better.
This would minimize the number of memory allocations.

Copy link

ContributorAuthor

anntzer commentedFeb 14, 2018

It's really not that hard to do it in python, see e.g. alsohttps://github.com/matplotlib/matplotlib/pull/10436/files#diff-8b813556cc81b4a1a066f8d62e6519aeR28 which is perhaps an even better way to do this (and if both PRs come in I'll factor that out in a common function). I'm not too worried about inefficiency of the numpy version (pure C may beslightly more efficient but that's true for everything using numpy and this specific part is no worse than a bunch of other places where we use numpy).

cairo can render to a user-supplied buffer (https://pycairo.readthedocs.io/en/latest/reference/surfaces.html#cairo.ImageSurface.create_for_data) but I'm not sure whether wx can expose access to the bitmap buffer? Note also that the cairo buffer requires specific data alignment (although the doc is not totally clear on whether alignment is only required for performance or also just for correctness). In any case I'd say the current cairo backend is so unefficient that an extra buffer copy is the last of my worries.

Copy link

Contributor

DietmarSchwertberger commentedFeb 15, 2018

It seems that wx.Bitmap does not expose the memory buffer, even though that would be an obvious feature.

But there's this method which could be used if the size did not change:https://wxpython.org/Phoenix/docs/html/wx.Bitmap.html#wx.Bitmap.CopyFromBuffer

Maybe one of the formats does already the required reordering:

wx.BitmapBufferFormat_RGBA
wx.BitmapBufferFormat_ARGB32

In that case,BitmapFromBuffer could be replaced by:

if not self.bitmap or size_changed:    self.bitmap = wx.Bitmap(width, height, 32)self.bitmap.CopyFromBuffer(buf, wx.BitmapBufferFormat_...)

That's a bit inefficient as the Bitmap's memory will be initialized, but most of the time it saves allocation/deallocation.

Copy link

ContributorAuthor

anntzer commentedFeb 15, 2018

The seemingly normal

diff --git a/lib/matplotlib/backends/backend_wxcairo.py b/lib/matplotlib/backends/backend_wxcairo.pyindex 71a6831c7..84b82e64c 100644--- a/lib/matplotlib/backends/backend_wxcairo.py+++ b/lib/matplotlib/backends/backend_wxcairo.py@@ -36,6 +36,7 @@ class FigureCanvasWxCairo(_FigureCanvasWxBase, FigureCanvasCairo):         _FigureCanvasWxBase.__init__(self, parent, id, figure)         FigureCanvasCairo.__init__(self, figure)         self._renderer = RendererCairo(self.figure.dpi)+        self.bitmap = wxc.EmptyBitmap(0, 0)      def draw(self, drawDC=None):         width = int(self.figure.bbox.width)@@ -44,13 +45,11 @@ class FigureCanvasWxCairo(_FigureCanvasWxBase, FigureCanvasCairo):         self._renderer.set_ctx_from_surface(surface)         self._renderer.set_width_height(width, height)         self.figure.draw(self._renderer)-        buf = np.frombuffer(surface.get_data(), dtype="uint8").reshape((height, width, 4))-        if sys.byteorder == "little":-            b, g, r, a = np.rollaxis(buf, -1)-        else:-            a, r, g, b = np.rollaxis(buf, -1)-        rgba8888 = np.dstack([r, g, b, a])-        self.bitmap = wxc.BitmapFromBuffer(width, height, rgba8888)+        if ((width, height)+                != (self.bitmap.GetWidth(), self.bitmap.GetHeight())):+            self.bitmap = wxc.EmptyBitmap(width, height)+        self.bitmap.CopyFromBuffer(+            surface.get_data(), wx.BitmapBufferFormat_ARGB32)         self._isDrawn = True         self.gui_repaint(drawDC=drawDC, origin='WXCairo')

gives me (py2.7 wx 3.0.2.0; don't have phoenix installed now)

  File "matplotlib/pyplot.py", line 253, in show    return _show(*args, **kw)  File "matplotlib/backend_bases.py", line 192, in show    manager.show()  File "matplotlib/backends/backend_wx.py", line 1298, in show    self.canvas.draw()  File "matplotlib/backends/backend_wxcairo.py", line 53, in draw    surface.get_data(), wx.BitmapBufferFormat_ARGB32)  File "/usr/lib/python2.7/site-packages/wx-3.0-gtk3/wx/_gdi.py", line 814, in CopyFromBuffer    return _gdi_.Bitmap_CopyFromBuffer(*args, **kwargs)RuntimeError: Failed to gain raw access to bitmap data.

Copy link

Contributor

DietmarSchwertberger commentedFeb 16, 2018

It might actually be trivial: there'swx.lib.wxcairo.BitmapFromImageSurface.
I've just tested on Windows and wxPython 4.0.1.
Could you please try on 3.0.2? If it's working there as well, I will test on Ubuntu as well.

    def draw(self, drawDC=None):        import wx.lib.wxcairo as wxcairo        width = int(self.figure.bbox.width)        height = int(self.figure.bbox.height)        surface = cairo.ImageSurface(cairo.FORMAT_ARGB32, width, height)        self._renderer.set_ctx_from_surface(surface)        self._renderer.set_width_height(width, height)        self.figure.draw(self._renderer)        self.bitmap = wxcairo.BitmapFromImageSurface(surface)        self._isDrawn = True        self.gui_repaint(drawDC=drawDC, origin='WXCairo')

Copy link

ContributorAuthor

anntzer commentedFeb 17, 2018

I can't easily test on Windows. Looks like you have access to a wider range of setups than I do, so feel free to push a better solution to this PR or open another one.

DietmarSchwertberger added2 commits

February 17, 2018 16:17

use wx.lib.wxcairo.BitmapFromImageSurface to convert from cairo to wx

95f0748

remove now unused imports again

54a7b6c

Copy link

Contributor

DietmarSchwertberger commentedFeb 17, 2018

OK, the version withwx.lib.wxcairo.BitmapFromImageSurface is working on Ubuntu with 4.0.1 and 3.0.2 as well. Also it has been in the old and new wxPython demos. So it should work everywhere.
Thanks a lot (I've followed your suggestion and am using the cairo Windows binaries from Anaconda.)