938549 - Recompiling shaders when highlighting is very heavy and loses cached state

Status: RESOLVED FIXED

(DevTools Graveyard :: WebGL Shader Editor, defect, P2)

Description

[Victor Porof [:vporof][:vp]]

To highlight rendered geometry in red when hovering a program, the responsible fragment shaders are replaced with a simple draw-all-things-in-plain-red alternative.

This is very bulky, awkward and can slow down rendering when compiling the new/old sources. Terrible, considering the fact that it happens when the mouse moves.

Moreover, because the simplified draw-all-things-in-plain-red fragment shader doesn't contain any uniforms, the client will lose any cached state, which is unrecovered when recompiling back the old source ("unhighlighting").

This is especially obvious with the unreal engine demo. After highighting the first ten shaders, you'll notice that almost all textures look different, the prebaked lighting is lost etc.

The fix would be to go through a completely different route for highlighting. Compiling things is costly, but changing blending parameters is pretty much free.

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: webgl-highlight.patch

This does the trick. Uses blending hacks to achieve the same effects without recompiling fragment shaders.

Comment 2

[Victor Porof [:vporof][:vp]]

Attachment: webgl-highlight.patch

Try is green. Changes are straightforward.

Comment 3

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 8333728 [details] [diff] [review]
webgl-highlight.patch

Review of attachment 8333728 [details] [diff] [review]:
-----------------------------------------------------------------

sure!

::: browser/devtools/shadereditor/test/browser_se_programs-blackbox-01.js
@@ +137,2 @@
>    yield ensurePixelIs(debuggee, { x: 127, y: 127 }, { r: 255, g: 255, b: 0, a: 255 }, true, "#canvas1");
> +  yield ensurePixelIs(debuggee, { x: 127, y: 127 }, { r: 0, g: 0, b: 64, a: 255 }, true, "#canvas2");

matching highlight pixel colors?

Comment 4

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/0abfe7ea3533

Comment 5

[Victor Porof [:vporof][:vp]]

Comment on attachment 8333728 [details] [diff] [review]
webgl-highlight.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New feature (bug 910953)
User impact if declined: Using this tool in a certain way might break some pages with WebGL content. The Shader Editor is a novelty tool and no other browser has it, so WebGL devs might heavily depend on it. It'd be nice if their pages didn't start acting weirdly during development. This patch fixes this issue, and depends on bug 937627.
Testing completed (on m-c, etc.): locally, fx-team
Risk to taking this patch (and alternatives if risky): None
String or IDL/UUID changes made by this patch: None

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0abfe7ea3533

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/56f4f6a8d4d9