Translation-node: changing an offset can leave stale pixels
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 3.1k
- Forks
- 237
- Avg merge
- 1h 43m
- Merged PRs (30d)
- 1
Description
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with translation_node_t::set_offset(), then trace xdg_toplevel_t::handle_surface_commit() and xdg_toplevel_view_t::handle_toplevel_state_changed() to understand the geometry update path. Build and run the provided offsetdemo reproducer, and verify that alternating the window-geometry origin repaints both the newly covered and vacated regions without unrelated client damage.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- computer-graphics
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100