refactor(engine)!: GPU pass fusion stack 5/5 — shader migration - #2171
refactor(engine)!: GPU pass fusion stack 5/5 — shader migration#2171yuto-trd wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (29): Last reviewed commit: "refactor(engine)!: stack 5/5 — migrate b..." | Re-trigger Greptile |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2302ae19e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a2302ae to
9163623
Compare
9163623 to
2200c61
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2200c61c34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2200c61 to
cf41190
Compare
cf41190 to
8738e28
Compare
8738e28 to
9c9f4af
Compare
9c9f4af to
2228754
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2228754db5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2228754 to
f08d99e
Compare
f08d99e to
13fad9d
Compare
Code Review BotNo comment/code divergences or documentation drift detected. Reviewed 38 file(s); skipped 6. |
13fad9d to
5260b9e
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
5260b9e to
1d84d47
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
cf164a4 to
a25fe7a
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
a25fe7a to
ea1d94a
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9885d95796
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db38a972c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 970dbcf4d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1116b63574
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
7dd5c0c to
36f8e97
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No TODO comments were found. |
36f8e97 to
a795101
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No TODO comments were found. |
Nine pixel-wise color effects, ColorShift, Mosaic, and the displacement-map transforms migrate to the SKSL/GLSL shader pipeline with explicit target ownership; the SKSLShaderBuilder ownership change and LUT snapshot/reuse (ConditionalWeakTable under the ownership gate) are included. Blur and DropShadow stay on the legacy path. The ColorShift snapshot-intent tests are dropped with the removed readback-failure surface; the FrameProviderImpl retention-heuristic test stays excluded with its separately shipped change.
Minimum allowed line rate is |
a795101 to
72417e0
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No TODO comments were found. |
|
Superseded by #2221, which merges every layer of this stack into one branch, propagates the s2 reduction through layers 3–5 (this stack no longer built above s2), and drops the 61.5 MB evidence archive. |
Description
Stacked-PR slice 5/5 of
speckit/004-gpu-pass-fusion.This final slice moves the non-Gaussian Shader migration into its own review boundary:
CurrentPixeldescriptions using cached parsedSkslSourceinstances;WholeSourcedescriptions;SKSLShaderBuilderowns per-build state and exposes caller-ownedBuild()results without leaking the disposable runtime effect;Blur.csandDropShadow.csremain outside this migration slice. The final S1-S4 review changes and immutable evidence archive are propagated through ordinary signed merge commits.Stack
speckit/004-s1-spec— specification and contractsspeckit/004-s2-engine— record-then-plan engine and consumer migrationspeckit/004-s3-tests— pipeline and rendering test suitesspeckit/004-s4-evidence— benchmarks, paired evidence, and acceptanceAffected areas
Beutl.Engine(rendering / scene / track)Beutl.ProjectSystem(project / document persistence)Beutl.Editor,Beutl.Editor.Components,Beutl.Controls)Beutl.Extensibility(plugin abstractions)Beutl.NodeGraph(node editor)Beutl.FFmpegIpc/Beutl.FFmpegWorker(media IPC boundary)Beutl.Api(server API client)Breaking changes
SKSLShader.ApplyToNewTargetandSKSLShader.Effectare removed. Allocate withCustomFilterEffectContext.CreateTargetLikeorCreateTarget, map inputs withUseMappedInputShader, configure metadata throughSKSLShaderBuilder.UniformsandChildren, and render withSKSLShader.RenderToTarget. The caller owns eachSKShaderreturned bySKSLShaderBuilder.Build().The lower stack also contains the documented
Beutl.Enginerender-authoring migration. Downstream plugins must use named resource bindings, immutable callback state,PaintedRenderCanvas,IRenderTargetFactory.MaximumDimension, and an explicitRenderAllocationBudgetas described in the S1/S2 breaking-change contract.Review follow-up (2026-08-09)
SkslSourceobjects for all migrated current-pixel built-ins, including separate LUT 1D and 3D sources.All five unresolved inline review threads were answered and resolved.
Test plan
Beutl.PublicApiContractTests: 195/195 passed.SourceGeneratorTest: 30/30 passed.Beutl.HeadlessUITests: 250/250 passed.Beutl.Graphics3DTests: 14/14 passed with no skips.Full UnitTests was green before the final ownership-test addition: 6,486 passed, 3 skipped, 0 failed. On the final tree, two full runs each produced 6,487 passed, 3 skipped, and one failure in the unrelated
ProxyJobQueueTests.Cancel_ParkedUnavailableJob_WakesDrainLoopWithoutWaitingBackofftest. The isolated case passed, while repeated fixture execution reproduced the pre-existing lost-cancel race on run 25. Proxy queue production and tests are byte-identical tomainand S4 and were intentionally not changed in this GPU pass-fusion PR.Evidence status
overallAcceptancePassed=false; T123 remains pending.Fixed issues / References
Review follow-up (2026-08-10)
Validation at final HEAD:
Two final full UnitTests runs each produced 6,498 passed, 3 skipped, and one failure in the unchanged main-identical
ProxyJobQueueTests.Cancel_ParkedUnavailableJob_WakesDrainLoopWithoutWaitingBackoffrace; the isolated test passed 1/1. No Proxy code or tests were changed. The final historical evidence manifest isf10ab3ba6f03f36621c9e4254d7cb9467481fce301245b75434855e75b2c9303; formal performance gates remain false and T123 remains pending.