fix Firefox color rendering by respecting surface texture format #3910
fix Firefox color rendering by respecting surface texture format #3910Ayush2k02 wants to merge 5 commits intoGraphiteEditor:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a color rendering discrepancy observed in Firefox by implementing a more robust texture handling mechanism. It introduces a shader-based blitter to facilitate texture copying and on-the-fly format conversion, ensuring that the output correctly matches the surface's native format. This change enhances cross-browser compatibility and visual fidelity, particularly for users on Firefox. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses an issue with color rendering on Firefox by correctly handling different surface texture formats. The change replaces a direct texture copy with a more flexible blitter that can perform format conversions. The approach of querying the surface capabilities for a suitable format is correct. I've identified a minor code duplication issue that could be addressed to improve maintainability. Overall, this is a good fix.
| let surface_caps = surface.get_capabilities(&self.context.adapter); | ||
| let surface_format = surface_caps.formats.iter().copied().find(|f| f.is_srgb()).unwrap_or(surface_caps.formats[0]); |
There was a problem hiding this comment.
This logic to determine the surface format is also present in editor/src/node_graph_executor/runtime.rs. To avoid this duplication and improve maintainability, consider storing the determined surface_format in the Surface struct and reusing it.
You could add a format field to the Surface struct:
pub struct Surface {
pub inner: wgpu::Surface<'static>,
pub target_texture: Mutex<Option<TargetTexture>>,
pub blitter: TextureBlitter,
pub format: wgpu::TextureFormat,
}Then, you can populate it in create_surface_inner and use surface.surface.format in runtime.rs to avoid recalculating it.
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/node_graph_executor/runtime.rs">
<violation number="1" location="editor/src/node_graph_executor/runtime.rs:352">
P1: Surface format selection ignores wgpu’s preferred format ordering by forcing an sRGB match first.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
This change will degrade performance. (I recently changed it from a blitter to the direct texture copy) Can you give more details for when this workaround is needed? |
|
Oh, this is only needed for Firefox based browsers , while importing an image in Firefox, the colours are inverted |
|
can you tell me which exact test would you use here to benchmark the performance here, that way I might be able to come up with a better solution |
I previously used the wgpu inspector extension to debug this. The issue with using a blitter is, that it creates bind groups which we need to wait for javascript to garbage collect (and it is also just less efficient). So it would be could, if we could avoid it. |
9b97ab7 to
2e842cb
Compare
|
@TrueDoctor , Done, have updated the branch to check for format and then only use blitter if the formats differ |
|
Here is the updated recording for both chrome and firefox after the recent commit Screen.Recording.2026-03-24.at.8.25.55.AM.mov |
859817c to
c19c6a0
Compare
|
!build (Run ID 23978166532) |
|
I unfortunately don't have much time at the moment but if you want to test something for you (e.g. running your branch with debug statements etc.) I can do that |
|
Yeah, that would be awesome . Honestly I can do this and share with you as
well, like by the end of today , totally upto you, feel like it would be
better if I do the testing myself though
…On Sat, 4 Apr 2026 at 17:33, Dennis Kobert ***@***.***> wrote:
*TrueDoctor* left a comment (GraphiteEditor/Graphite#3910)
<#3910 (comment)>
I unfortunately don't have much time at the moment but if you want to test
something for you (e.g. running your branch with debug statements etc.) I
can do that
—
Reply to this email directly, view it on GitHub
<#3910 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AXHAYMTYKQWYVSXBLXMRU4D4UD2YFAVCNFSM6AAAAACWVW2ZWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DCOBXGAYTGMJZGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Actually, nvm . M taking a look at this right now :) . Also, thanks for sharing the webGpu inspector console, didn't know you could check that way |


Replace direct copy_texture_to_texture() with a shader-based blitter that handles format conversion between Rgba8Unorm (Chrome) and Bgra8Unorm (Firefox)
Screen.Recording.2026-03-18.at.5.48.13.AM.mov
Fixes #3880