Bug 43608 – [chromium] Implement GLES2Canvas/Texture in terms of GraphicsContext3D instead of direct OpenGL calls

• Component: New Bugs
• Status: RESOLVED FIXED

[James Robinson]

[chromium] Implement GLES2Canvas/Texture in terms of GraphicsContext3D instead of direct OpenGL calls

[James Robinson]

Created attachment 63691 [details]
Patch

[James Robinson]

What say ye, Stephen?

I don't plan to land this until the GraphicsContext3D change lands in https://bugs.webkit.org/show_bug.cgi?id=43362 (assuming it does), but would appreciate a review.

[James Robinson]

BTW, we should rename these classes as well and move them somewhere common at some point.

[Stephen White]

Comment on attachment 63691 [details]
Patch

WebCore/platform/graphics/chromium/GLES2Canvas.cpp:117
 +          m_context->deleteBuffer(m_quadVertices);
While you're there, you should probably change this to m_quadIndices (bug).

WebCore/platform/graphics/chromium/GLES2Canvas.cpp:138
 +      m_context->prepareTexture();
I think this prepareTexture() call is wrong.

WebCore/platform/graphics/chromium/GLES2Canvas.cpp:167
 +      m_context->prepareTexture();
Same as above, prepareTexture() can go away (I think).

WebCore/platform/graphics/chromium/GLES2Texture.cpp:70
 +          return 0;
I think this will leak the texture.  If you're going to early-return here, you should deleteTexture().  It might be better to just move this check to the start of the function.

WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:801
 +          platformContext()->restore();
Rather than adding a platformContext()->restore here, I'd prefer that this block be moved above platformContext()->save().

Looks good other than that.

[James Robinson]

Created attachment 63746 [details]
Patch

[James Robinson]

PTAL.

[James Robinson]

Created attachment 63750 [details]
Patch

[Stephen White]

(In reply to comment #7)
> Created an attachment (id=63750) [details]
> Patch

LGTM (as a non-reviewer).

[Dimitri Glazkov (Google)]

Comment on attachment 63750 [details]
Patch

WebCore/ChangeLog:10
 +          applying the regex s/gl([A-Z])/m_context->%1</ and removing unnecessary makeCurrent() calls.
I approve the use of regular expressions to describe the changes being made. I would expect nothing less from you thereon.

[James Robinson]

Comment on attachment 63750 [details]
Patch

Clearing flags on attachment: 63750

Committed r64872: <http://trac.webkit.org/changeset/64872>

[James Robinson]

All reviewed patches have been landed.  Closing bug.