Closed Bug 1097411 Opened 5 years ago Closed 5 years ago

Extend error reporting for GL enums.


(Core :: Canvas: WebGL, defect)

Not set





(Reporter: kamidphish, Assigned: kamidphish)




(1 file, 1 obsolete file)

No description provided.
Attached patch Extend error reporting (obsolete) — Splinter Review
Attachment #8524361 - Flags: review?(jgilbert)
Comment on attachment 8524361 [details] [diff] [review]
Extend error reporting

Review of attachment 8524361 [details] [diff] [review]:

::: dom/canvas/WebGLContextUtils.cpp
@@ +505,5 @@
>  void
>  WebGLContext::ErrorInvalidEnumInfo(const char *info, GLenum enumvalue)
>  {
> +    const char* name = EnumName(enumvalue);
> +    if (!name || name[0] == '[')

We should really have two functions:
Infallible `const char* EnumName(GLenum val)`
Fallible `bool EnumName(GLenum val, const char* const out_name)`

In theory, the fallible version could just return null, but I think it's more clear which is fallible this way.

Use the fallible version here.

@@ +508,5 @@
> +    const char* name = EnumName(enumvalue);
> +    if (!name || name[0] == '[')
> +        return ErrorInvalidEnum("%s: invalid enum value 0x%04x", info, enumvalue);
> +
> +    return ErrorInvalidEnum("%s: invalid enum value %s", info, name);

So here's the problem with this. It assumes (GLenum val)->(const char* name) is one-to-one. This isn't universally true, though. 

The proper construction here for all GLenums is map<GLenum val, set<const char*> names>. 

Maybe the question is whether this matters. Restrict `GLenum val` to >= 0x10 and we should be one-to-one again.
Attachment #8524361 - Flags: review?(jgilbert) → review+
Yeah, I know it's not one-to-one, but how do you select the result you want? An option to the function call, I suppose.
Updated with fallible and non-fallible versions of EnumName.
Attachment #8526465 - Flags: review?(jgilbert)
Attachment #8524361 - Attachment is obsolete: true
Attachment #8526465 - Flags: review?(jgilbert) → review+
(In reply to Dan Glastonbury :djg :kamidphish from comment #6)

*sigh* Programming is hard.
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.