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

Don't accumulate damage from surfaces below opaque surfaces #864

Closed
soreau opened this issue Nov 12, 2020 · 4 comments · Fixed by #2275
Closed

Don't accumulate damage from surfaces below opaque surfaces #864

soreau opened this issue Nov 12, 2020 · 4 comments · Fixed by #2275
Milestone

Comments

@soreau
Copy link
Member

soreau commented Nov 12, 2020

Currently, wayfire does unnecessary repainting, behind surface opaque regions. This can be seen by placing a surface with opaque region set, in front of a client with constant damage like video playback, with showrepaint plugin enabled.

Actual: Wayfire repaints the entire surface damage region, even if opaque surfaces that are not being damaged exist in front/above.

Expected: Wayfire does not repaint the opaque regions (except for valid surface damage) of surfaces that are in front of other damage behind/under it.

@ammen99 ammen99 added this to the 0.8 milestone Dec 11, 2020
@valpackett
Copy link
Contributor

Seems like there was an attempt to do this, I wonder what went wrong:

/* Subtract opaque region from workspace damage. The views below
* won't be visible, so no need to damage them */
repaint.ws_damage ^= ds->surface->get_opaque_region(pos);
repaint.to_render.push_back(std::move(ds));

@ammen99
Copy link
Member

ammen99 commented Jan 27, 2021

@myfreeweb That code avoids repainting below opaque surfaces. However, at this point the damage is already scheduled. The issue however is about ignoring client damage as a whole if it comes from below an opaque surface. For example, you have an animated background and fullscreen client on top. Currently, we'd repaint only the fullscreen client, however, we shouldn't repaint at all unless the fullscreen client itself commits.

@valpackett
Copy link
Contributor

valpackett commented Jan 27, 2021

Ah, hm, seems like that does work now. I remember somehow seeing full screen damage for my wallpaper plugin when moving an opaque window, but now it's clearly only thin rectangles where it's actually needed. Maybe it was just some mistake on my end.

But in the "animated background and fullscreen client on top" situation, why wouldn't that code essentially reduce the actually-repainted damage area to zero?

@ammen99
Copy link
Member

ammen99 commented Jan 27, 2021

But in the "animated background and fullscreen client on top" situation, why wouldn't that code essentially reduce the actually-repainted damage area to zero?

That code is executed top-down with the total damage already computed.

@ammen99 ammen99 modified the milestones: 0.8, 0.9 Jun 3, 2021
ammen99 added a commit that referenced this issue Mar 27, 2024
Fixes #864

The optimization is hidden behind a flag for now, needs more testing.
ammen99 added a commit that referenced this issue Mar 27, 2024
Fixes #864

The optimization is hidden behind a flag for now, needs more testing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants