Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WxCairo backend. #10395

Merged
merged 1 commit into from Feb 11, 2018

Conversation

Projects
None yet
4 participants
@anntzer
Copy link
Contributor

commented Feb 8, 2018

PR Summary

Same as #10210, but for wx.
attn @DietmarSchwertberger perhaps :-)
not release critical by any means, but would still be nice to have for 2.2.

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

@anntzer anntzer added the GUI/wx label Feb 8, 2018

@anntzer anntzer added this to the v2.2 milestone Feb 8, 2018

@DietmarSchwertberger

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2018

Unfortunately, the situation with Cairo binaries for my main platform Windows is still difficult.
I could review the files and confirm that the refactoring does not break anything, but can't test much on the Cairo side.
Anyway, I would like to see this and PR #10208 in 2.2. Even if they would not be widely used, it would be good to gain some experience and build upon for 3.0.

@anntzer

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2018

fwiw anaconda seems to provide pycairo binaries for Windows (https://anaconda.org/anaconda/pycairo).

@DietmarSchwertberger

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2018

Yes, I have seen this, but not tested whether it would be compatible with wx. My interest is in enabling people to embed Matplotlib into wx applications. With wxGlade now also the occasional user should be able to build a wx GUI. Anaconda is something that I would not recommend for this purpose and extracting binaries from Anaconda to 'normal' Python seems also a bit out of scope for most users...

@tacaswell tacaswell merged commit 0e07bdd into matplotlib:master Feb 11, 2018

7 of 8 checks passed

codecov/patch 12.79% of diff hit (target 50%)
Details
ci/circleci: docs-python27 Your tests passed on CircleCI!
Details
ci/circleci: docs-python35 Your tests passed on CircleCI!
Details
codecov/project/library 63.92% (target 50%)
Details
codecov/project/tests 98.61% remains the same compared to 84008ea
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details

@anntzer anntzer deleted the anntzer:wxcairo-minimal branch Feb 11, 2018

@DietmarSchwertberger

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2018

While testing for bug 10418 I tried all wx backends on my Ubuntu 17.10 VM. The old backends seem to be OK, but with the Cairo I got this one:

Traceback (most recent call last):
  File "fourier_demo_wx_sgskip.py", line 102, in sliderHandler
    self.param.set(value)
  File "fourier_demo_wx_sgskip.py", line 64, in set
    feedbackKnob.setKnob(self.value)
  File "fourier_demo_wx_sgskip.py", line 235, in setKnob
    self.canvas.draw()
  File "/home/osboxes/.local/lib/python3.6/site-packages/matplotlib-0+unknown-py3.6-linux-x86_64.egg/matplotlib/backends/backend_wxcairo.py", line 44, in draw
    buf = surface.get_data()
NotImplementedError: Surface.get_data: Not Implemented yet.

According to the documentation of pycairo, get_data() is 'Not yet available in Python 3'.
Maybe there's a workaround or there should be some documentation / warning?

@DietmarSchwertberger

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2018

OK, I've found a minor problem in 'backend_wxagg.py' which breaks embedding_in_wx3.py:
Toolbar is not imported from backend_wx and so from matplotlib.backends.backend_wxagg import Toolbar fails.

The original version was:

from .backend_wx import (_BackendWx, FigureManagerWx, FigureCanvasWx,
    FigureFrameWx, DEBUG_MSG, NavigationToolbar2Wx, Toolbar)

The new version is lacking the Toolbar import:

from .backend_wx import (
    _BackendWx, _FigureCanvasWxBase, FigureFrameWx, NavigationToolbar2Wx)

@tacaswell: would you mind commiting the Toolbar import or should I add it to the examples PR?

@anntzer

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2018

surface.get_data was implemented for Py3 in pycairo 1.11.0 (https://pycairo.readthedocs.io/en/latest/changelog.html#v1-11-0) and I would suggest just bumping the min version to it.

@tacaswell

This comment has been minimized.

Copy link
Member

commented Feb 11, 2018

That seems a bit agressive in terms of time (<1 yr) but not that aggressive in terms of versions (they are now in 1.16).

I think bumping the minimum on 1.11 and then having packages that can not hit that patch out the new cairo backends.

@DietmarSchwertberger

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2018

Ubuntu 17.10 comes with 1.10 in dist-packages. Even after -mpip I ended up with the old version. Now it's working and I can't reproduce why it failed initially. A small difference with the fourier demo: the waveforms are blue while with WxAgg they are red....

@DietmarSchwertberger

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2018

@anntzer : I have just checked again on Ubuntu. Indeed the red waveforms are plotted in blue.
Could it be that there's a mismatch in byte ordering? I tried green, which works OK. But blue and red are exchanged.

@anntzer anntzer referenced this pull request Feb 12, 2018

Closed

cairo rendering for wx #9202

4 of 6 tasks complete
@anntzer

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2018

Almost certainly a byteordering issue. I find https://wxpython.org/Phoenix/docs/html/wx.Bitmap.html#wx.Bitmap.FromBuffer a bit sparse in the description of the kind of input needed; I can probably manually work it out but do you know the details there (and spare me some manual testing)?

@anntzer anntzer referenced this pull request Feb 12, 2018

Merged

Fix wxcairo byteorder. #10429

0 of 6 tasks complete
@DietmarSchwertberger

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2018

I think it's actually https://wxpython.org/Phoenix/docs/html/wx.Bitmap.html#wx.Bitmap.FromBufferRGBA via wx_compat.BitmapFromBuffer = wx.Bitmap.FromBufferRGBA.
Cairo should be ARGB.
If at the same time Cairo and wx differ in LSB vs. MSB, then it would explain things where Alpha is correct and red/blue exchanged. Alpha must be correct as otherwise black or white would fail.

Anyway it seems that FromBufferRGBA would not create a copy of the memory area. For re-ordering that's required, though.
Maybe try CopyFromBuffer with format wx.BitmapBufferFormat_ARGB32:
https://wxpython.org/Phoenix/docs/html/wx.Bitmap.html#wx.Bitmap.CopyFromBuffer
That one explicitely mentions 'native endian order'. If it's still wrong, then a lowlevel re-ordering routine would be nice.

@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018

@anntzer

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2018

I think https://github.com/matplotlib/matplotlib/pull/10429/files is a reasonable way to do this because we're going to need to write it ourselves for tkcairo anyways (which also needs a argb32->rgba8888 conversion). (Ultimately I'll just factor it out in a helper function.)

But otherwise something like

diff --git a/lib/matplotlib/backends/backend_wxcairo.py b/lib/matplotlib/backends/backend_wxcairo.py
index 71a6831c7..337b5b2ea 100644
--- a/lib/matplotlib/backends/backend_wxcairo.py
+++ b/lib/matplotlib/backends/backend_wxcairo.py
@@ -44,13 +44,8 @@ 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)
+        self.bitmap = wx.EmptyBitmap(width, height).CopyFromBuffer(
+            surface.get_data(), wx.BitmapBufferFormat_ARGB32)
         self._isDrawn = True
         self.gui_repaint(drawDC=drawDC, origin='WXCairo')

seems to work too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.