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

Queue refresh after drawing (required for wayland) #72

Closed
wants to merge 1 commit into from

Conversation

@Mystro256
Copy link
Contributor

Mystro256 commented Feb 12, 2017

Recreation of Pull Request #68:

Original description:

I noticed that compiling and running on Wayland (an alternative to XOrg/X11 for BSD and Linux), the screen would never refresh automatically unless the window is resized. Requesting a Refresh() following DrawArea() and DrawOSD() does the trick.

Although with that said, Wayland seems to require drawing with Cairo, as the GUI will segfault without it, so I've wrapped this in #ifndef NO_CAIRO to avoid any unintended issues with other systems.

Side note, I noticed a comment in CairoDrawingPanel::DrawArea() about drawing with cairo being very slow for WXMSW. Perhaps this also fixes that issue, which could be required for cairo to work correctly, but I'm not sure.

Quote from last relevant comment:
@rkitover said:

So according to the docs here:

http://docs.wxwidgets.org/2.8/wx_wxwindow.html#wxwindowrefresh

this queues a repaint event for the window, so what your change does is, in effect, make a loop that sends another paint event after every paint event. This may be OK if we can't find a more elegant solution.

What we would ideally like to do is check in OnIdle() if the emulator system has a frame ready to be drawn that is different from the previous frame, and only then send a Refresh() to the panel.

@rkitover rkitover added the bug label Feb 12, 2017
@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 12, 2017

@Mystro256

This is the issue with OpenGL on Wayland:
http://trac.wxwidgets.org/ticket/17702

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 12, 2017

@Mystro256

What happens without this patch, does the window just stay black? Does it stay on the first frame drawn?

@Mystro256

This comment has been minimized.

Copy link
Contributor Author

Mystro256 commented Feb 12, 2017

@ZachBacon

This comment has been minimized.

Copy link
Contributor

ZachBacon commented Feb 12, 2017

I think the plan is going to replace wxglcanvas with something like egl.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 12, 2017

@Mystro256

I have an idea I used in my useless mac Quartz2D renderer, I will make a branch and link it here.

rkitover added a commit that referenced this pull request Feb 12, 2017
At the end of CairoDrawingPanel::DrawArea, draw a 4 pixel rectangle with
a transparent brush and pen on the wx dc to hopefully make the frame
appear.

I used this trick with the Mac Quartz2D renderer.
@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 12, 2017

@Mystro256 can you please try the cairo_wayland branch and see if it fixes the display issue for Cairo on Wayland?

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 12, 2017

@Mystro256 BTW we are on #vba-m on Freenode if you use IRC.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 12, 2017

@Mystro256 also wanted to ask, does the Simple renderer work on Wayland? How does the performance of the Simple and Cairo renderers compare?

I'd look at some of this stuff myself, but I don't have a non-VM linux right now, and my VM software only supports xorg.

@Mystro256

This comment has been minimized.

Copy link
Contributor Author

Mystro256 commented Feb 13, 2017

Simple and Cairo seems to be the same, but I think this likely because I believe only cairo drawing for WX is supported on wayland, so it's likely all routed via the same code paths in the backend. Disabling the cairo cmake flag seems to segfault vba-m on launch in wayland, which seems consistent with my guess (similar to how the opengl wx bug mentioned above causes sefaults due to hardcored Xorg calls).

I'll give the branch a shot and let you know. As for IRC, I don't use it unfortunately.

@Mystro256

This comment has been minimized.

Copy link
Contributor Author

Mystro256 commented Feb 13, 2017

Looks like the fix does nothing. I take back what I said about cairo drawing being required for build time, but simple and cairo still gives similar performances, likely due to most wayland implementations using cairo to draw regardless. Opengl still crashes vba-m in wayland, which is due to the bug you described above.

I've been meaning to bug a WX dev about it, but I'm betting it's not due to a WX bug, as I noticed wx tutorials using refresh() to force re-queuing redraws. I'm assuming this doesn't affect Xorg, as I'm told XOrg takes a more "active" approach to redrawing windows, opposed to wayland which is made to be more efficient. I have no idea how Windows or Quartz works.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 13, 2017

@Mystro256 that still leaves the question of why the Simple renderer works and Cairo does not.

I'm going to turn Wayland back on in my Fedora VM and try it out.

@Mystro256

This comment has been minimized.

Copy link
Contributor Author

Mystro256 commented Feb 13, 2017

Sorry I may have been confusing on how I phrased it; both simple and Cairo have this issue. I thought it was just Cairo but it seems like it's both. While OpenGL does not work due to the wx bug described above.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 14, 2017

I tried to make a Fedora VM with Wayland, but Wayland seems to require kernel modesetting which is only supported by specific video drivers, so no luck. I'll try on my pc laptop.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 14, 2017

@Mystro256 I got wayland running on my arch VM with weston + fbdev-backend.so + uvesafb, so I should be able to test this now.

rkitover added a commit that referenced this pull request Feb 14, 2017
At the end of CairoDrawingPanel::DrawArea, draw a 4 pixel rectangle with
a transparent brush and pen on the wx dc to hopefully make the frame
appear.

I used this trick with the Mac Quartz2D renderer.
@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 14, 2017

@Mystro256 I cannot reproduce the rendering bug in Weston, both the simple and cairo renderers work fine.

Any chance you could try rawhide and see if it happens there too?

If not, perhaps we should just revisit this when f26 comes out.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 14, 2017

@Mystro256 on second thought, it was explained to me in #wayland that this is likely a difference between mutter and weston, so I will try f25 on real hardware after all.

@Mystro256

This comment has been minimized.

Copy link
Contributor Author

Mystro256 commented Feb 14, 2017

Understood; do you think this is a bug in mutter itself since it works in Weston? I've already found a few bugs already that I've reported to gnome in regards to Wayland.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 14, 2017

It's either mutter or some sort of interaction between GTK and mutter, I don't know enough about this to say, also I need to install something to take a look myself.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 16, 2017

@Mystro256 I managed to install rawhide in vmware fusion (which has a real drm driver) and run the gnome wayland, and reproduce the issue you are talking about.

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 17, 2017

@Mystro256 I think I have a fix, please try the cairo_wayland branch again.

You had the right idea about using Refresh(), just had to find the right place to put it.

Basically what I did was, instead of having:

core -> systemDrawScreen() -> panel::DrawArea(frame_buffer) -> panel::DrawArea(device_context)

I changed it to:

core -> systemDrawScreen() -> panel::DrawArea(frame_buffer) -> panel::Refresh() -> panel::PaintEv() -> panel::DrawArea(device_context)

and this puts things into the right state so that the frame actually appears in the window.

There is some other stuff I need to do for Wayland, I need to add some code to check for Wayland at runtime and disable opengl, otherwise you have the situation where the user has opengl in their config and the app segfaults on startup. And unify the OSD code (there are two sets of code right now to draw the OSD and sometimes you even see them both draw text on the frame....)

@Mystro256

This comment has been minimized.

Copy link
Contributor Author

Mystro256 commented Feb 18, 2017

Indeed, this seems to have a much cleaner implementation rather than my hacky workaround. Seems to work with Xorg and Wayland just fine. Providing it works with Msys and Mac, commit away.

Thanks for your help, I made a tracker bug for the opengl issue: #76

rkitover added a commit that referenced this pull request Feb 20, 2017
Use a gdk call (part of gtk) to detect if the app is using Wayland, and
if so disable OpenGL, because wxGLCanvas segfaults under Wayland.

This requires linking to gtk libs, so add some cmake and installdeps
code for that.

If the user has opengl as the render method in their config, it will not
be changed, but at runtime will be set to simple under Wayland.

Call Refresh() to queue a PaintEv from DrawArea(data) instead of calling
DrawArea(device_context) directly. This fixes frames not displaying
under Wayland.

Remove the DrawOSD() call from PaintEv, this was causing the OSD to
sometimes show up twice in one frame, because DrawArea(data) draws the
OSD directly on the buffer.

Add new files wayland.cpp and wayland.h with a bool IsItWayland()
global function. This is where the aforementioned gdk call is.
rkitover added a commit that referenced this pull request Feb 20, 2017
Use a gdk call (part of gtk) to detect if the app is using Wayland, and
if so disable OpenGL, because wxGLCanvas segfaults under Wayland.

This requires linking to gtk libs, so add some cmake and installdeps
code for that.

If the user has opengl as the render method in their config, it will not
be changed, but at runtime will be set to simple under Wayland.

Call Refresh() to queue a PaintEv from DrawArea(data) instead of calling
DrawArea(device_context) directly. This fixes frames not displaying
under Wayland.

Remove the DrawOSD() call from PaintEv, this was causing the OSD to
sometimes show up twice in one frame, because DrawArea(data) draws the
OSD directly on the buffer.

Add new files wayland.cpp and wayland.h with a bool IsItWayland()
global function. This is where the aforementioned gdk call is.
@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 20, 2017

@Mystro256 please review and try a98f4a1 from the cairo_wayland branch. It also addresses #76.

If all looks good to you will merge to master.

How close are we to a usable fedora package? What are the major blockers aside from this?

@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 20, 2017

@Mystro256 another thing I wanted to mention, when I was trying Weston on my arch VM, I didn't know that the arch wxWidgets is compiled with GTK2 not GTK3, so the app was actually running under XWayland and not Wayland and that's why I didn't see the bug.

@Mystro256

This comment has been minimized.

Copy link
Contributor Author

Mystro256 commented Feb 20, 2017

Ah yes, that would make a lot of sense. You would have to use "wxgtk-gtk3" from the AUR to test Wayland on Arch.

Looks good, feel free to pull the change in. I think the only other issue is #51.

rkitover added a commit that referenced this pull request Feb 20, 2017
Disable OpenGL support under Wayland because wxGLCanvas segfaults, and
fix an issue with drawn frames not appearing.

If the user has opengl as the render method in their config, it will not
be changed, but at runtime will be set to simple under Wayland.

To fix the issue with frames not being drawn, Call Refresh() to queue a
PaintEv from DrawArea(data) instead of calling DrawArea(device_context)
directly.

Also remove the DrawOSD() call from PaintEv, this was causing the OSD to
sometimes show up twice in one frame, because DrawArea(data) draws the
OSD directly on the frame data.

Add new files wayland.cpp and wayland.h with a bool IsItWayland() global
function. This uses a GDK (part of GTK) call to detect Wayland.  This
unfortunately requires linking GTK libs separately.

Add cmake code to detect the version of GTK used by the wx being linked
and link it as well. Add gtk2 and gtk3 dev packages to the code for the
supported linux dists in ./installdeps.
@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Feb 20, 2017

Merged to master, closing this.

@rkitover rkitover closed this Feb 20, 2017
rkitover pushed a commit that referenced this pull request Aug 15, 2019
Allow enabling GBA RTC regardless of rom types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.