Summary: | [chromium] Implement GLES2Canvas/Texture in terms of GraphicsContext3D instead of direct OpenGL calls | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Robinson <jamesr> | ||||||||
Component: | New Bugs | Assignee: | James Robinson <jamesr> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | fishd, senorblanco | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
James Robinson
2010-08-05 22:03:33 PDT
Created attachment 63691 [details]
Patch
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. BTW, we should rename these classes as well and move them somewhere common at some point. 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.
Created attachment 63746 [details]
Patch
PTAL. Created attachment 63750 [details]
Patch
(In reply to comment #7) > Created an attachment (id=63750) [details] > Patch LGTM (as a non-reviewer). 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.
Comment on attachment 63750 [details] Patch Clearing flags on attachment: 63750 Committed r64872: <http://trac.webkit.org/changeset/64872> All reviewed patches have been landed. Closing bug. |