Bug 73621 - [chromium] Use ANGLE's texture_usage and texture_storage extensions when allocating compositor textures
Summary: [chromium] Use ANGLE's texture_usage and texture_storage extensions when allo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Vangelis Kokkevis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-01 22:11 PST by Vangelis Kokkevis
Modified: 2011-12-05 15:44 PST (History)
6 users (show)

See Also:


Attachments
Patch (14.52 KB, patch)
2011-12-01 22:32 PST, Vangelis Kokkevis
no flags Details | Formatted Diff | Diff
Patch (14.52 KB, patch)
2011-12-05 12:01 PST, Vangelis Kokkevis
kbr: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vangelis Kokkevis 2011-12-01 22:11:41 PST
GL_ANGLE_texture_usage (http://angleproject.googlecode.com/svn/trunk/extensions/ANGLE_texture_usage.txt)
By calling glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_USAGE_ANGLE, GL_FRAMEBUFFER_ATTACHMENT_ANGLE) on textures used by RenderSurfaces, ANGLE can eliminate the allocation of a system memory buffer for the texture.

GL_EXT_texture_storage (http://angleproject.googlecode.com/svn/trunk/extensions/EXT_texture_storage.txt)
By switching texture allocation in the compositor to using  glTexStorage2DEXT instead of glTexImage2D we can eliminate the unnecessary allocation of additional mip levels for each texture.


The two extensions are exposed to the command buffer via:  http://codereview.chromium.org/8772033/ which must land first in the chromium tree.
Comment 1 Vangelis Kokkevis 2011-12-01 22:32:20 PST
Created attachment 117568 [details]
Patch
Comment 2 WebKit Review Bot 2011-12-01 22:35:51 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 3 Darin Fisher (:fishd, Google) 2011-12-01 22:39:48 PST
Comment on attachment 117568 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117568&action=review

> Source/WebKit/chromium/public/platform/WebGraphicsContext3D.h:385
> +    virtual void texStorage2DEXT(WGC3Denum target, WGC3Dint levels, WGC3Duint internalformat,

I defer to kbr@ for review of WebGC3D, but I see no issues with this change ;-)
Comment 4 WebKit Review Bot 2011-12-01 23:11:06 PST
Comment on attachment 117568 [details]
Patch

Attachment 117568 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10703308
Comment 5 Vangelis Kokkevis 2011-12-05 09:21:36 PST
Ken, James: ping?
Comment 6 James Robinson 2011-12-05 11:10:27 PST
What's the compile failure on the cr-linux bot? I saw that and assumed it'd be addressed somehow so I haven't looked at the actual patch yet.
Comment 7 Kenneth Russell 2011-12-05 11:25:26 PST
Comment on attachment 117568 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117568&action=review

Looks good overall. One minor issue that will fix the build failure. Please upload a revised patch and we can r+/cq+ it.

> Source/WebKit/chromium/public/platform/WebGraphicsContext3D.h:386
> +                                 WGC3Dint width, WGC3Dint height) = 0;

Give this an empty body like the method above. That will fix the cr-linux build failure. I gather from fishd@ that this is OK to leave in essentially permanently.
Comment 8 Vangelis Kokkevis 2011-12-05 12:01:37 PST
Created attachment 117910 [details]
Patch
Comment 9 Vangelis Kokkevis 2011-12-05 12:02:33 PST
(In reply to comment #8)
> Created an attachment (id=117910) [details]
> Patch

Fixed compile issue as per Ken's suggestion. James, would you mind taking a look at the compositor change?  Thanks.
Comment 10 James Robinson 2011-12-05 12:10:08 PST
Comment on attachment 117910 [details]
Patch

Compositor changes LGTM
Comment 11 Kenneth Russell 2011-12-05 12:28:02 PST
Comment on attachment 117910 [details]
Patch

Looks fine; let this clear the cr-linux EWS bot before committing. r=me
Comment 12 WebKit Review Bot 2011-12-05 12:34:53 PST
Comment on attachment 117910 [details]
Patch

Attachment 117910 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10729807

New failing tests:
svg/custom/linking-uri-01-b.svg
Comment 13 Vangelis Kokkevis 2011-12-05 15:44:17 PST
Committed r102055: <http://trac.webkit.org/changeset/102055>