nan icon indicating copy to clipboard operation
nan copied to clipboard

More TypedArray helpers?

Open zbjornson opened this issue 10 years ago • 3 comments

Right now in node-canvas we've got stuff like this:

#if NODE_MAJOR_VERSION == 0 && NODE_MINOR_VERSION <= 10
    Local<Int32> sizeHandle = Nan::New(length);
    Local<Value> caargv[] = { sizeHandle };
    clampedArray = global->Get(Nan::New("Uint8ClampedArray").ToLocalChecked()).As<Function>()->NewInstance(1, caargv);
#else
    clampedArray = Uint8ClampedArray::New(ArrayBuffer::New(Isolate::GetCurrent(), length), 0, length);
#endif

and this:

#if NODE_MAJOR_VERSION == 0 && NODE_MINOR_VERSION <= 10
 if (info[0]->ToObject()->GetIndexedPropertiesExternalArrayDataType() == kExternalPixelArray) {
    clampedArray = info[0]->ToObject();
    length = clampedArray->GetIndexedPropertiesExternalArrayDataLength();
#else
 if (info[0]->IsUint8ClampedArray() && info[1]->IsUint32()) {
    clampedArray = info[0].As<Uint8ClampedArray>();
    length = clampedArray->Length();
#endif

Since TypedArrayContents was added (thank you!), wondering if there's interest in adding more helpers for TypedArrays? Maybe something like:

  • Nan::New<TypedArrayT>(Nan::New<ArrayBuffer>(byteLength[, offset[, length]]))
  • Nan::New<TypedArrayT>(length)

Not so sure on argument testing. It would be nice if NAN_METHOD unified the interface so info[x]->IsUint8Array() (edit: and all the other TypedArray types) just worked. Otherwise is the only option Nan::IsUint8ClampedArray, Nan::IsUint8Array, ... and Nan::TypedArrayLength(TypedArray) ?

zbjornson avatar Nov 19 '15 19:11 zbjornson

It would be interesting if every typed array function could be fully supported in a nice fashion, doing just a handful is less attractive.

NAN_METHOD does nothing more than define a function with an argument called info of type Nan::FunctionCallbackInfo<v8::Value>. The class could probably be extended to accommodate IsUint8Array rather easily, if that is the right thing to do.

kkoopa avatar Nov 21 '15 01:11 kkoopa

Sorry -- I meant to support all of the TypedArray types, just didn't list them explicitly.

Any syntax/approach feedback on what I proposed? I can work on a PR sometime (might be a bit -- anyone else is free to do this too), but wanted to get feedback before starting.

zbjornson avatar Nov 21 '15 01:11 zbjornson

Implement wrapper classes for each type using the interface of newer V8 and make it work consistently. Basically want the look and feel to be as close to latest V8 as possible across versions, while keeping the same observable behavior. I'd suggest planning the design before starting on the implementation, as that is the easiest way of getting something consistent and working.

On November 21, 2015 3:40:06 AM GMT+02:00, Zach Bjornson [email protected] wrote:

Sorry -- I meant to support all of the TypedArray types, just didn't list them explicitly.

Any syntax/approach feedback on what I proposed? I can work on a PR sometime (might be a bit -- anyone else is free to do this too), but wanted to get feedback before starting.


Reply to this email directly or view it on GitHub: https://github.com/nodejs/nan/issues/521#issuecomment-158573083

kkoopa avatar Nov 21 '15 11:11 kkoopa