Closed Bug 533663 Opened 14 years ago Closed 14 years ago

Use JS WebGL classes in WebGL


(Core :: Graphics: CanvasWebGL, defect)

Not set





(Reporter: vlad, Assigned: vlad)




(3 files, 3 obsolete files)

Attached patch work-in-progress, v0 (obsolete) — Splinter Review
bug 532774 implements the WebGL Array classes in pure JS; we need to pass these down to WebGL in some useful form.

We take the approach of writing custom quickstub functions, and teach qsgen about methods that have entirely custom quickstubs provided via #include.  (Or maybe it should just generate those as extern? Hmm.)  Via this mechanism we can support overloaded method names as well, where a dummy quickstubbed method in the IDL calls out to N real methods; e.g. "bufferData" is declared as a dummy "void bufferData([optional] in long dummy);" function, along with three bufferData_size, bufferData_buf, bufferData_array variants, which are [noscript].  A custom quickstub is written for the dummy bufferData, which calls one of _size, _buf, or _array depending on the input types.

This also greatly simplifies the webgl side of things.
Attached patch patch, v1 (obsolete) — Splinter Review
This works, and seems to run the webgl content that I've tried.  No benchmarking yet.
Assignee: nobody → vladimir
Attachment #416720 - Attachment is obsolete: true
Attached patch v2 (obsolete) — Splinter Review
Ready to go.

mrbkap, can you take a look at the xpconnect code in src/nsXPConnect.cpp, where I set up the type aliases?  Is that the right place to do this?
Attachment #420204 - Attachment is obsolete: true
Attachment #421667 - Flags: review?(mrbkap)
Attached patch updated, v3Splinter Review
Updated, sorry for churn.. folded a few other patches in.

mrbkap, xpconnect stuff didn't change here

Jeff, can you give the other stuff a quick once over?  It's almost all WebGL-contained code, except for stripping out the old WebGL*Array classes from DOM stuff.
Attachment #421667 - Attachment is obsolete: true
Attachment #421690 - Flags: review?(mrbkap)
Attachment #421690 - Flags: review?(jmuizelaar)
Attachment #421667 - Flags: review?(mrbkap)
Comment on attachment 421690 [details] [diff] [review]
updated, v3

I'm a bit out of my depth here, but everything seemed sane from what I could tell.
Attachment #421690 - Flags: review?(jmuizelaar) → review+
Attached patch updated, v4Splinter Review
v4 for mrbkap, for xpconnect stuff
Attachment #421903 - Flags: review?(mrbkap)
Comment on attachment 421903 [details] [diff] [review]
updated, v4

r=me with "if (" changed to "if(" (welcome to XPConnect!).
Attachment #421903 - Flags: review?(mrbkap) → review+
Attached patch v5, js changesSplinter Review
Split out js changes; main change is the removal of JSFUN_GENERIC_NATIVE, so the xpconnect code doesn't go nuts in security checks.

Everything else is the same as in v4.
Attachment #422150 - Flags: review?(brendan)
Comment on attachment 422150 [details] [diff] [review]
v5, js changes

> js_InitTypedArrayClasses(JSContext *cx, JSObject *obj)
> {
>-    JSObject *proto;
>+    JSObject *proto, *stop;

Nit: could split these in C++ style and move down to before first set or init.

Uber-nit, or note: the "stop" name in jsiter.cpp means StopIteration singleton, here it means "stop recursively over-initializing", or something. Still kinda works4me as a name ;-).

>+    /* Idempotency required: we initialize several things, possibly lazily. */
>+    if (!js_GetClassObject(cx, obj, JSProto_ArrayBuffer, &stop))
>+        return NULL;
>+    if (stop)
>+        return stop;

Bonus points later for proto->setPrivate(NULL) instead of proto->setPrivate(0), and no need for blank line after that call and before the return proto; line.

Attachment #422150 - Flags: review?(brendan) → review+
You need to log in before you can comment on or make changes to this bug.