Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

wx backends: don't use ClientDC any more#11944

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

Draft
DietmarSchwertberger wants to merge14 commits intomatplotlib:main
base:main
Choose a base branch
Loading
fromDietmarSchwertberger:wx-noclientdc

Conversation

DietmarSchwertberger
Copy link
Contributor

PR Summary

This is work in progress and needs some testing...

Fix for

At the moment the wx backends have problems on Wayland as ClientDC is not working.
This PR changes the backends to trigger a Refresh instead and then paint everything in theEVT_PAINT handler. Also,BufferedPaintDC is used now as this is the standard approach for double-buffering.

Overlays are used for drawing rubberbands. This is also done on Mac OS now, but only with a border, not with a half-transparent box, as this does not work on Retina displays.

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 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
Copy link
ContributorAuthor

@anntzer : would you mind reviewing this one?
You have more experience with all the backends.
If I'm right, thendraw() needs to callRefresh as it can be called by the user or fromshow(). On the other hand, if_isDrawn is False, then_OnPaint is callingdraw to render the figure before painting. So at the momentRefresh is probably called too often.
Maybe it would be best to move the rendering into a separate method, e.g._draw() or add an argumentdraw(refresh=True).
What do you think?

@tacaswelltacaswell added this to thev3.1 milestoneAug 27, 2018
@tacaswell
Copy link
Member

Tagged this as 3.1 for now. Please re-milestone if it needs to be backported.

@@ -761,29 +763,6 @@ def _get_imagesave_wildcards(self):
wildcards = '|'.join(wildcards)
return wildcards, extensions, filter_index

def gui_repaint(self, drawDC=None, origin='WX'):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Need to add an API change note (it's mostly an implementation detail so not going to ask a deprecation period, which probably wouldn't make sense anyways).

# Macs. N.B. In future versions of wx it may be possible to
# detect Retina displays with window.GetContentScaleFactor()
# and/or dc.GetContentScaleFactor()
self.retinaFix = 'wxMac' in wx.PlatformInfo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

All these attributes need removal notices.


if previous_rubberband:
# extend by one pixel to avoid residuals on Mac OS
if left > 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I tend to write theseleft = max(left - 1, 0) as the bound is clearer (you don't need to check whether it was > or >=). Not that it really matters here, just a suggestion.

self.gui_repaint(drawDC=drawDC)
drawDC.Destroy()
# draw without transparency which is buggy on Retina displays
dc.SetPen(wx.Pen(wx.BLUE, 1, wx.SOLID))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The blue seems a bit deeper than in the original version. Also none of the other backends (well, dunno for macos) actually color the rectangle, so perhaps we may just leave it as transparent).


class FigureCanvasWx(_FigureCanvasWxBase):
# Rendering to a Wx canvas using the deprecated Wx renderer.

def draw(self, drawDC=None):
def draw(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Add note re: API change.

@anntzer
Copy link
Contributor

I don't know the details of the wx drawing model, but for Qt we do callupdate() fromdraw() (and we do wantfig.canvas.draw() to update the actual widget; that's a very common way of doing it), andhttp://doc.qt.io/qt-5/qwidget.html#update states that

This function does not cause an immediate repaint; instead it schedules a paint event for processing when Qt returns to the main event loop. This permits Qt to optimize for more speed and less flicker than a call to repaint() does.Calling update() several times normally results in just one paintEvent() call.

so we're not overdoing things. How are things on wx's side?

@DietmarSchwertberger
Copy link
ContributorAuthor

DietmarSchwertberger commentedAug 27, 2018
edited
Loading

@anntzer
Thanks a lot.
With the implementation as it is now,Refresh gets called from the EVT_PAINT handler, so I'm quite sure it's being painted twice if_isDrawn is False.
It's probably best to separate into the API functiondraw() and the internal rendering function_draw().

About the rubberband:
Currently the wx backends use the coloured box with 50% transparency, except on Mac OS due to the high dpi problems of the overlays.
RubberbandWx draws just a black border on Mac OS. I'm not sure about theNavigationToolbar2Wx
Of course, it would be possible to unify this and it would save a few lines. On the other hand, I like the semi-transparent blue box.
I will change to a dotted black line on Mac OS, though. That's probably more common than a solid blue or black line.

I will add notes about the API changes.
I thought about keepingretinaFix, but it would have to be moved to the canvas anyway.

I will come back to you when I've done the changes. (I need to push the commits to be able to test on Mac OS.)

@anntzer
Copy link
Contributor

retinaFix can be made private if it moves somewhere else, it's not as if it was ever intended as a public API anyways.

@ChrisBarker-NOAA
Copy link

RubberbandWx draws just a black border on Mac OS.
I will change to a dotted black line on Mac OS, though. That's probably more common than a solid blue or black line.

If it can be a dashed line drawn with XOR -- that's ideal for making it visible against any backround. Now suer that that interacts with Overlay, though

@DietmarSchwertberger
Copy link
ContributorAuthor

@ChrisBarker-NOAA : thanks, but this does not seem to work. I think that this also did not work with ClientDC on Mac OS. The only way to get this behaviour would be to modify the canvas' bitmap, as it was done with the previous implementation of NavigationToolbar2Wx on Mac OS. This is something that I would like to avoid, though.

"""
DEBUG_MSG("draw()", 1, self)
def _draw(self):
DEBUG_MSG("_draw()", 1, self)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

just kill these DEBUG_MSGs?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks. Will do so.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

There are also some functionsdebug_on_error,fake_stderr andraise_msg_to_str which probably do not serve a purpose. It would probably be best to remove these as well. What dou you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We can always remove them later (or now if you want), the point was just not to introduce more of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

And/or replace by logging messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Maybe combine_draw() anddraw() by adding arefresh=True argument todraw() that controls whetherself.Refresh() is called, and then usingself.draw(refresh=False) instead ofself._draw().

@DietmarSchwertberger
Copy link
ContributorAuthor

@ChrisBarker-NOAA : using anything than a simple drawing function would also cause problems on Retina displays, as the overlay background is scaled wrongly. I think that was the reason why overlays were not used on Mac OS previously.
With a simple nontransparent pen, this does not matter as the underlay does not shine through.

@ChrisBarker-NOAA
Copy link

I just looked at some old overlay code of mine -- and indeed, not XOR. So what I did is draw a dashed black and white line:

            # a black and white line so you can see it over any color.            dc.SetPen(wx.Pen('WHITE', 2))            dc.SetBrush(wx.TRANSPARENT_BRUSH)            dc.DrawLine(self.StartMove, pos)            dc.SetPen(wx.Pen('BLACK', 2, wx.SHORT_DASH))            dc.DrawLine(self.StartMove, pos)

@DietmarSchwertberger
Copy link
ContributorAuthor

DietmarSchwertberger commentedSep 4, 2018
edited
Loading

Ah, thanks, that's probably the best option. I will take this one for Mac OS.

@anntzer
Copy link
Contributor

  1. Zooming doesn't actually work once you release the mouse window (I get a gray background with nothing).
  2. I got extremely confused at how to adapt mplcairo's wx integration to the new code (currently athttps://github.com/anntzer/mplcairo/blob/master/lib/mplcairo/wx.py, new version athttps://github.com/anntzer/mplcairo/blob/wx/lib/mplcairo/wx.py, would appreciate if you could advise...
  3. Should the definition ofdraw (which just calls_draw andRefresh) be moved to _FigureCanvasWxBase instead of being duplicated? Also perhaps rename_draw to something clearer, e.g._update_bitmap or similar (if I understood its goal correctly).

@DietmarSchwertberger
Copy link
ContributorAuthor

@anntzer :
On what platform and backend does zooming fail?

Doesn't the supplied code forbackend_wxcairo.py work?
Basically, the backend should render intoself.bitmap, callRefresh and then_FigureCanvasWxBase._OnPaint will paint the bitmap to the screen, usingBufferedPaintDC (which is the standard approach for double-buffering on wx).

    def _draw(self):        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    def draw(self):        """        Render the figure and trigger a screen refresh.        """        self._draw()        self.Refresh()

Unfortunately,draw can't be centralized at_FigureCanvasWxBase as then it would be called recursively fromFigureCanvasAgg.draw assuper().draw() resolves to_FigureCanvasWxBase.draw.
I think I commented some time ago that I don't like the implementation ofFigureCanvasAgg.draw, as it is very intransparent what gets called...

@anntzer
Copy link
Contributor

On what platform and backend does zooming fail?

Fedora 28 (in case anyone wonders, my Arch machine is momentarily out of service...).

Doesn't the supplied code for backend_wxcairo.py work?

Actually a basic pop-up-a-plot work, it's animation that doesn't work (e.g. examples/animation/strip_chart.py), sorry, should have clarified that.

I can't (don't want to) use the codepath from backend_wxcairo because that disables blitting (you redraw the whole figure at every iteration). backend_wxcairo correctly inherits supports_blitting = False from backend_cairo (even though technically it's only gtk3cairo that doesn't support blitting (because it needs to draw directly on the X surface that gtk3 handles to it, whereas wxcairo could just stash the ImageSurface and blit normally from it), but that's an unrelated point...), but for mplcairo.wx I have no reason not to support blitting.

@DietmarSchwertberger
Copy link
ContributorAuthor

OK, so inmplcairo/wx.py just replace thegui_repaint call with a call toRefresh.
Withbackend_wxcairo the strip_chart animation seems to work for me on Ubuntu with Wayland.

I don't have a Fedora 28 VM. So I need some time to install one.

@anntzer
Copy link
Contributor

anntzer commentedSep 5, 2018
edited
Loading

Replacing gui_repaint by Refresh works in the old codebase (before this PR). After this PR, if I replace gui_repaint by Refresh, it looks like something is preventing wx from running its event loop, unless I try to throw events at it: the plot stays blank (except for the axes), but if I click on the window, or on one of the buttons (let's say the mouse is initially out of the window) then I get one or two updates to the animation, which then freezes again, and so on.
(As a comparison, things work just fine on qt and tk, seehttps://github.com/anntzer/mplcairo/blob/master/lib/mplcairo/qt.py andhttps://github.com/anntzer/mplcairo/blob/master/lib/mplcairo/tk.py for how they are done.)

@DietmarSchwertberger
Copy link
ContributorAuthor

DietmarSchwertberger commentedSep 5, 2018
edited
Loading

OK, maybe you can attach a script that uses mplcairo for animation. If I got it right, then I need to place the contents from mplcairo somewhere next to such a script and then just run the script, right? I could then try at least on Ubuntu with and without Wayland.
Or would I need some more complicated installation?

@ChrisBarker-NOAA
Copy link

That behavior of Refresh() is expected:

https://wxpython.org/Phoenix/docs/html/wx.Window.html#wx.Window.Refresh

.Refesh() tell wx that the Window needs to be repainted, but it doesn't trigger a Paint event right then.

The usual way to get it done NOW is:

.Refresh()

followed by

.Update()

Update(self)Calling this method immediately repaints the invalidated area of the window and all of its children recursively (this normally only happens when the flow of control returns to the event loop).Notice that this function doesn’t invalidate any area of the window so nothing happens if nothing has been invalidated (i.e. marked as requiring a redraw). Use Refresh first if you want to immediately redraw the window unconditionally.

BTW, not sure if there is any point to using a BufferedDC. The idea behind double buffering is that you only repaint the screen once the drawing is complete on an offscreen buffer. But int his case, all the drawing is being done by AGG or Cairo to an offscreen buffer anyway, and the Draw() call is simply blitting that image to the Screen. -- no need to blit it to an offscreen buffer first, only to have to transfer it again.

NOTE: I'm working off ten year(or more!) old information here, back when I did a lot with wx drawing, (And BufferedDC didn't exist, so I wrote my own) but I suspect this part is the same.

@DietmarSchwertberger
Copy link
ContributorAuthor

Honestly, I have never used Update. Would be worth a try, but in my tests the Refresh with later repaint was enough. It could be that Update has the same problems as ClientDC, but that would need testing.

I would not expect BufferedDC to make an additional copy, as it just receives the buffer bitmap that was filled from AGG or Cairo:dc = wx.BufferedPaintDC(self, bmp).
The wx backend code is somewhat inefficient, as it makes copies of the bitmap on e.g. Windows, but that's a different story...

@ChrisBarker-NOAA
Copy link

No, Update is not quite as aggressive as ClientDC about "NOW!" -- At least on OS-X, Client DC does it really right now, which may interfere with other drawing the OS is doing, whereas Update() puts it on the queue for the OS right away, but still lets the OS schedule it.

Due to this, on the past, on OS-X, using Refresh(), Update() and drawing in a PaintDC was not fast enough to follow the mouse...

As for the BufferedDC -- if it's not writing to a buffer, then what's the point ???

Note also that OS-X (and some others?) is already double-buffered by the OS anyway.

Maybe wx.Window.IsDoubleBuffered() would help here?

@ChrisBarker-NOAA
Copy link

Hmm -- not sure the point of wxBufferedPaintDC if you aren't drawing to it anyway... (as opposed to simply doing a PaintDC.DrawBitmap() call, but it looks like you are right. I did see this note in the BufferedDC docs:

"Finally, please note that GTK+ 2.0 as well as OS X provide double buffering themselves natively. You can either use wx.Window.IsDoubleBuffered to determine whether you need to use buffering or not, or use wx.AutoBufferedPaintDC to avoid needless double buffering on the systems which already do it automatically."

@anntzer
Copy link
Contributor

(unrelated point: the timer bug was fixed in#12023, you can just rebase on top of it)

@tacaswelltacaswell removed the Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labelMay 1, 2020
@tacaswelltacaswell modified the milestones:v3.3.0,v3.4.0May 1, 2020
@tacaswell
Copy link
Member

re-milestoned to 3.4 and removed the release critical label.

@anntzer
Copy link
Contributor

(Neither of my examples are using Wayland.)

@DietmarSchwertberger
Copy link
ContributorAuthor

@anntzer Thanks. Could you please check whether these are all gtk2 builds? From the recipe file, I would guess that the conda build is gtk2.

@anntzer
Copy link
Contributor

it's gtk2/wxWidgets 3.0.5 on fedora, gtk3/wxWidgets 3.0.4 on arch.

@DietmarSchwertberger
Copy link
ContributorAuthor

I have just spent too much time again with the ArchLinux2018.09 VM. The platform upgrade commandpacman -Syu is broken. The guest extensions do not work properly. The python-wxpython package installs a wxPython for Python3.8 while the platform Python is 3.7.

I'm not willing to work on fixing platform problems after all the experiences with the Arch and Fedora VMs.
If nobody wants to debug on his platform(s), then I would suggest to close this PR and declare Wayland unsupported.

@jklymak
Copy link
Member

Is it possible to resurrect the non-Wayland parts of this PR? If not, what is the todo here?

@jklymakjklymak marked this pull request as draftApril 23, 2021 16:32
@QuLogicQuLogic modified the milestones:v3.5.0,v3.6.0Aug 18, 2021
@timhoffmtimhoffm modified the milestones:v3.6.0,unassignedApr 30, 2022
@story645story645 modified the milestones:unassigned,needs sortingOct 6, 2022
@github-actions
Copy link

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

@github-actionsgithub-actionsbot added the status: inactiveMarked by the “Stale” Github Action labelMay 13, 2023
@github-actionsgithub-actionsbot removed the status: inactiveMarked by the “Stale” Github Action labelMay 30, 2023
@rcomerrcomer added the status: inactiveMarked by the “Stale” Github Action labelMay 30, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@newvillenewvillenewville left review comments

@anntzeranntzeranntzer left review comments

@jklymakjklymakjklymak left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Projects
None yet
Milestone
future releases
Development

Successfully merging this pull request may close these issues.

11 participants
@DietmarSchwertberger@tacaswell@anntzer@ChrisBarker-NOAA@newville@abulka@jklymak@QuLogic@story645@timhoffm@rcomer

[8]ページ先頭

©2009-2025 Movatter.jp