Skip to content

Improve robustness against attempts to produce zero-sized textures

Simon McVittie requested to merge wip/smcv/shell2538-robustness into master

  • cogl: Don't allow creating sized textures with 0 pixels

    A texture with no pixels isn't a useful thing to have, and breaks assumptions elsewhere. For example, CoglFramebuffer assumes that after a texture has been allocated, it will have width and height both greater than 0.

    In particular, this works around a crash when gnome-shell tries to blur a background that hasn't yet had any space allocated for it - which it seems is really an actor layout bug, but more robustness seems good to have.

    Workaround for gnome-shell#2538 (closed).

  • cogl: Defend against empty or unallocated framebuffers

    It isn't immediately obvious that this is impossible, because there's some "action at a distance" going on with framebuffers that have their size set lazily, after their textures get allocated; so let's make this a critical warning rather than crashing.

    In particular, this works around a crash when gnome-shell tries to blur a background that hasn't yet had any space allocated for it - which it seems is really an actor layout bug, but more robustness seems good to have.

    Workaround for gnome-shell#2538 (closed).

As with gnome-shell!1171 (closed), I know @verdre was looking into a proper solution, but I wanted to get at least a workaround into place, so that upgrading from 3.36.0 to 3.36.1 in Debian isn't a stability regression.

I would have no problem with dropping this in favour of an actual solution in the layout code, but adding at least one of these commits might be a good "defence in depth" approach to preventing compositor crashes, which are sufficiently disruptive that robustness seems good to have.

When reproducing gnome-shell#2538 (closed) with gnome-shell 3.36.1 (in particular without gnome-shell!1171 (closed)) and mutter 3.36.1 with these changes, I get the g_return_val_if_fail (width >= 1, NULL) triggered.

The "Defend against empty or unallocated framebuffers" commit is essentially untested (because the first commit seems to stop us from getting there), but converting a hard crash into a critical warning seems hopefully harmless, and from looking at CoglFramebuffer code I couldn't easily tell whether a caller was able to ensure that the assertion can't fail. If maintainers would prefer just the first commit, I'm happy to drop the second one.

Edited by Simon McVittie

Merge request reports