Re: WaveShaperNode.curve

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/>
>>>>
>>>
>>>
>>
>