Translation-node: changing an offset can leave stale pixels

[Ckrest]

Wayfire version

0.11.0 (git), wlroots 0.20.0

Analysis and line references below are against upstream master at 4c34a7bf.

My running build is a local branch based on upstream f058fdab and carries unrelated
local patches.

GPU / Driver

NVIDIA RTX 4090, proprietary 580.173.02, OpenGL 4.6.0, Vulkan 1.4.312.

The issue is in scene-graph damage tracking and is not GPU-specific.

Describe the bug

translation_node_t::get_bounding_box() includes the current offset, but
translation_node_t::set_offset() only assigns the new offset and emits no damage:

wf::geometry_t wf::scene::translation_node_t::get_bounding_box()
{
    return get_children_bounding_box() + get_offset();
}

void wf::scene::translation_node_t::set_offset(wf::pointf_t offset)
{
    this->offset = offset;
}

The header documents damage as the caller's responsibility:

/**
 * Set the offset the node applies to its children.
 * Note that damage is not automatically applied.
 */

However, the direct XDG-toplevel wm_offset update path does not honor that contract.
When a client commits a new XDG window-geometry origin without a size change,
xdg_toplevel_t::handle_surface_commit() updates wm_offset and emits
xdg_toplevel_applied_state_signal; xdg_toplevel_view_t::handle_toplevel_state_changed()
then calls surface_root_node->set_offset(...) followed by
scene::update(..., update_flag::GEOMETRY).

That path never supplies an old/new damage pair, and update_flag::GEOMETRY refreshes
visibility and input state without generating rendering damage. The newly covered area
gets drawn, but the vacated area is not necessarily repainted.

Expected behavior

Both the node's old and new coverage should be damaged when its translation changes. The
vacated region should be repainted without requiring unrelated client damage or another
compositor reconfiguration.

To Reproduce

Video demonstration: https://www.youtube.com/watch?v=7BUkTUqCypc

Reproducer source: https://gist.github.com/Ckrest/7a80769ac67f8d369e35f037e2465b26

The client paints a 900×900 buffer exactly once, declares a 500×500 XDG window geometry,
and then alternates only the geometry origin between (200, 200) and (0, 0) every two
seconds. Because the buffer is never repainted, all subsequent damage is the compositor's
responsibility. Stale content remains in the vacated region on every origin change.

Build and run

curl -sLO https://gist.githubusercontent.com/Ckrest/7a80769ac67f8d369e35f037e2465b26/raw/offsetdemo.c

proto="$(pkg-config --variable=pkgdatadir wayland-protocols)/stable/xdg-shell/xdg-shell.xml"
wayland-scanner client-header "$proto" xdg-shell-client-protocol.h
wayland-scanner private-code  "$proto" xdg-shell-protocol.c

cc offsetdemo.c xdg-shell-protocol.c $(pkg-config --cflags --libs wayland-client) -o offsetdemo
./offsetdemo

Additional context

I ran into it because older versions of electron register themselves as minimized before wayland accualy suspends them. This causes the clients to stop painting their glow or shadow, and wayland shifts thier positions to where the glow was briefly before minimizing. That shift on minimizing or unminimizing caused the Shadow from the windows not to be properly marked as damaged in cleanup, leaving behind pixels.

I tested the following local change:

void wf::scene::translation_node_t::set_offset(wf::pointf_t offset)
{
    if (this->offset == offset)
    {
        return;
    }

    wf::regionf_t damage = get_bounding_box();
    this->offset = offset;
    damage |= get_bounding_box();
    wf::scene::damage_node(this, damage);
}

This emits a single unioned damage signal, and only when the offset actually changes.
With it, the reproducer no longer leaves stale pixels in either direction and ordinary
window dragging continues to work. The existing subsurface reorder batching can be
retained by assigning the protected offset member directly on the update_offset(false)
path.

A narrower alternative is to add old/new damage specifically around the direct XDG
wm_offset path. Centralizing the invariant in set_offset() is more robust, but it
changes the documented caller-owned damage contract and may add redundant signal
propagation where callers already emit damage themselves.