Skip to content

fix(region-selector): render first frame synchronously to avoid frame-callback deadlock - #220

Merged
hthienloc merged 2 commits into
hthienloc:mainfrom
corrm:fix/region-selector-first-frame
Aug 18, 2026
Merged

fix(region-selector): render first frame synchronously to avoid frame-callback deadlock#220
hthienloc merged 2 commits into
hthienloc:mainfrom
corrm:fix/region-selector-first-frame

Conversation

@corrm

@corrm corrm commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Render the region selector's first frame synchronously instead of waiting for a wl_surface.frame callback, fixing a deadlock that leaves the selection overlay invisible (and input-less) on Hyprland 0.56 and similar compositors.

Use Case

Region/scroll capture is completely unusable on affected compositors: the overlay never appears, and after the plugin's 60s timeout the user gets a misleading "Rust screenshot backend is not installed" toast even though the backend is installed and working. See #219.

Changes

  • dms-screenshot-rs/src/region_selector/backend.rs:
    • request_render(): if the output has not committed its initial buffer yet, call render_output() directly instead of waiting for a frame callback (which some compositors never fire for buffer-less surfaces).
    • Track this with a new initial_render_done flag per output, set after the first successful commit.
    • All subsequent redraws still go through the existing frame-callback path, unchanged.

Related Issues

Closes #219

Testing

  • Tested on Hyprland 0.56.2, x86_64, dual 1920x1080 monitors, DMS 1.5.3, plugin 5.0.0-beta.3:
    • Before: added trace logging shows zwlr_layer_surface_v1.configure arriving for both outputs, but render_output() never runs — no buffer committed, hyprctl layers shows the selection layers at alpha 0, capture killed at 60s (exitCode=124), "backend not installed" toast.
    • After: buffers commit immediately after configure, overlay appears, a drawn region completes with {"status":"success",...} and opens the annotation modal.
  • full mode unaffected (still works as before).
  • Plugin loads without errors; rebuilt binary installed to backend/<arch>/ per the README flow.

Screenshots / Screen Recordings

Not available (Wayland screen content from the affected overlay was, by definition, not rendered).

Checklist

  • I have tested these changes on my setup
  • I have updated the documentation (README, IPC/settings docs) if needed
  • I have verified the plugin loads without errors

Summary by Sourcery

Ensure the region selector displays and accepts input reliably on compositors that do not issue frame callbacks before the first buffer commit.

Bug Fixes:

  • Render the region selector's initial frame synchronously when needed, preventing compositor frame-callback deadlocks that leave the overlay invisible and unusable.

Enhancements:

  • Preserve the existing frame-callback rendering path for subsequent redraws while tracking whether each output has completed its initial render.

…-callback deadlock

Some compositors (e.g. Hyprland >= 0.56) never fire wl_surface.frame
callbacks for a layer surface that has not committed a buffer yet.

request_render() only drew a frame after such a callback, so the
selector deadlocked before its first frame: the overlay never became
visible, input never arrived, and the plugin killed the process after
its 60s timeout (surfacing as a misleading "backend not installed"
toast).

Draw the first frame for each output synchronously; subsequent redraws
still go through frame callbacks as before.
@sourcery-ai

sourcery-ai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Reviewer's Guide

Renders the region selector’s first frame synchronously instead of waiting for a wl_surface.frame callback, by tracking an initial-render flag per output and short‑circuiting the frame-callback path to directly invoke render_output when needed, preventing a deadlock on compositors that never send frame events for buffer-less layer surfaces.

Sequence diagram for synchronous first-frame render in region selector

sequenceDiagram
    participant Compositor
    participant SelectorState
    participant OutputEntry

    Compositor->>SelectorState: zwlr_layer_surface_v1.configure
    SelectorState->>SelectorState: request_render(output_idx)
    SelectorState->>OutputEntry: mark dirty
    SelectorState->>OutputEntry: check initial_render_done
    alt initial_render_done is false and surface exists and no frame_callback
        SelectorState->>SelectorState: render_output(state, qh, output_idx)
        SelectorState->>Compositor: wl_surface.commit
        SelectorState->>OutputEntry: set initial_render_done = true
    else initial_render_done is true or no surface or has frame_callback
        SelectorState-->>OutputEntry: wait for wl_surface.frame callback
    end

    rect rgb(230,230,230)
    Compositor-->>SelectorState: wl_surface.frame callback
    SelectorState->>SelectorState: request_render(output_idx)
    SelectorState->>OutputEntry: reuse frame_callback path
    SelectorState->>SelectorState: render_output(state, qh, output_idx)
    SelectorState->>Compositor: wl_surface.commit
    end
Loading

File-Level Changes

Change Details Files
Add per-output tracking of whether the initial frame has been rendered.
  • Extend OutputEntry with an initial_render_done boolean flag.
  • Set initial_render_done to true after a successful commit in render_output so future renders know a buffer has been committed.
dms-screenshot-rs/src/region_selector/backend.rs
Render the first frame synchronously when no buffer has ever been committed, instead of relying on wl_surface.frame callbacks.
  • Refactor request_render to first read the output immutably to compute has_frame_callback, needs_direct_render, and has_surface without taking &mut prematurely.
  • Short-circuit request_render when there is an outstanding frame callback or no surface, leaving the existing deferred-render path unchanged.
  • Invoke render_output directly when needs_direct_render is true and the surface exists, so the first frame is drawn and committed immediately.
  • Reacquire a mutable reference to the output only when needed after the decision logic to mark it dirty or fetch the surface handle.
dms-screenshot-rs/src/region_selector/backend.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#219 Ensure the region selector renders its first frame synchronously instead of waiting for a wl_surface.frame callback, so that on compositors like Hyprland 0.56 the overlay appears and capture can proceed without deadlocking.
#219 Eliminate the 60s hang and resulting misleading 'Rust screenshot backend is not installed' error when performing a region capture on affected compositors.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In request_render, consider restructuring to avoid the multiple state.outputs.get/get_mut calls per invocation; a single mutable borrow captured up front would simplify the logic and reduce the chance of subtle borrowing or state bugs.
  • The new initial_render_done flag is only set in render_output; confirm it is reset or reinitialized appropriately when an output or surface is recreated so that a replaced surface still forces a synchronous first frame.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `request_render`, consider restructuring to avoid the multiple `state.outputs.get/get_mut` calls per invocation; a single mutable borrow captured up front would simplify the logic and reduce the chance of subtle borrowing or state bugs.
- The new `initial_render_done` flag is only set in `render_output`; confirm it is reset or reinitialized appropriately when an output or surface is recreated so that a replaced surface still forces a synchronous first frame.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

…g on new surface

Address review feedback:

- request_render now uses a single scoped mutable borrow for the
  decision (configured/closed/dirty/frame-callback/first-frame) and
  only re-borrows to register a frame callback on the deferred path.

- Reset initial_render_done when a layer surface is created in
  setup_layer_surfaces. Surfaces are in practice only created once per
  run, so this is defensive, but it makes the invariant explicit: a
  fresh surface always forces a synchronous first frame.
@corrm

corrm commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

I investigated both points against the code before changing anything:

1. request_render borrow structure
The literal suggestion of holding a single mutable borrow across the whole call doesn't compile — render_output needs &mut state, so any live borrow of state.outputs must end first. But the intent is right, so I restructured: the decision (configured/closed/dirty/outstanding-frame-callback/first-frame) now happens in one scoped mutable borrow, and the deferred path only re-borrows to register the frame callback. No more triple get/get_mut dance (0d4e6b1).

2. initial_render_done when surfaces are recreated
Verified: setup_layer_surfaces is the only place output.surface is assigned, and it runs exactly once per process run, before the event loop. The Closed event only marks the entry unusable (closed = true, configured = false) — both render paths skip such entries and nothing re-creates a surface afterwards. So the flag cannot go stale today. Still, to make the invariant explicit and future-proof the code, initial_render_done is now reset at the surface-creation site, so any new surface will always force a synchronous first frame (0d4e6b1).

Both changes are behavior-preserving; rebuilt and re-tested (region capture works end-to-end on the affected setup, no new warnings).

@hthienloc

Copy link
Copy Markdown
Owner

Thanks for the PR. I don't use Hyprland, so having someone like you submit a fix is really helpful.

If you find any other bugs related to this backend on Hyprland, feel free to open another issue, preferably with some screenshots or a video so I can better understand the issue.

@hthienloc
hthienloc merged commit 376398b into hthienloc:main Aug 18, 2026
1 check passed
@corrm

corrm commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the PR. I don't use Hyprland, so having someone like you submit a fix is really helpful.

If you find any other bugs related to this backend on Hyprland, feel free to open another issue, preferably with some screenshots or a video so I can better understand the issue.

I really like the plugin so i thought it worth the fix, thanks for taking time working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Region selector invisible on Hyprland 0.56 — capture times out after 60s with 'backend not installed' error

2 participants