Wxwidgets / Kicad: High load with EGL support enabled (Issue #24977)
baldrianbandit
Description
When moving certain symbols on the screen in Kicad, CPU load ramps up to 100% easily while framerate drops.
Using builds of wxwidgets and Kicad without EGL support, it runs fine.
Versions
wxwidgets-common-git 3.2.6.r45.gfd0f60daea-1
wxwidgets-gtk3-git 3.2.6.r45.gfd0f60daea-1
xorg-server 21.1.14-1
xf86-video-amdgpu 23.0.0-2
6.11.9-arch1-1
To Reproduce:
See https://gitlab.com/kicad/code/kicad/-/issues/19149
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I'm really not sure what can/are we supposed to do here. If the only difference is using EGL vs using GLX, then it seems like EGL implementation (i.e. driver) is suboptimal, but there is nothing we can do about it at this level. If the problem is something that wxGTK itself does, it would be great to have a practical way to reproduce it, i.e. a patch to one of our OpenGL samples.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
baldrianbandit
Thanks for reaching out @vadz. May I ask for your opinion: I'm seeing this on all kind of machines with Intel / AMD consumer / professional grade GPUs running Arch Linux and Xorg. Do you think it might relate to either Xorg or drivers or the way Kicad uses Wxwidgets?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Sorry, I can't really answer that. My first instinct would be to say that if the same code works correctly when using GLX, then the problem should be in the EGL driver. But, of course, I could be missing something here, and either KiCad or wx could conceivable also have some bug in their OpenGL code which somehow doesn't manifest itself with GLX. It's just really difficult to know without having a simple way of reproducing the problem.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
wx 3.2.6 disables vsync when using EGL, while with GLX the vsync is enabled.
We should be drawing no faster than OP's mouse poll rate (just 125 Hz), which still may or may not cause this high CPU usage, because KiCad might be doing more than one draw call per mouse move.
With GLX the draw rate is limited to 60 Hz.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
To prevent EGL+Wayland from breaking, maybe wx can introduce a swap buffers call that uses Wayland protocols to wait.
While EGL+Xorg should use the normal swap interval method.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)To prevent EGL+Wayland from breaking, maybe wx can introduce a swap buffers call that uses Wayland protocols to wait.
While EGL+Xorg should use the normal swap interval method.
Would be great to have some API for this in wx 3.3, also explicit swap interval control.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)I still think we should just use GLX with X and not EGL at all (i.e. implement what was proposed in #22325), but I'm not sure what would you like to do for EGL/Wayland case: aren't we already waiting for Wayland as we only redraw from the frame callback handler? Or do you mean we should avoid doing anything in wxGLCanvasEGL::SwapBuffers() before the next callback call (which is something I'd be tempted to try)?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)I still think we should just use GLX with X and not EGL at all
Then separate versions of wx+apps need to be provided for X11 and Wayland.
If all the Wayland/wx bugs get fixed it should be fine to use EGL.
KiCad tries to render as soon as mouse move events arrive (possibly at 125Hz or 1000Hz rate). If swap interval is set, it waits for vsync in *SwapBuffers, not using much CPU cycles.
For Wayland I think it's possible to use wp_presentation_feedback::sync_output event to wait for vsync, with some timeout (33 ms?), and then call eglSwapBuffers with disabled swap interval.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)I still think we should just use GLX with X and not EGL at all
Then separate versions of wx+apps need to be provided for X11 and Wayland.
No, I mean select the backend to use dynamically. This requires quite some work, of course, which is why I still didn't do it, but I think it would be worth it.
If all the Wayland/wx bugs get fixed it should be fine to use EGL.
There is this "If", and supporting X also makes wxGLCanvasEGL code more complicated. It would be nice not to have to do it.
KiCad tries to render as soon as mouse move events arrive (possibly at 125Hz or 1000Hz rate).
But why do this instead of just calling Refresh() and let the system generate the repaint event?
If swap interval is set, it waits for vsync in
*SwapBuffers, not using much CPU cycles.
For Wayland I think it's possible to use wp_presentation_feedback::sync_output event to wait for vsync, with some timeout (33 ms?), and then call
eglSwapBuffers(andglXSwapBufferstoo on XWayland?) with disabled swap interval.
If you don't use wxEVT_PAINT at all, then you need some API to wait for vsync, but I think that the alternative to using the paint events should be rendering (possibly with blocking) from a worker thread instead of waiting for vsync in the main one, which seems both more complicated and less efficient...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)But why do this instead of just calling
Refresh()and let the system generate the repaint event?
Because if we catch GTK paint event on the canvas holder and call Refresh on the canvas, it triggers another paint event in the canvas holder, locking everything up. Works on MSW though. We need to redesign this canvas holder system.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)Calling Refresh() from wxEVT_PAINT handler doesn't seem like a good idea, I'm surprised it doesn't result in problems under MSW. But you wrote that you render "as soon as mouse moves", so what I meant was to call Refresh() from wxEVT_MOTION handler — and then just handle painting as usual.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)so what I meant was to call
Refresh()fromwxEVT_MOTIONhandler — and then just handle painting as usual.
If there's too much wxEVT_MOTION events (1000 Hz poll rate mouse) and a complex canvas, we can't call Refresh for each of the motion events. We skip the rendering until the majority of events have been processed. I think Refresh() is not immediate so it didn't work if we call it in the middle of processing these wxEVT_MOTION events.
Although it's been some time since I've looked at it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)And I don't think the paint events are related to monitors' vblank.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)Just a reminder that apart from Wayland-specific SwapBuffers hacks, the app should be able to set the swap interval.
Otherwise using GLX on X11 in wx 3.3 will cause high CPU load too (vs just with EGL on wx 3.2).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)@vadz If there a chance to address the above for wx 3.3?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)Sorry, I'm still not really sure what exactly needs to be done here, so I don't think it's reasonable to do something in 3.3.0. I do plan to return to OpenGL-related issues soon after it and hope to have this fixed in 3.3.1.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)- Revert the setting of 0 swap interval on GLX+X11 because this is a regression compared to 3.2.
- Don't override the swap interval, provide an API for the user app control it.
- Try to implement SwapBuffers that work on Wayland somehow (but a workaround can implemented in the user app at first)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)Re (1), do you mean to revert 3dde6bd (Disable swap interval in GLX wxGLCanvas implementation too, 2023-12-25) or somehow do it only for XWayland but not for the "genuine" X11? The former would reopen #23512 and I don't know how to do the latter (we could check for WAYLAND_DISPLAY in the environment, but is this really foolproof?).
Re (2), even if provide such an API, how would the application use it? If it can determine the best swap interval to use, why can't we do it? Of course, the problem is that I don't know how would the application do it too.
And for (3) I'm not sure which of GLX/X11, EGL/X11 and EGL/Wayland cases do you mean?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)Looking at KiCad code, it looks like we might just need to call wxGLSetSwapInterval() with 1 instead of 0 as argument?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)Re (1), do you mean to revert 3dde6bd (Disable swap interval in GLX wxGLCanvas implementation too, 2023-12-25) or somehow do it only for XWayland but not for the "genuine" X11? The former would reopen #23512 and I don't know how to do the latter (we could check for
WAYLAND_DISPLAYin the environment, but is this really foolproof?).
Re (2), even if provide such an API, how would the application use it? If it can determine the best swap interval to use, why can't we do it? Of course, the problem is that I don't know how would the application do it too.
Generally only the user app knows the best swap interval for its task, so wx can let the user app set it, instead of overriding it. KiCad can use it's own XWayland detection. Not all apps have multiple OpenGL windows affected by the Wayland throttling.
And for (3) I'm not sure which of GLX/X11, EGL/X11 and EGL/Wayland cases do you mean?
EGL+Wayland. But if these hacks don't work we'll probably have to live with high CPU usage until Wayland compositors stop throttling GPU drivers for background windows. (Now I'm not sure if SwapBuffers hacks should be implemented in wx)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)So in other words:
- Stop overriding the swap interval.
- Copy KiCad's
SetSwapIntervalimplementation to wx.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)Something like #25449 then (I'll add the docs and fallback on Mesa to it if this is fine)?
Or do you think that we shouldn't even default to setting it to 0 (or, maybe, to 1?)?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)Well there should be SetSwapInterval that is implemented in all backends, not just GLXSetSwapInterval.
Note that the swap interval is 0 by default on GLX, so it doesn't make sense to set it to 0 by wx.
For EGL the default is 1, and I don't think wx should change it by itself.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)Sorry, I'm totally lost, if the swap interval for GLX is 0 anyhow, why setting it to 0 in 3dde6bd fixed anything and why do we need to stop doing it?
And, of course, we had a long discussion and decided that setting it to 0 for EGL was better than blocking when a window was hidden...
I thought I understood what the problem was, but now I'm back to not understanding anything at all.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)Sorry, I'm totally lost, if the swap interval for GLX is 0 anyhow, why setting it to 0 in 3dde6bd fixed anything and why do we need to stop doing it?
3dde6bd had an effect because KiCad was setting the swap interval to 1, and 3dde6bd starter overriding it.
3dde6bd probably only worked around some GPU driver throttling bugs when using GLX+Wayland (XWayland). I think it should be solved at XWayland/compositor/driver levels instead. On GLX+X11 it only made the CPU usage higher than needed and allowed tearing.
It's better to move EGL+Wayland, GLX+Wayland detection logic to KiCad, so we can implement ugly hacks to make it work. But before that, we want to encourage users to report the throttling bugs to XWayland/compositors/Mesa.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)The initial goal of all these changes was to fix one well-defined problem: that an application with any hidden OpenGL windows slowed down to a crawl (#23512). I don't understand what goal are we pursuing now any more :-(
It seems to me that you're saying that we should just revert all the swap-interval changes in wx and let the application handle it itself. Please correct me if I'm wrong, but if not, I see 2 big problems with this:
- The goal above is not realized, i.e. by default applications using
wxGLCanvaswill still experience catastrophic slowdown if any of their windows is hidden when using Wayland. - 3.3.0 will be incompatible with 3.2, to which these changes have been backported, and where we're not going to back them out.
Maybe all we need to do is to indeed revert 3dde6bd? If I understand your explanations correctly, this shouldn't result in any change in behaviour by default (i.e. if the application doesn't do anything) and it's not in 3.2, so it doesn't introduce any incompatibility issues either.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)@dsa-t Any thoughts about just reverting that commit? I'd like to make RC 2 relatively soon.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)
- The goal above is not realized, i.e. by default applications using
wxGLCanvaswill still experience catastrophic slowdown if any of their windows is hidden when using Wayland.
I think this should be dealt with on Wayland/app/system levels, not in wx.
2. 3.3.0 will be incompatible with 3.2, to which these changes have been backported, and where we're not going to back them out.
It's fine in this case I think.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)
- The goal above is not realized, i.e. by default applications using
wxGLCanvaswill still experience catastrophic slowdown if any of their windows is hidden when using Wayland.
If you want to work around this at wx level, wx can set 0 swap interval for EGL by default. But it shouldn't override application's value.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)I don't think anybody expects their application to slow down to 1 update/minute, so I'd definitely like to solve it by default. The trouble is that I don't know how to know whether the application already does something about it on its own — we could check the swap interval, but I'm not even sure if its default value is always the same.
More concretely, I still have no idea what to do here...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)I don't think anybody expects their application to slow down to 1 update/minute
Apparently that's as-designed for Wayland.
I don't know how to know whether the application already does something about it on its own
Just make a SetSwapInterval in wx API, so apps don't have to implement their own.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)I don't think anybody expects their application to slow down to 1 update/minute
Apparently that's as-designed for Wayland.
This wouldn't be the only discussable Wayland choice... We try to conform to the native platform, of course, but I'm pretty sure nobody expects their application, which works fine under Windows and, maybe, even used to work fine with a previous version of wx under Linux, to suddenly start doing this.
I don't know how to know whether the application already does something about it on its own
Just make a SetSwapInterval in wx API, so apps don't have to implement their own.
OK, I can extend #25449 to all platforms and remember if it was called and not call it ourselves if it has been. If this is enough, let's do it.
BTW, I'd welcome any thoughts from the developers of the other projects using OpenGL with wx, especially if you already do something with the swap interval.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#24977)Some inspiration from SDL for Wayland: RetroPie/SDL@4bd1379
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
baldrianbandit
Closed #24977 as completed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
baldrianbandit
baldrianbandit left a comment (wxWidgets/wxWidgets#24977)Closing this since it seems somewhat solved to me.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#24977)FWIW with #26023 it will be possible to choose to use GLX (with X11 only, of course) with the same binary.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.