-
-
Notifications
You must be signed in to change notification settings - Fork 145
Fixed bug: Flickering UI on canvas #127 #1150
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
base: 0.10
Are you sure you want to change the base?
Conversation
|
Just out of curiosity, how does removing the following from |
Incorporated changes to wxCanvas implementation (Amulet-Team/Amulet-Map-Editor#1150), after updating base to 0.10.42
|
I implemented your fix into my flatpak project, but all I get is this now: What did you test this on? I'm running in a 3.12.9 VENV, and in Flatpak which is also 3.12. |
|
I have been busy working on other things so I haven't had a chance to look over this.
I don't see this in the diff. This was something I changed recently. The diff seems a bit messed up so maybe something bugged.
I think you have experienced this issue before. Does it happen without these changes? |
When I revert the source and remove the changes, it doesn't happen. As for platform independence, wxPython behaves differently on each platform. During my adventures in Python Problem Solving (which has been quite the infuriating ride sometimes!), here's what I've learned about the platform differences in wxPython:
This means that there's no universally functional way to implement wxPython - you have to do some sort of platform or feature detection to make everything work everywhere. I was also informed that platform detection (which was used in this fix) is less reliable than using feature detection with And of course, within Linux, sometimes you have to further differentiate between X11 and Wayland, though |
|
Some of those are just platform differences that there are no way around. |
|
Switching to QT means a complete re-write. Like, from the ground up - total overhaul. I mean, you could mix and match, but would probably be a coding nightmare. Wayland support would improve at least, and since that seems to be where they want to go anyway, keeping X11 support in the long term probably doesn't make sense. |
|
I have completely rewritten the core library so the GUI needs rewriting anyway |
I believe this is just a merge conflict with #1161 because we are both changing the line creating self._location_button (currently line 74 in file.py). The code you referenced came from their commit. My change was simply to switch from using the canvas to the newly created wx_parent. I have tried to resolve the merge conflict, but I'm not super familiar with github and resolving merge conflicts, so let me know if I've messed it up. |
Hmmm, that's strange. I'm using anaconda on Manjaro. I originally tested with python 3.11, but just tried it with python 3.12 and it seems to be ok, I will try pulling down your code and see if I can troubleshoot. It looks like it is crashing in the wx._glcanvas.GLAttributes function which should be unrelated to my changes, but I'll try checking it out. |
| if sys.platform == "linux": | ||
| self.panel = wx.Panel(self, style=wx.STAY_ON_TOP) | ||
| self.button_sizer = wx.BoxSizer(wx.HORIZONTAL) | ||
| self.button_sizer.AddStretchSpacer(1) | ||
| self.button_sizer.Add(self.panel, 0, wx.EXPAND, 0) | ||
| self._file_panel = FilePanel(self, self.panel) | ||
| self.panel.SetSizer(self._file_panel) | ||
| self.panel.SetBackgroundColour((155, 178, 216, 255)) | ||
| self._canvas_sizer.Add(self.button_sizer, 0, wx.EXPAND, 0) | ||
| else: | ||
| self._file_panel = FilePanel(self, self) | ||
| self._canvas_sizer.Add(self._file_panel, 0, wx.EXPAND, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue exists on macos as well. Your check only applies the custom behaviour to linux.
Can the linux behaviour be used on windows and macos as well?
I would prefer to have your custom behaviour on all platforms if it works on all platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this as well in the long run, but this was a quick way to keep the same windows behavior while making the linux side usable again.
The 2 switches on sys.platform I had to use were:
-
Make the BaseCanvas inherit from wx.Window instead of wx.glcanvas.GLCanvas. This seems to be related to some underlying difference in the implementation of wx.glcanvas.GLCanvas, though I am not extremely familiar with wx widgets. I think switching to Qt in the future is the solution to this.
-
Giving the FilePanel an underlying panel on linux. Since linux seems unable to support transparent backgrounds for wx elements on top of a GLCanvas, I added this panel with a solid background. We could do the same on windows, but the buttons would no longer have a transparent background. Not the end of the world, but I'd say a slightly negative change, so I maintained the previous behavior for windows. I'm fine with it either way though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I understand
| if sys.platform == "linux": | ||
| Canvas_Type = wx.Window | ||
| else: | ||
| Canvas_Type = GLCanvas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer one consistent behaviour across all platforms.
Does the wx.Window behaviour work across all platforms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to not work correctly on Windows. Similar (though different) flickering behavior occurs when trying to use the Linux solution on Windows.
|
I have created an Ubuntu virtual environment to review this. |
|
The VM was working well yesterday but has been a nightmare to work with today for some reason. Ultimately I think these changes are a bit too intrusive and I am worried they may break some things I can't think of. I have come up with a much less intrusive solution in #1203 that hooks into the This does not 100% solve the flickering issue. There is the odd frame where the overlay is not rendered for some reason but it is a lot better than it was before. @Ryan3141 @EvilSupahFly @Alastair-L please can you review #1203 and let me know how it behaves on your computers. |
|
Apparently my attempted fix does not work correctly. I am hesitant to merge this because there are a few breaking changes and I don't know what effect that will have. |

This fixes button flickering on linux #127. This seems to be due to attaching wx widgets directly to a bare wx.GLCanvas and also using bare wx.BoxSizers. With minor modification this may help for macOS, though I have no way to test it.
I fixed this by adding a member variable to the BaseCanvas class called _opengl_canvas. For Windows this member variable is simply set to self. For linux the BaseCanvas now inherits from wx.Window and _opengl_canvas points to a GLCanvas. The surrounding code was updated refer to _opengl_canvas whenever relevant rather than the BaseCanvas widget. On linux the _opengl_canvas also needed to forward all of its input events to the BaseCanvas.
Another issue causing the flickering seemed to be the use of raw wx.BoxSizers for layouts. I fixed this in several cases by adding a containing wx.Panel for linux and a solid background color. To facilitate this, several classes required a separation of references to the canvas and to their wx parent class.
Fixes #127