Closed Bug 1032129 Opened 5 years ago Closed 5 years ago

Connections to AudioParams are not rendered

Categories

(DevTools Graveyard :: Web Audio Editor, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 34

People

(Reporter: sole, Assigned: jsantell)

References

Details

Attachments

(1 file, 1 obsolete file)

Apparently only "audio" connections between nodes are rendered in the graph, but connections to AudioParams are not, which is sad because it's the most crazy thing in Web Audio and therefore the coolest thing. It's also confusing, because you end up having nodes that are seemingly not related but they actually are.

For example this code creates 2 oscillators where the output of the second turns into an LFO via a Gain node that multiplies it. But the graph just renders them independently:
http://soledadpenades.com/files/t/lxjs2014/examples/01_oscillator/ (press "run")

I was thinking about what the best option would be here, perhaps render lines in a different colour to denote it's not audio but a parameter that is being affected by such connection.
Depends on: 986705, 994336
Bug 986705 was for this originally, but pulling that out to just be for the back end events -- some additional thoughts regarding rendering this is in that bug as well. This may require changing how all the rendering works, as it's mostly handled through d3/dagre-d3, so may require it's own solution.. and some new UX ideas.

Wishing I had this during my JSConf talk!
Well looks like we CAN render multiple connections per node: https://bugzilla.mozilla.org/attachment.cgi?id=8448301

Will see if it's easy to change the colour of these connections and add a label. I don't think this is the best solution, but should work in the meantime for lack of a beautiful solution
That is beautiful!
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
Attachment #8472563 - Flags: review?(vporof)
No longer depends on: 994336
Try looks almost good, build failures from try last few days so running it once more to check  https://tbpl.mozilla.org/?tree=Try&rev=791a9fb35bf1
Comment on attachment 8472563 [details] [diff] [review]
1032129-render-audioparam-connections.patch

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

::: browser/devtools/webaudioeditor/webaudioeditor-controller.js
@@ +77,5 @@
>  /**
>   * Track an array of audio nodes
>   */
>  let AudioNodes = [];
> +let AudioNodeConnections = new WeakMap(); // <AudioNodeView, Set<AudioNodeView>>

Ugh. What is this, c#?
Attachment #8472563 - Flags: review?(vporof) → review+
\m/
Attachment #8472563 - Attachment is obsolete: true
Attachment #8474793 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/1d876f502f1e
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/1d876f502f1e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 34
I've added a note about this in the MDN page: https://developer.mozilla.org/en-US/docs/tools/Web_Audio_Editor#Connections_to_AudioParams.

Let me know if it works for you!
Flags: needinfo?(jsantell)
Looks great -- thanks Will!
Flags: needinfo?(jsantell)
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.