Skip to content

Require prepared resources for child frame rendering - #495

Merged
eval-exec merged 1 commit into
mainfrom
refactor/prepared-child-render
Oct 7, 2026
Merged

eval-exec merged 1 commit into
mainfrom
refactor/prepared-child-render

Conversation

@eval-exec

Copy link
Copy Markdown
Owner

Child rendering currently accepts optional composition scratch and panics if a translucent or resizing child reaches the draw without the required leases. Require a PreparedChildFrame instead: its private constructor checks required leases, scratch dimensions, and resize aliases while borrowing the child payload and resources. Rendering consumes the prepared value.

Propagate preparation failures through full and retained render paths so the runtime restores renderer state, keeps the frame dirty, and retries without presentation. Restore root font bindings even when a later child fails preparation. This uses ordinary Rust types and adds no dependencies.

Validation:

  • Runtime render-pass suite: 39 passed, including missing-scratch retry/recovery, resize composition, and budget admission.
  • Offscreen opacity suite: 14 passed, including missing, mismatched, and aliased scratch checks.
  • Compile-fail examples: 2 passed, covering payload borrowing and preparation consumption.
  • Renderer unit suite: 748 passed; 3 existing image-diagnostic tests failed because tmp/imgmsg/fixtures files are absent from this checkout.
  • Formatting and git diff --check passed.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 16:11
@coderabbitai

coderabbitai Bot commented Oct 7, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: aaef1b41-0d5a-4f02-811b-4ab860e1a89d
📥 Commits

Reviewing files that changed from the base of the PR and between 66f3455 and cd4f1a6.

📒 Files selected for processing (13)
  • crates/neomacs-display-runtime/src/render_thread/render_pass/chrome/mod.rs
  • crates/neomacs-display-runtime/src/render_thread/render_pass/full_render.rs
  • crates/neomacs-display-runtime/src/render_thread/render_pass/mod.rs
  • crates/neomacs-display-runtime/src/render_thread/render_pass/retained_static.rs
  • crates/neomacs-display-runtime/src/render_thread/render_pass/scene.rs
  • crates/neomacs-display-runtime/src/render_thread/render_pass/scene/native_boundary_tests.rs
  • crates/neomacs-display-runtime/src/render_thread/render_pass/scene/pane_size_boundary_tests.rs
  • crates/neomacs-display-runtime/src/render_thread/render_pass/scene/retained_owner_tests.rs
  • crates/neomacs-display-runtime/src/render_thread/render_pass/scene/tests.rs
  • crates/neomacs-renderer-wgpu/src/renderer/child_frames.rs
  • crates/neomacs-renderer-wgpu/src/renderer/child_frames/tests/child_frames_test.rs
  • crates/neomacs-renderer-wgpu/src/renderer/mod.rs
  • crates/neomacs-renderer-wgpu/tests/offscreen_frame/opacity_test.rs
 ______________________________________________
< This is O(n) in theory and O(🤡) in practice. >
 ----------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@eval-exec
eval-exec merged commit 853bcd6 into main Oct 7, 2026
20 of 34 checks passed

Copilot AI 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.

🔵 Needs a closer look

It reworks error-recovery control flow across many files in the GPU render-thread hot path, and its correctness depends on runtime/GPU behavior that cannot be fully verified here.

0 open findings

What changed in this PR

This PR hardens the wgpu child-frame rendering path. Previously, render_child_frame_prepared accepted optional composition scratch leases and panicked (via .expect) if a translucent or resizing child reached the draw without its required leases. The PR replaces that with a PreparedChildFrame proof type whose private fields force construction through a validating new constructor that checks lease presence, scratch dimensions, and resize-attachment aliasing. Preparation failures now propagate as FrameRenderFailure through the full and retained render paths so the runtime restores renderer scale/size, keeps the frame dirty, and retries without presenting—replacing a hard panic with graceful recovery. This fits into the render-thread's existing FrameRenderFailure retry model and adds no new dependencies.

Changes:

  • Introduce PreparedChildFrame (validated, consumed by draw) and ChildPreparationError; drop the preflighted-scratch field from ChildResizePicture.
  • Thread Result<(), FrameRenderFailure> through scene → chrome → full_render/retained_static → render_pass/mod, restoring renderer state and marking dirty on failure.
  • Move root font-binding restoration so it runs even when a later child fails preparation; add regression tests for missing/wrong-size/aliased scratch and retry-recovery.
File Description
renderer/​child_frames.rs Adds PreparedChildFrame/ChildComposition/ChildPreparationError, the validating new, and refactors render_child_frame_prepared to consume the prepared value.
renderer/​mod.rs Exports the new ChildPreparationError and PreparedChildFrame types.
render_pass/​scene.rs Builds the prepared child, propagates preparation failure as WindowNotReady, and relocates root font restoration ahead of the error return.
render_pass/​chrome/​mod.rs, full_render.rs, retained_static.rs, mod.rs Return/propagate FrameRenderFailure; restore scale/size and mark dirty on failure.
renderer/​.../​child_frames_test.rs, tests/​offscreen_frame/​opacity_test.rs Unit coverage for opaque/fractional/wrong-size/aliased preparation outcomes.
render_pass/​scene/​tests.rs + boundary/​owner test files Update call sites to .unwrap() the new Result; add missing-scratch retry/recovery regression.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

2 participants