Fix wxcairo byteorder. #10429

anntzer · 2018-02-12T07:05:00Z

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

DietmarSchwertberger · 2018-02-14T22:02:22Z

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.

anntzer · 2018-02-14T23:37:00Z

It's really not that hard to do it in python, see e.g. also https://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 be slightly 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.

DietmarSchwertberger · 2018-02-15T01:31:29Z

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.

anntzer · 2018-02-15T02:12:21Z

The seemingly normal

diff --git a/lib/matplotlib/backends/backend_wxcairo.py b/lib/matplotlib/backends/backend_wxcairo.py
index 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.

DietmarSchwertberger · 2018-02-16T20:37:26Z

It might actually be trivial: there's wx.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')

anntzer · 2018-02-17T02:28:01Z

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 · 2018-02-17T15:29:32Z

OK, the version with wx.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.)

Backport PR #10429 on branch v2.2.x

Fix wxcairo byteorder.

fff747a

anntzer added the GUI: wx label Feb 12, 2018

tacaswell added this to the v2.2.0 milestone Feb 12, 2018

DietmarSchwertberger added 2 commits February 17, 2018 16:17

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

95f0748

remove now unused imports again

54a7b6c

DietmarSchwertberger self-requested a review February 17, 2018 15:30

DietmarSchwertberger approved these changes Feb 17, 2018

View reviewed changes

tacaswell merged commit f3a9a65 into matplotlib:master Feb 19, 2018

lumberbot-app bot pushed a commit that referenced this pull request Feb 19, 2018

Backport PR #10429: Fix wxcairo byteorder.

e0a591a

lumberbot-app bot mentioned this pull request Feb 19, 2018

Backport PR #10429 on branch v2.2.x #10540

Merged

anntzer deleted the wxcairobyteorder branch February 19, 2018 22:24

dstansby added a commit that referenced this pull request Feb 20, 2018

Merge pull request #10540 from matplotlib/auto-backport-of-pr-10429

4658539

Backport PR #10429 on branch v2.2.x

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix wxcairo byteorder. #10429

Fix wxcairo byteorder. #10429

Uh oh!

anntzer commented Feb 12, 2018 •

edited by DietmarSchwertberger

Loading

Uh oh!

DietmarSchwertberger commented Feb 14, 2018 •

edited

Loading

Uh oh!

anntzer commented Feb 14, 2018

Uh oh!

DietmarSchwertberger commented Feb 15, 2018

Uh oh!

anntzer commented Feb 15, 2018

Uh oh!

DietmarSchwertberger commented Feb 16, 2018

Uh oh!

anntzer commented Feb 17, 2018

Uh oh!

DietmarSchwertberger commented Feb 17, 2018

Uh oh!

Uh oh!

Uh oh!

Fix wxcairo byteorder. #10429

Fix wxcairo byteorder. #10429

Uh oh!

Conversation

anntzer commented Feb 12, 2018 • edited by DietmarSchwertberger Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

DietmarSchwertberger commented Feb 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anntzer commented Feb 14, 2018

Uh oh!

DietmarSchwertberger commented Feb 15, 2018

Uh oh!

anntzer commented Feb 15, 2018

Uh oh!

DietmarSchwertberger commented Feb 16, 2018

Uh oh!

anntzer commented Feb 17, 2018

Uh oh!

DietmarSchwertberger commented Feb 17, 2018

Uh oh!

Uh oh!

anntzer commented Feb 12, 2018 •

edited by DietmarSchwertberger

Loading

DietmarSchwertberger commented Feb 14, 2018 •

edited

Loading