libweston/compositor: Do not map subsurfaces without buffer

We can end in subsurface_committed() in different scenarios without the surface having an attached buffer. While setting the mapped state to true in that case doesn't matter for that (sub)surface itself, it triggers its own child subsurfaces to get mapped when they shouldn't.

Closes #426 (closed)

---

Includes !749 (merged)

Note that this patch only affect subsurfaces. I did a round of testing of some real-life applications that make use of subsurfaces:

• gstreamer waylandsink demos, both 1.18 and main/1.20
• firefox (wayland backend, with and without gfx.webrender.compositor.force-enabled)
• meshlab (now broken as expected, see https://bugreports.qt.io/browse/QTBUG-86229)
• libdecor

With this branch (i.e. including !749 (merged)) they now all behave as expected and in line with other compositors. I could not spot any regressions.

@daniels: gentle reminder to consider this for %10.0.1 as well (maybe you can add the tag so it doesn't get forgotten).

changed milestone to %10.0.1

resolved all threads

added 41 commits

• 492d60aa...dc3b3493 - 39 commits from branch wayland:main
• f56791b9 - libweston/compositor: Do not map subsurfaces without buffer
• 2a52cd8e - tests: Add test for subsurfaces mapping hierachies

Compare with previous version

added 3 commits

• 66374d48 - 1 commit from branch wayland:main
• ae50ca09 - libweston/compositor: Do not map subsurfaces without buffer
• 04ff4fbf - tests: Add test for subsurfaces mapping hierachies

Compare with previous version

approved this merge request

added 7 commits

• 04ff4fbf...73b17da7 - 5 commits from branch wayland:main
• 8b04534c - libweston/compositor: Do not map subsurfaces without buffer
• c83f0a15 - tests: Add test for subsurfaces mapping hierachies

Compare with previous version

enabled an automatic merge when the pipeline for c83f0a15 succeeds

This looks good to me. Our surface/view mapping code is really unpleasant and almost certainly wants to be refactored to be more explicit. But this does the right thing - we need to explicitly mark the subsurface as mapped in the core, since it's not managed by the shells like other surface types (this is suboptimal, as shells should only manage the view's map state, and the core should take care of the surface map state ...), but we don't need to explicitly unmap because this happens in the attach handler called for all surface types.

It's really nice to see the test exercising a lot of potential corner cases, and verifying the whole machinery rather than just the individual bug case.

Thanks @rmader!

merged

mentioned in merge request !869 (merged)