Re: WaveShaperNode.curve

> shaper.curve = new Float32Array(100);
> for (var i = 0; i < 100; ++i) {
>   // oops, at this point the array has been internally copied,
>   // and while the code below doesn't throw an exception,
>   // it effectively sets the curve property to an all-0 array,
>   // which is not what the author has intended.
>   shaper.curve[i] = whatever;
> }


This code will bomb anyway if the array is neutered after setting the curve (the length of a
neutered array is 0). 

It is easier to tell devs that arrays be filled before setting them as a curve (also applies to other similar 
places) than to warn against doing the above because the array is neutered. Furthermore, what would be 
the point of having a getter for the shaper.curve property if all it will provide is a neutered array? 	
RoC's suggestion of a setCurve() without a getter is in this case a much cleaner solution.

In general, please consider whether any "setBlah" qualifies as a full property before
also including a getter for it. In this case, what we (devs) really need is a write-only property
to avoid all this potential confusion. A setCurve() will offer a clear point where any consistency
checks on the array can be expected to be performed.

Best,
-Kumar

On 14 May, 2013, at 3:55 AM, Ehsan Akhgari <ehsan.akhgari@gmail.com> wrote:

> Does that mean that you now agree that we should neuter it?  :-)
> 
> --
> Ehsan
> <http://ehsanakhgari.org/>
> 
> 
> On Mon, May 13, 2013 at 3:57 PM, Chris Rogers <crogers@google.com> wrote:
> 
> 
> 
> On Mon, May 13, 2013 at 12:45 PM, Ehsan Akhgari <ehsan.akhgari@gmail.com> wrote:
> The downside of that approach is that the UA will accept code which doesn't do what they intended, such as:
> 
> shaper.curve = new Float32Array(100);
> for (var i = 0; i < 100; ++i) {
>   // oops, at this point the array has been internally copied,
>   // and while the code below doesn't throw an exception,
>   // it effectively sets the curve property to an all-0 array,
>   // which is not what the author has intended.
>   shaper.curve[i] = whatever;
> }
> 
> I think this would be a terrible API.
> 
> Agreed, that's a good point.   Then lets not copy the array.
>  
> 
> For reusing the ArrayBuffer, the author can just read the curve property again and get a copy back which they can use for other purposes...
> 
> --
> Ehsan
> <http://ehsanakhgari.org/>
> 
> 
> On Sun, May 12, 2013 at 10:16 PM, Chris Rogers <crogers@google.com> wrote:
> 
> 
> 
> On Sun, May 12, 2013 at 2:20 PM, Ehsan Akhgari <ehsan.akhgari@gmail.com> wrote:
> The current WebKit implementation of this node is racy, since the processing code only protects against simultaneous setting of the curve property, not against modifying the contents of the ArrayBuffer.
> 
> In the Gecko implementation, I'm just copying the contents of the array upon setting curve for now, but I think a better fix would be to neuter the contents of the array, and provide a copy of the original contents of the array if contents reads the curve property again.
> 
> Does this make sense?
> 
> I much prefer an internal copy, and that can even be optimized as a fast pointer swap.  I don't like the idea of harming the ArrayBuffer so that it can't be used again.
>  
> 
> Thanks!
> --
> Ehsan
> <http://ehsanakhgari.org/>
> 
> 
> 
>