Bug 30276 - Implement getActiveAttrib and getActiveUniform
Summary: Implement getActiveAttrib and getActiveUniform
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-10 06:18 PDT by Oliver Hunt
Modified: 2009-11-06 16:59 PST (History)
1 user (show)

See Also:


Attachments
Patch v1 (27.65 KB, patch)
2009-10-10 06:54 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch v1 (27.68 KB, patch)
2009-10-10 12:53 PDT, Oliver Hunt
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2009-10-10 06:18:57 PDT
as in title
Comment 1 Oliver Hunt 2009-10-10 06:54:03 PDT
Created attachment 40990 [details]
Patch v1
Comment 2 mitz 2009-10-10 10:51:20 PDT
Comment on attachment 40990 [details]
Patch v1

A few notes, even though I can’t review this patch:

> +        mechanical work needed to set up a webgl context for testing.

It’s spelled WebGL.

> + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY

This is not the correct license.

> +#include "PlatformString.h"
> +
> +#include <wtf/PassRefPtr.h>

Extra newline.

> +    int size() const { return m_size; }
> +private:

But there should be a newline before “private:”

> + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY

License.

> +module html {
> +    interface [

Should have a newline there.

> +        Conditional=3D_CANVAS,
> +    ] CanvasActiveInfo {
> +        readonly attribute int size;
> +        readonly attribute unsigned int type;
> +        readonly attribute DOMString name;
> +    };
> +}

And another one before the last brace.

> +PassRefPtr<CanvasActiveInfo> CanvasRenderingContext3D::getActiveAttrib(CanvasProgram* program, unsigned long index, ExceptionCode& ec)
> +{
> +    ec = 0;

You shouldn’t reset ec. The caller should do it if it cares (the autogenerated bindings do).

> +PassRefPtr<CanvasActiveInfo> CanvasRenderingContext3D::getActiveUniform(CanvasProgram* program, unsigned long index, ExceptionCode& ec)
> +{
> +    ec = 0;

Ditto.

> +        bool getActiveAttrib(CanvasProgram* program, unsigned long index, ActiveInfo&);
> +        bool getActiveUniform(CanvasProgram* program, unsigned long index, ActiveInfo&);

No need to name the “program” parameter.

>  #include "CachedImage.h"
> +#include "CanvasActiveInfo.h"
>  #include "CanvasBuffer.h"
>  #include "CanvasFramebuffer.h"
>  #include "CanvasArray.h"

These are not in the right order.
Comment 3 Oliver Hunt 2009-10-10 12:53:38 PDT
Created attachment 40994 [details]
Patch v1
Comment 4 Eric Carlson 2009-10-10 13:28:15 PDT
Comment on attachment 40994 [details]
Patch v1

r=me
Comment 5 Oliver Hunt 2009-10-10 14:17:32 PDT
Committed r49420