fontend: Make OBSProject a parent window #11773
Conversation
kkartaltepe
added
Seeking Testers
|
Hello! I took this PR for a spin on my own KDE+Wayland setup on arch linux and can confirm it works. |
|
Commit prefix: |
kkartaltepe
force-pushed
the
projector-stuff
branch
from
a327f74 to
10fee81
Compare
|
|
kkartaltepe
force-pushed
the
projector-stuff
branch
from
10fee81 to
26de4bd
Compare
This window was previously a parent of OBSQTDisplay, however OBSQTDisplay is supposed to be completely rendered by OBS. On linux this causes issues like not having window decorations and weird rendering artifacts during resize. For wayland systems that negotiate explicit sync it also triggers protocol violations (crashes) due to Qt and OBS thinking they should be rendering this window. Instead make the projector a parent window owned by Qt that only contains one widget, the OBSQTDisplay. This makes the window behave like the main OBS window and preview. fixes obsproject#6283 Co-authored-by: cg2121 <claytong1214@gmail.com>
kkartaltepe
force-pushed
the
projector-stuff
branch
from
26de4bd to
003ac88
Compare
| weakSource(OBSGetWeakRef(source_)) | ||
| { | ||
| QVBoxLayout *layout = new QVBoxLayout; | ||
| layout->addWidget(&display); |
There was a problem hiding this comment.
This is dangerous insofar as layouts will not forward any move events that their parent widget receives, only resize events.
We need to handle the move event explicitly however, as the position of the native window attached to the OBSQTDisplay needs to be updated on move events as well, otherwise the native window (and thus the surface libobos will render to) will be "stuck" on the screen, untethered from the actual Qt widgets.
There was a problem hiding this comment.
Got it, though since there cant be any movements of the layout this sounds safe. Though that layouts need to propagate move events if they contain native children and dont sounds like we could raise a Qt bug for it.
There was a problem hiding this comment.
It's enough if the window that the layout belongs to is resized but the layout itself doesn't change size. The internal geometry calculations will figure out that it would have only "moved" within its window and thus not emit a "resize" event and then the rendered output will "hover" outside of the window.
If you wrap a QTDisplay inside a layout, you also have to explicitly update the native window dimensions relative to its layout position in the move event handler to fix this. @Warchamp7 ran into this exact issue in #11862.
The reason for this is that the native window exists outside of the widget coordinate system and needs to be absolutely resized and repositioned (by setting origin as well as dimensions), using the Qt-relative offsets and dimensions to create the appropriate values in "native space".
As layouts only handle resize events, they also can detect (and resize) native windows attached to widgets only during a resize.
There was a problem hiding this comment.
Right its position will always be 0,0 relative to the window so the problematic case of it ever changing doesnt seem possible. Warchamp was testing with more complicated layouts that could move.
The reason for this is that the native window exists outside of the widget coordinate system and needs to be absolutely resized and repositioned (by setting origin as well as dimensions), using the Qt-relative offsets and dimensions to create the appropriate values in "native space".
As layouts only handle resize events, they also can detect (and resize) native windows attached to widgets only during a resize.
Right that sounds like something that is worth reporting to qt, since any usage of native widgets within any slightly complicated layout sounds like it would trigger that. And with that description there is no way to solve this but to know the top level window and for the top level window to explicitly know every native child, which seems pretty broken compared to the composable widgets system qt has for everything else.
There was a problem hiding this comment.
Right its position will always be 0,0 relative to the window
That's not an assumption we should make or depend on, as it will lead to unexpected weird behaviour, e.g. whenever we choose to add a dynamic toolbar at the top of the window or make either changes to improve the interface or warchamp chooses to change the layout.
Right that sounds like something that is worth reporting to qt, since any usage of native widgets within any slightly complicated layout sounds like it would trigger that. And with that description there is no way to solve this but to know the top level window and for the top level window to explicitly know every native child, which seems pretty broken compared to the composable widgets system qt has for everything else.
Yeah we can report to get clarification on this, because the reason for layouts not propagating move events is that widgets in layouts don't need them (just their parent will need to move). And my impression is that we "abuse" native widget functionality in a way that it was not designed for and just got lucky so far.
There was a problem hiding this comment.
That's not an assumption we should make or depend on, as it will lead to unexpected weird behaviour, e.g. whenever we choose to add a dynamic toolbar at the top of the window or make either changes to improve the interface or warchamp chooses to change the layout.
Its the only requirement we even have for the projectors, im not sure how you can say we cant depend on it. And as in the 2nd part even if we did try to add any support it would be broken immediately since maybe warchamp would want a different top level to hold all those cool new features and just add the projector as a subwidget which would break all this again no matter what we did in this widget.
There was a problem hiding this comment.
Oh that's my bad - my comments concerned the use of OBSQTDisplay within widget layouts that use it as part of an overall interface, not the specific case of OBSProjector which is supposed to be a container for an OBSQTDisplay, which usually will not hold any further UI elements.
As long as any resize of the window will lead to the OBSProjector instance to be resized as well, then that event will propagate to the OBSQTDisplay even within a QLayout.
Description
This window was previously a parent of OBSQTDisplay, however OBSQTDisplay is supposed to be completely rendered by OBS. On linux this causes issues like not having window decorations and weird rendering artifacts during resize. For wayland systems that negotiate explicit sync it also triggers protocol violations (crashes) due to QT and OBS thinking they should be rendering this window.
Instead make the project a parent window owned by Qt that only contains one widget, the OBSQTDisplay. This makes the window behave like the main OBS window and preview.
fixes #6283
I gave up on gnome after seeing that qt and gnome just cant agree on how to fullscreen at the wayland protocol even, and just disable it like cg did in #11558 but only in the known broken config since this probably works on KDE but should be tested.
Still needs windows and mac testing since although its supposed to be more correct do all the sizing before
showthese changes might not be correct on all platforms.Motivation and Context
Presumably with syncobj being enabled this could now not only affect nvidia users, gnome enables it by default in wayland now so this issue could surface for anyone if mesa changes defaults.
How Has This Been Tested?
Works on sway/xwayland, "works" on gnome/xwayland.
Types of changes
Checklist: