-
Notifications
You must be signed in to change notification settings - Fork 232
layer: Only consider main surface for geometry #1897
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
Conversation
YaLTeR
left a comment
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.
What makes me feel uneasy is that surface with its subsurfaces is supposed to be an indivisible whole. I feel like this is just client working around the lack of set_geometry in layer-shell, and it works because of wrong handling in other compositors...
Yes absolutely and this has been brought up time and time again for both wlr-layer-shell and the proposed ext-layer-shell. But this PR doesn't concern itself with that, it just fixes smithay's behaviour. Since the bbox was used for the layer_geometry, e.g. the render-elements were computed with an already offset layer position and then applied the subsurface offset again, leading to the whole layer-surface shifting (without any re-arranging happening). That cannot be intended in either case. The alternative would be to purposefully offset the render-location to counter act any negatively offset subsurfaces. But that would also break clients and need some additions to our arrange-method to consider the layer-surface size to include subsurfaces (which it doesn't currently). So I'd rather leave the agency to offset the layer-surfaces manually or clip it to compositors. |
f449cdd to
4c5744e
Compare
|
ping @cmeissl, @PolyMeilex, @chrisduerr, @kchibisov as YaLTeR wanted some more input and you spring to mind as potentially having an opinion on this. (No obligation and sorry for the ping, if you aren't interested at all.) |
|
Is there a simpler client to test this? The one in the issue seems like something I'd rather avoid. I'm curious if this is even an issue on catacomb, since I don't actually use Smithay's layer shell geometry calculation. So I guess I'd be in favor of just letting the compositors deal with it. :P |
None I know of, but you basically just need something that sticks to either the top or left edge and then spawns a subsurface with a negative offset. If you are affected depends on if you use bbox to determine the render-location before calling |
Well I don't call that either. 😄 |
Fixes #1858.
The program in question (wl_shimeji) might position some negatively offset surfaces on it's main layer-shell surface.
This resulted in a bounding box with negative offset, which in turn shifted the render-location, which is obviously wrong.
I'd rather leave it to compositors to potentially clamp contents outside of the positioned layer-surface, as this obviously allows surfaces to overlap other layer-shell surfaces.
But in any way this is better than our current behaviour, which breaks the placement logic.