Summary: | Frame callbacks not send on 'empty' commits | ||
---|---|---|---|
Product: | [Plasma] kwin | Reporter: | Robert Mader <robert.mader> |
Component: | wayland-generic | Assignee: | KWin default assignee <kwin-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | argymeg, david, kde, nate, physkets, seqularise |
Priority: | HI | Keywords: | wayland |
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | Other | ||
OS: | Linux | ||
Latest Commit: | https://invent.kde.org/plasma/kwin/commit/256245cd5004f0e199ffc5f04585c92f8399c895 | Version Fixed In: | 5.21 |
Sentry Crash Report: |
Description
Robert Mader
2020-10-31 10:33:21 UTC
Thank you for reporting this issue. Indeed, currently, kwin won't repaint a window unless it's been damaged. Also, if you discover a bug in kwin/wayland in the future, can you please use official channels to report it? It looks this issue has been known for almost half a year and we are made aware of it only now. :( Yeah, the correct forwarding of bugs it an area for improvement :/ Now that I have an account here I'll try to do that more regularly. For FF<->Gnome and FF<->wlroots it appears to work mostly by having some accidental overlap between devs who work on both projects. There is, however, a list of known KWin-only bugs at https://bugzilla.mozilla.org/show_bug.cgi?id=1609115 - maybe one of you finds time to go through them and check if there's still something actionable? Concerning this bug I'd like to highlight the Mutter solution (https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1218) which decoupled frame callback emission from drawing all together. There's a list of scenarios where things can go wrong otherwise - for example if damage only happens in a part of a surface that's off-screen (see https://gitlab.gnome.org/GNOME/mutter/-/issues/817#note_738633). Just so you have that in mind when drafting a solution. Some more notes here: we just enabled the frame callback based vsync emulation by default on the Wayland backend in Firefox 85. The Wayland backend is not enabled by default yet and 85 will only released in the end of January, however users explicitly enabling it and running nightly or soon beta will run into this issue. Therefore I personally am quite eager to see it resolved :) The benefit of using frame callbacks are twofold: on one hand we can run at refresh rate on non-60Hz monitors, on the other we can stop rendering when the compositors stops sending callbacks, for example when the window is completely covered, reducing power consumption. So overall quite desirable. I'm do not know how kwin handles callbacks, but I'd like to point out to how mutter initially handled this: https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/839/diffs?commit_id=d49d10b14f4e0fa80e6867979b26fab383610b39 While the solution is not optimal (and thus was later replaced with a more comprehensive solution mentioned above already), something similar might work for kwin as well - and in mutter it was only 4 lines. Hoping you find time and energy to look into this soon, best regards Heh, in our cases, it will be a bit more involved. The compositor tries to avoid repainting as much as possible. In particular, if no damage has occurred, nothing will be repainted, no page flip will be scheduled, etc. With the ongoing compositing timing rework, most of those issues will be addressed. So, it should be a rather trivial change. @Robert I have one question though, does Firefox continuously creates frame callbacks (even if nothing has been changed) or only on demand? in our case* > does Firefox continuously creates frame callbacks (even if nothing has been changed) or only on demand? Not completely continuous, but always if FF *may* repaint soon. > The compositor tries to avoid repainting as much as possible. Yeah, that was also a motivation in Mutter to move frame callback handling completely outside of the paint code eventually (https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1218). Totally agree that this is quite involving to really get right :/ FTR.: the FF patch was pushed back at least another train, so will not ride to release before 86 in February. A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/654 Git commit 798cfae321c9cfac3b99638fbaf5b0a61713cfaf by Vlad Zahorodnii. Committed on 02/02/2021 at 14:00. Pushed by vladz into branch 'master'. Introduce SurfaceInterface::hasFrameCallbacks() The new method can be used to check if the surface has a pending frame callback. M +5 -0 src/server/surface_interface.cpp M +1 -0 src/server/surface_interface.h https://invent.kde.org/plasma/kwayland-server/commit/798cfae321c9cfac3b99638fbaf5b0a61713cfaf Git commit 6f5e8e7a6a56b26592d47c0b85e8d86b4c765874 by Vlad Zahorodnii. Committed on 02/02/2021 at 14:01. Pushed by vladz into branch 'Plasma/5.21'. Introduce SurfaceInterface::hasFrameCallbacks() The new method can be used to check if the surface has a pending frame callback. (cherry picked from commit 798cfae321c9cfac3b99638fbaf5b0a61713cfaf) M +5 -0 src/server/surface_interface.cpp M +1 -0 src/server/surface_interface.h https://invent.kde.org/plasma/kwayland-server/commit/6f5e8e7a6a56b26592d47c0b85e8d86b4c765874 Git commit 72fda78cf612bd4fd82132f381e74bcad70c7be7 by Vlad Zahorodnii. Committed on 03/02/2021 at 06:50. Pushed by vladz into branch 'master'. wayland: Schedule repaints even on empty commits If only frame callbacks have been committed, the compositor must still schedule compositing. M +28 -0 scene.cpp M +2 -0 scene.h M +4 -20 subsurfacemonitor.cpp M +1 -0 subsurfacemonitor.h M +6 -0 toplevel.cpp M +2 -0 toplevel.h https://invent.kde.org/plasma/kwin/commit/72fda78cf612bd4fd82132f381e74bcad70c7be7 Git commit 256245cd5004f0e199ffc5f04585c92f8399c895 by Vlad Zahorodnii. Committed on 03/02/2021 at 06:50. Pushed by vladz into branch 'Plasma/5.21'. wayland: Schedule repaints even on empty commits If only frame callbacks have been committed, the compositor must still schedule compositing. (cherry picked from commit 72fda78cf612bd4fd82132f381e74bcad70c7be7) M +28 -0 scene.cpp M +2 -0 scene.h M +4 -20 subsurfacemonitor.cpp M +1 -0 subsurfacemonitor.h M +6 -0 toplevel.cpp M +2 -0 toplevel.h https://invent.kde.org/plasma/kwin/commit/256245cd5004f0e199ffc5f04585c92f8399c895 Great, thanks a lot! |