Skip to content

feat(effects): check an effect's code off the main thread, and price the tiers on an M1 - #1220

Merged
EtienneLescot merged 1 commit into
integration/shader-effectsfrom
feat/shader-effects-async-check
Oct 14, 2026
Merged

EtienneLescot merged 1 commit into
integration/shader-effectsfrom
feat/shader-effects-async-check

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Summary

A Metal compile of a new effect takes 120 ms on an M1, up to 375 ms for the first in a process (#1206), against 10 ms for FXC. checkShaderEffect ran it synchronously, on the main process's thread, at every addEffect and setEffectDefinition that writes code.

Targets integration/shader-effects, rebased on it after #1206 merged (whose spec section H.3 this updates). The review's "a throwing addon reads as not checked" carries over: the service now awaits the promise inside its try, so a rejection lands there too.

  • The addon. checkShaderEffect is an AsyncTask on the libuv pool, like segmentFrame. CompositorViewService.checkShaderEffect and the ShaderEffectChecker type return a promise.
  • The tools. runDocumentTool asks first, for the code and parameter layout the call will store (shaderEffectToCheck), then hands the answer to the executor (shaderEffectCheckLookup), which stays synchronous. The cursor read and the auto-zoom plan already work that way. With edits off, nothing is asked.
  • The tiers in time. addEffect's description and the MCP contract resource give each tier's cost per 1080p frame on the RTX 4070 Ti and on the M1, as the spec's C.7 asks.
  • Docs: spec D.4 and H.3, rendering-performance.md.

Related issue

Part of the shader effects work (#1199, #1202, #1206).

Type of change

  • Performance
  • Enhancement
  • Documentation

Release impact

  • No release note needed: shader effects are not reachable from the app yet

Desktop impact

  • Not platform-specific (the compile it moves off the main thread is FXC, Metal or SPIR-V depending on the machine)

Testing

  • npx tsc --noEmit and npx tsc -p tsconfig.test.json --noEmit pass.
  • npx vitest --run electron/ai-edition electron/mcp electron/native-bridge src/lib/ai-edition src/native workbench: 2224 tests pass. The new ones cover:
    • runDocumentTool waiting for a slow checker before accepting or refusing;
    • setEffectDefinition checking new code against the definition's parameters;
    • nothing asked with edits off or for a tool that compiles nothing;
    • the lookup answering only for the same code and layout;
    • the service passing the addon's answer through, and reading an addon that throws or rejects as not checked (the rejection case fails without the await).
  • npm run lint: no errors. cargo build -p compositor-view-napi builds on macOS.
  • Checked through the addon on the M1: the two MCP examples compile on Metal in 245 ms and 127 ms.
  • Not run: the full npm run test, and the in-app agent in the GUI.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 10a7cfeb-5b97-4755-9f77-8a075687aedd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · 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.

@EtienneLescot EtienneLescot mentioned this pull request Oct 14, 2026
3 tasks done
@EtienneLescot
EtienneLescot force-pushed the feat/shader-effects-macos branch from 80c6dde to e16a014 Compare October 14, 2026 16:23
@EtienneLescot
EtienneLescot changed the base branch from feat/shader-effects-macos to integration/shader-effects October 14, 2026 17:46
…the tiers on an M1

A Metal compile takes 120 ms for a new source on an M1, up to 375 ms for
the first in a process (#1206), against 10 ms for FXC. checkShaderEffect
ran it on the main process's thread.

- The addon's checkShaderEffect is an AsyncTask on the libuv pool, like
  segmentFrame. The service and the checker type return a promise; the
  service awaits it inside its try, so a rejecting addon still reads as
  "not checked", as a throwing one did.
- runDocumentTool asks first, for the code addEffect or
  setEffectDefinition will store (shaderEffectToCheck), then hands the
  answer to the executor, which stays synchronous, as the cursor read and
  the auto-zoom plan already do. Edits off: nothing is asked.
- addEffect's description and the MCP contract resource give each tier
  in ms on the RTX 4070 Ti and on the M1, as the spec's C.7 asks.
@EtienneLescot
EtienneLescot marked this pull request as ready for review October 14, 2026 17:51
@EtienneLescot
EtienneLescot force-pushed the feat/shader-effects-async-check branch from 5a564fc to 401157e Compare October 14, 2026 17:51
@EtienneLescot
EtienneLescot merged commit a496aae into integration/shader-effects Oct 14, 2026
17 checks passed
@EtienneLescot
EtienneLescot deleted the feat/shader-effects-async-check branch October 14, 2026 17:53
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.

1 participant