Repository navigation
Fix ContrastAdjustment and CrossHatch with multisampled input - #3062
Conversation
With FPP multisampling enabled, these filters declare a regular 2D sampler for a multisample texture. On our desktop GL reproduction, ContrastAdjustment renders black and CrossHatch loses the scene. Use the shared RESOLVE_MS define in both materials and sample contrast through getColor(), as the other multisample-aware filters do. Leave the existing GLES resolve fallback unchanged. Check compiled sampler types and compare single-sample/MSAA framebuffer output for both filters. The native regression fails on upstream and passes on desktop OpenGL and ANGLE with this patch.
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
jaime-jmebot
left a comment
jaime-jmebot
left a comment
There was a problem hiding this comment.
Hi, thanks for putting this fix together. The changes address the two filters’ multisample mismatch cleanly: both materials now enable the shared resolve path, and ContrastAdjustment samples through getColor() just like the existing multisample-aware filters. That keeps single-sample behavior on the regular sampler while selecting the multisample sampler when needed, without changing the GLES fallback.
I also reviewed the regression test, which checks both the compiled sampler type and rendered output with single-sample and multisampled filter input. The CI runs are green, including the OpenGL and ANGLE screenshot-test jobs. This looks like a focused fix with useful coverage for the reported failure mode. Thanks for tackling it!
|
Thanks! |
Enable FPP multisampling with
filters.setNumSamples(4)and add either ContrastAdjustmentFilter or CrossHatchFilter. On desktop OpenGL, FPP supplies a multisample texture, but these two materials declare an ordinary 2D sampler. ContrastAdjustment also bypasses the shared multisample sampling helper.On the tested NVIDIA driver, ContrastAdjustment renders black and CrossHatch shows only its pattern, losing the scene. Incorrect sampler/texture combinations have undefined output, so other drivers can look different and may be fine.
Use RESOLVE_MS in both materials and getColor() in the contrast shader, matching the existing multisample-aware filters. The existing GLES resolve fallback is unchanged.
The native regression checks compiled sampler types and single-sample/MSAA framebuffer output for both filters. It fails on upstream and passes on OpenGL and ANGLE.