Bug 40746 – WebGLShader now keeps track of its shader type

• Component: WebGL
• Status: RESOLVED DUPLICATE

[Paul Sawaya]

Created attachment 58945 [details]
WebGLShader now keeps track of its shader type

WebGLShader needs to keep track of which type of shader it is (vertex, or fragment) to prepare for future enhancements to WebGL.

[Kenneth Russell]

Comment on attachment 58945 [details]
WebGLShader now keeps track of its shader type

WebCore/html/canvas/WebGLShader.h:45
 +          GraphicsContext3D::WebGLEnumType shaderType;
To conform to the WebKit style guide this should be "m_shaderType".


WebCore/html/canvas/WebGLShader.cpp:43
 +      shaderType = type;
This can be initialized in the initializer list: e.g.

  : CanvasObject(ctx)
  , m_shaderType(type)

[Chris Marrin]

Comment on attachment 58945 [details]
WebGLShader now keeps track of its shader type

> Index: WebCore/html/canvas/WebGLShader.cpp
> ===================================================================
> --- WebCore/html/canvas/WebGLShader.cpp	(revision 60639)
> +++ WebCore/html/canvas/WebGLShader.cpp	(working copy)
> @@ -40,6 +40,8 @@ PassRefPtr<WebGLShader> WebGLShader::cre
>  WebGLShader::WebGLShader(WebGLRenderingContext* ctx, GraphicsContext3D::WebGLEnumType type)
>      : CanvasObject(ctx)
>  {
> +    shaderType = type;
> +
>      setObject(context()->graphicsContext3D()->createShader(type));
>  }
>  
> Index: WebCore/html/canvas/WebGLShader.h
> ===================================================================
> --- WebCore/html/canvas/WebGLShader.h	(revision 60639)
> +++ WebCore/html/canvas/WebGLShader.h	(working copy)
> @@ -41,6 +41,8 @@ namespace WebCore {
>  
>      private:
>          WebGLShader(WebGLRenderingContext*, GraphicsContext3D::WebGLEnumType);
> +            
> +        GraphicsContext3D::WebGLEnumType shaderType;
>  
>          virtual void _deleteObject(Platform3DObject);
>  

WebCore/html/canvas/WebGLShader.cpp:43
 +      shaderType = type;
This can be initialized in the constructor, as in:

    : CanvasObject(ctx)
    , m_shaderType(type)


WebCore/html/canvas/WebGLShader.h:45
 +          GraphicsContext3D::WebGLEnumType shaderType;

The convention is to name this m_shaderType


WebCore/html/canvas/WebGLShader.h:46
 +  

You should add an accessor:

    GraphicsContext3D::WebGLEnumType shaderType () const { return m_shaderType; }

[Paul Sawaya]

Created attachment 59015 [details]
Added accessor, and made other code style changes as requested

[Sam Weinig]

Comment on attachment 59015 [details]
Added accessor, and made other code style changes as requested

>  WebGLShader::WebGLShader(WebGLRenderingContext* ctx, GraphicsContext3D::WebGLEnumType type)
> -    : CanvasObject(ctx)
> +    : CanvasObject(ctx), m_shaderType(type)

The ", m_shaderType(type)" should be on its own line.


This change is also missing a ChangeLog.  Please read http://webkit.org/coding/contributing.html for more information on how to do this.

[Zhenyao Mo]

Sorry I didn't see this bug before.  The feature is actually implemented in another bug: https://bugs.webkit.org/show_bug.cgi?id=41380.

*** This bug has been marked as a duplicate of bug 41380 ***

[Daniel Bates]

Comment on attachment 59015 [details]
Added accessor, and made other code style changes as requested

Clearing commit-queue flag to get this out of the commit-queue.