Re: WaveShaperNode.curve

On Tue, May 14, 2013 at 1:57 AM, Srikumar Karaikudi Subramanian <
srikumarks@gmail.com> wrote:

>
> On 14 May, 2013, at 9:07 AM, "Robert O'Callahan" <robert@ocallahan.org>
> wrote:
>
> On Tue, May 14, 2013 at 2:58 PM, Srikumar Karaikudi Subramanian <
> srikumarks@gmail.com> wrote:
>
>>  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).
>>
>
> That's a good thing. Code that fails obviously and synchronously is better
> than code that silently fails to work, from the point of view of an author
> trying to find bugs.
>
>
> Agreed in general. What I was trying to point out was that if a dev would
> expect this code to modify the curve used by the shaper, neutering the
> array won't serve that purpose either.
>

The goal here is to _not_ allow modifying the array under WaveShaperNode's
nose.


> The bigger inconvenience of neutering, as I've said before, is that a dev
> cannot now use the same shape curve array with two wave shaper nodes. If
> the system is to somehow sneak in behind the neutered array and get the
> contents, then there really needs to be a whole section in the spec on how
> objects are to respond to array properties when neutered arrays are passed
> 'cos it is all non-obvious. As a dev, I worry more about the following not
> working -
>
> var curve = new Float32Array(N);
> // ... fill up curve with data...
> shaper1.curve = shaper2.curve = curve;
>

If we make the getter return a new JS object pointing to an array with a
copy of the data passed to the setter, then this code will work fine.


>
>> 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.
>>
>
> We could arrange for the getter of 'curve' to return a new non-neutered
> array, with the old contents, if the original array has been neutered.
>
>
> Possible solution, but this makes a copy anyway and raises further
> questions.
>

Sure, but there's no way to fix this situation by avoiding to copy in all
cases.  It's better to copy in the non-common case.


> If the first code is expected to work, when should the shaper node check
> that the contents of this new array has changed and update itself?
>

There's not a good answer for that, except that, it shouldn't have to.  :-)


>
>> 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.
>>
>
> Sounds good to me :-). Maybe we could add setCurve(), deprecate the
> attribute, make setting the attribute neuter the array, and make getting
> the attribute return a new array with the latest curve data.
>
>
> This is close to best, but since compatibility is of interest here, the
> deprecated attribute shouldn't be spec'd to neuter because it'll break code
> that shares an array between shaper nodes. setCurve() should then be spec'd
> such that modifying the array after setCurve() is called should not affect
> the shaper node.
>

I don't think that's a good argument.  I think spec'ing neutering on set
and copying on get is the best we can do compatibility wise.  I don't
expect that to break a lot of existing content, and we should not be
interested in the theoretical breakages.


> This situation is analogous to WebGL where a texture is created from an
> array of pixels. The pixel data is copied and the array with the pixel data
> is free to be modified with no expectation of immediately affecting the
> texture. The advantage of an internal copy is that the copy can use a more
> efficient representation if it so desires - for example, by supersampling
> the array using polynomial interpolation, for higher quality if that cost
> cannot be paid in the inner processing loop.
>
> To sum up the points on this -
> a) there is loss of functionality in neutering array attributes after
> they're set,
> b) what a node should do with neutered array data is non-obvious and needs
> separate specification if neutered arrays behave differently from
> zero-length arrays,
>  c) there are potential efficiency advantages to a node keeping internal
> copies, and
> d) internal copies are more friendly to implementations that might sandbox
> the audio code in a separate process (like Chrome does with WebGL).
>
> On a wider note, the only case I can find for neutering is for Blob
> objects after close() is called on them (
> http://dev.w3.org/2006/webapi/FileAPI/). That makes sense and I wouldn't
> expect to use a Blob after close()ing it. Any other cases that might serve
> as guides for when to neuter and when not to?
>

Note that I'm not suggesting neutering as a good way to design the API, I'm
suggesting that as the best compromise to fix the API that is already
broken, because overhauling the API without changing WebKit/Blink is not
practical.

--
Ehsan
<http://ehsanakhgari.org/>