986705 - Implement 'connect-param' event for Web Audio Editor

Status: RESOLVED FIXED

(DevTools Graveyard :: Web Audio Editor, defect)

Description

[Jordan Santell [:jsantell] [@jsantell]]

Currently supported "connect-node", which allows us to display a graph of nodes, but need to support "connect-param" when an AudioNode is connected to an AudioParam, and then subsequently display that somehow.

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: hypershade_large.jpg

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

This may require some UX oversight, as the front end needs to specify the difference visually between connecting one node to another node, and connecting the output of a node to another node's specific AudioParam.

Attached is an image of Maya's Hypergraph, which is a simular modular routing environment, where each node can have multiple input and outputs. We could have multiple arrows going into another node, and mouseover reveals which AudioParam it's connected to, and display it as a different colour to indicate it's not the main connection, and is infact to an AudioParam

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

This depends on redoing the GUI in not D3 to allow new rendering of rendering node->param connections

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Changed to just be for server-side events, with front end handled in bug 1032129

Comment 5

[Jordan Santell [:jsantell] [@jsantell]]

This could be an issue in terms of what's exposed.

Currently we trace the relevant function calls, receiving xraywrappers of the caller, method name, and arguments, like in the following example, we receive the context, method name as a string, any arguments, as well as the return value; this is how we track things

ctx.createOscillator();

When connecting to an AudioParam, we lose a lot of context:

oscillator.connect(gainNode.gain);

In this call, we'd have the oscillator node, but just an unidentified AudioParam that is not related to it's parent node in anyway, since we just get the AudioParam itself. We need the additional following:

parentID: the ChromeOnly ID of the parent node to make the reference
paramName: the property name that the param is stored on the parent

This might be able to be added during instrumentation, but could also possibly be easy on the platform side, but let's see if it's doable without bugging them first.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 986705-wae-connect-param.patch

Some work on the back end for emitting these events

Comment 7

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

Comment on attachment 8449610 [details] [diff] [review]
986705-wae-connect-param.patch

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

::: toolkit/devtools/server/actors/webaudio.js
@@ +362,5 @@
>    _handleRoutingCall: function(functionCall) {
>      let { caller, args, window, name } = functionCall.details;
>      let source = caller;
>      let dest = args[0];
> +    let isAudioParam = dest ? getConstructorName(dest) === "AudioParam" : false;

Why this change?

@@ +460,5 @@
>     */
>    _constructAudioNode: function (node) {
>      // Ensure AudioNode is wrapped.
>      node = new XPCNativeWrapper(node);
> +    

Nit: whitespace.

@@ +530,5 @@
> +    let destActor = this._getActorByNativeID(param._parentID);
> +    emit(this, "connect-param", {
> +      source: sourceActor,
> +      dest: destActor,
> +      param: param._paramName 

Nit: trailing whitespace.

Comment 8

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8449610 [details] [diff] [review]
986705-wae-connect-param.patch

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

::: toolkit/devtools/server/actors/webaudio.js
@@ +362,5 @@
>    _handleRoutingCall: function(functionCall) {
>      let { caller, args, window, name } = functionCall.details;
>      let source = caller;
>      let dest = args[0];
> +    let isAudioParam = dest ? getConstructorName(dest) === "AudioParam" : false;

For disconnect calls, `dest` will be undefined, and the `instanceof` thing would require more wrapping/unwrapping magic, so decided to just do a constructor test that we use elsewhere

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 986705-wae-connect-param.patch

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

https://tbpl.mozilla.org/?tree=Try&rev=2178d9207439

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/2f3eda663e6f

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2f3eda663e6f