[Webkit-unassigned] [Bug 47070] [Texmap] [Qt] Texture mapper initial implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47070





--- Comment #6 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-10-04 06:14:24 PST ---
(From update of attachment 69623)
View in context: https://bugs.webkit.org/attachment.cgi?id=69623&action=review

> WebCore/ChangeLog:7
> +        The idea is that in time this would replace GraphicsLayerQt, and could serve as an implementation for other platforms that don't have a scenegraph library.

with time

> WebCore/ChangeLog:9
> +        an adequate level of stability, we can enable it by default and perhaps have it replace GraphicsLayerQt.

I don't like the 'perhaps' here... It sounds like we are adding code that we are not sure whether we will use or not, or just abandon.

Maybe write: "we will enable it by default and replace GraphicsLayerQt on platforms where that makes most sense.

Try to keep these within 80-100 chars per line

> WebCore/platform/graphics/opengl/TextureMapperGL.cpp:59
> +    void glDeleteFramebuffers(GLsizei n, const GLuint *framebuffers);
> +    void glGenFramebuffers(GLsizei n, GLuint *framebuffers);

* alignment

> WebCore/platform/graphics/opengl/TextureMapperGL.cpp:61
> +    void glGetFramebufferAttachmentParameteriv(GLenum target, GLenum attachment, GLenum pname, GLint *params);

* alignment

> WebCore/platform/graphics/opengl/TextureMapperGL.cpp:77
> +namespace WTF {

Maybe add a comment why you need this for now.

> WebCore/platform/graphics/opengl/TextureMapperGL.cpp:107
> +        printf("[TextureMapper GL] Command failed: Line %d\n\toperation: %s, \n\terror: %x\n", line, command, err);

I think webkit has some ways to do debugging.

> WebCore/platform/graphics/qt/TextureMapperQt.cpp:43
> +    virtual bool save(const String&);

Please add a variable like "save(const String& path)"

> WebCore/platform/graphics/qt/TextureMapperQt.cpp:49
> +    GraphicsContext* m_gc;

context

> WebCore/platform/graphics/qt/TextureMapperQt.cpp:127
> +TextureMapperQt::TextureMapperQt(GraphicsContext* gc)

context please, not gc

> WebCore/platform/graphics/qt/TextureMapperQt.cpp:135
> +void TextureMapperQt::bindSurface(BitmapTexture* aSurface)

why aSurface and not just surface?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.