Closed Bug 517717 Opened 13 years ago Closed 13 years ago

Fix WebGL array compatibility

Categories

(Core :: Canvas: 2D, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mwsteele, Assigned: mwsteele)

Details

Attachments

(2 files, 2 obsolete files)

Update naming and fix uniform*.
Attachment #401658 - Flags: review?(vladimir)
Comment on attachment 401658 [details] [diff] [review]
WebGL array names; uniform* arrays

Let's get rid of support for JS arrays entirely here, for consistency.  We might have to put it back later once we figure out how to fast-path things, but for now let's leave them out.

Also, in each

+        if (glTypeConst == LOCAL_GL_INT) {                                    \
+            WebGLIntArray *wga = static_cast<WebGLIntArray*>(v);              \
+            gl->f##glname(idx, wga->NativeCount() / c, ( ptrType *)wga->NativePointer()); \
+        } else if (glTypeConst == LOCAL_GL_FLOAT) {                           \

you still have to check that arrayLen % c == 0, and error out if not.

Also, in the if (glTypeConst == .. ) { } else if (glTypeConst == .. ), add an 

 else {
  return ErrorMessage("Unhandled glTypeConst");
}

or something so this doesn't bite us if we use it with a different type.
Attachment #401658 - Flags: review?(vladimir) → review-
Now with more sleep.
The macro fallbacks should really be compiler errors. The conditional checks are constant so they shouldn't get hit unless the macro was misused.
Attachment #401658 - Attachment is obsolete: true
Attachment #401673 - Flags: review?(vladimir)
Comment on attachment 401673 [details] [diff] [review]
Only use WebGL arrays; rename constructors.

NativeCount() != c isn't correct -- that means that you can't set more than 1 element.  There's a bug here in that we really should be passing the count in, now that we can create offset/views (or will be able to, anyway).

Also I see the spec still actually allows for sequence<float> etc. for uniform.. but I think that's going to go away?  Not sure, it might make sense to have it, but we can always put it back later.
Comment on attachment 401673 [details] [diff] [review]
Only use WebGL arrays; rename constructors.

Actually, let's go with this -- we're missing some arguments there anyway (count, namely) that we're going to need to bring back I think (for the matrix methods).
Attachment #401673 - Flags: review?(vladimir) → review+
Comment on attachment 401673 [details] [diff] [review]
Only use WebGL arrays; rename constructors.

Argh, no, we have to keep the JS bits as well.  So, recap:

- Split patch into two -- one that does the renaming

For the renaming patch:

- Rename the interfaces as well, otherwise we have a DEBUG assert that fires

For the Uniform*/VertexAttrib* patch:

- Allow for both JS arrays and Canvas*Arrays.

- if a Canvas*Array is specified, check ->NativeType() on it before doing the static_cast (or QI, but NativeType() check should be faster... even if we get a bogus xpconnect-generated object here, since NS_ERROR_* won't ever be a valid GL type)

- for uniformMatrix*, use % 0 to check that the length is correctly divisible, not == 0; we have to infer the count correctly here (and could have multiple matrices we're loading)
Attachment #401673 - Attachment is obsolete: true
part 1
Attachment #401725 - Flags: review?(vladimir)
part 2
Attachment #401744 - Flags: review?(vladimir)
Checked in
Assignee: nobody → mwsteele
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.