Closed Bug 938549 Opened 7 years ago Closed 6 years ago

Recompiling shaders when highlighting is very heavy and loses cached state

Categories

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

defect

Tracking

(firefox26 unaffected, firefox27 fixed, firefox28 fixed)

RESOLVED FIXED
Firefox 28
Tracking Status
firefox26 --- unaffected
firefox27 --- fixed
firefox28 --- fixed

People

(Reporter: vporof, Assigned: vporof)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

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.
Priority: -- → P2
Attached patch webgl-highlight.patch (obsolete) — Splinter Review
This does the trick. Uses blending hacks to achieve the same effects without recompiling fragment shaders.
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Try is green. Changes are straightforward.
Attachment #833428 - Attachment is obsolete: true
Attachment #8333728 - Flags: review?(rcampbell)
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?
Attachment #8333728 - Flags: review?(rcampbell) → review+
Depends on: 937627
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
Attachment #8333728 - Flags: approval-mozilla-aurora?
Blocks: 943596
https://hg.mozilla.org/mozilla-central/rev/0abfe7ea3533
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
Attachment #8333728 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qa-]
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.