Bug 45207 - [chromium] Implement ImageBufferSkia::draw on the GPU when possible
Summary: [chromium] Implement ImageBufferSkia::draw on the GPU when possible
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-03 15:35 PDT by James Robinson
Modified: 2010-09-07 17:41 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.05 KB, patch)
2010-09-03 15:43 PDT, James Robinson
no flags Details | Formatted Diff | Diff
Patch (5.35 KB, patch)
2010-09-03 17:36 PDT, James Robinson
no flags Details | Formatted Diff | Diff
adds a check that the source canvas is 2d before going down the accelerated path (5.57 KB, patch)
2010-09-03 17:50 PDT, James Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2010-09-03 15:35:22 PDT
[chromium] Implement ImageBufferSkia::draw on the GPU when possible
Comment 1 James Robinson 2010-09-03 15:43:31 PDT
Created attachment 66560 [details]
Patch
Comment 2 Kenneth Russell 2010-09-03 17:32:03 PDT
Comment on attachment 66560 [details]
Patch

We've discussed this offline, but in the multisampling case you definitely do not want to reallocate the color texture into which the blit occurs each frame, so releaseBackingTexture() is unneeded. Also, getBackingTexture() needs to be called each time we want to display the up-to-date content in the multisampling case, so it should probably be named getUpToDateBackingTexture() or similar.
Comment 3 James Robinson 2010-09-03 17:36:50 PDT
Created attachment 66572 [details]
Patch
Comment 4 James Robinson 2010-09-03 17:38:29 PDT
Good point.  Removed the release..() function and renamed the getter getRenderingResultsAsTexture(), which hopefully conveys that the returned texture gets the current rendering results.
Comment 5 Kenneth Russell 2010-09-03 17:44:37 PDT
Comment on attachment 66572 [details]
Patch

This looks better but I noticed one more thing; sorry.

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

> WebCore/html/canvas/CanvasRenderingContext2D.cpp:1286
> +    if (!isAccelerated() || !sourceContext || !sourceContext->isAccelerated())
For the time being, this needs a more specific check to make sure the source context is 2D. This won't work for the WebGL code path right now.
Comment 6 James Robinson 2010-09-03 17:50:49 PDT
Created attachment 66573 [details]
adds a check that the source canvas is 2d before going down the accelerated path
Comment 7 Kenneth Russell 2010-09-03 18:05:38 PDT
Comment on attachment 66573 [details]
adds a check that the source canvas is 2d before going down the accelerated path

Looks good to me.
Comment 8 James Robinson 2010-09-07 17:40:41 PDT
Landed at r66785 with a slightly-mismerged ChangeLog.
Comment 9 James Robinson 2010-09-07 17:41:21 PDT
Comment on attachment 66573 [details]
adds a check that the source canvas is 2d before going down the accelerated path

Clearing flags, patch has landed.