Summary: | [chromium] Implement software renderer | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexandre Elias <aelias> | ||||||||||||||||||||
Component: | New Bugs | Assignee: | Alexandre Elias <aelias> | ||||||||||||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||||||||||||
Severity: | Normal | CC: | abarth, aelias, cc-bugs, danakj, dglazkov, enne, fishd, jamesr, nduca, piman, schenney, tkent+wkapi, webkit.review.bot | ||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
Bug Depends on: | 93677, 93927 | ||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||
Attachments: |
|
Description
Alexandre Elias
2012-08-10 21:29:12 PDT
Created attachment 157857 [details]
Patch
Proof of concept, not yet for review. I wanted to put this up particularly for comments on the shared code between the GL and software renderers. In the current patch, I put it inside CCRenderer, but I'm thinking it won't be useful to CCExternalRenderer and that a class in between CCRenderer and the GL/software implementations might make more sense. I can't think of a great name for it though (CCInternalRenderer?). Ooh, thanks for putting this up to look at. I'm happy to see so much common code move out of LRC. I'm guessing that CCExternalRenderer is the renderer that sends a frame to the host compositor? If you don't like internal vs. external, I think direct vs. indirect or immediate vs. deferred also might be reasonable names. </bikeshed> I'm thinking CCDirectRenderer and CCDelegatingRenderer. CCDelegatingRenderer sounds best to me. Created attachment 158224 [details] Patch Rebased on Bug 93927 Comment on attachment 158224 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158224&action=review Pretty awesome. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:701 > +#else This can be done by looking at presence of a context3d on the output surface, right? > This can be done by looking at presence of a context3d on the output surface, right?
In the short term this patch still depends on a GL context to output anything. In the long term, in the absence of a context3d we'll still need to choose between CCSoftwareRenderer and CCDelegatingRenderer. So this isn't quite settled yet.
Chosing between the delegating renderer and direct can be done by checking the hasParentCompositor capability on the output surface, right? Created attachment 159794 [details]
Patch
Rebased
Created attachment 160860 [details]
Patch
This is getting into a reviewable state, please take a look. The preliminary implementation of the 2d tearoff is at https://chromiumcodereview.appspot.com/10873099 Anyone have thoughts on testing strategy? Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. Created attachment 161080 [details]
Patch
Rename everything to 'software'
Comment on attachment 161080 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161080&action=review This patch is exciting to see. > Source/WebCore/platform/graphics/chromium/cc/CCRendererSoftware.cpp:177 > + case CCDrawQuad::TiledContent: > + drawTileQuad(frame, CCTileDrawQuad::materialCast(quad)); > + break; You might also want to add solid color quad handling in this first rev so you can get gutter quads on page zoom. > Source/WebCore/platform/graphics/chromium/cc/CCRendererSoftware.cpp:189 > + SkRect vertexRect = toSkRect(quadVertexRect()); > + SkIRect uvRect = toSkIRect(IntRect(quad->textureOffset(), quad->quadVisibleRect().size())); > + > + WebTransformationMatrix quadRectMatrix; > + quadRectTransform(&quadRectMatrix, quad->quadTransform(), quad->quadRect()); I think this is wrong, because calculating the visible rect from the quad rect doesn't update the texture offset. You need to use IntRect(quad->textureOffset(), quad->quadRect().size()) if you're going to use quad->quadRect() later. Alternatively, you can pass quad->quadVisibleRect() to quadRectTransform and make the uvRect IntRect(quad->textureOffset() + quad->quadVisibleRect().location() - quad->quadRect.location(), quad->quadVisibleRect.size()) instead. (In reply to comment #12) > > Anyone have thoughts on testing strategy? What about running this path through a small subset of layout tests with pixel results? It's not ideal, but it's probably the least amount of work to get tests up and running. It looks like you have an adapter to just upload the software bitmap to the screen and a command line flag to enable it. Once that and this are landed, you could add a virtual layout test directory for software compositing to run existing tests through DRT with that flag set. Created attachment 161288 [details]
Patch
Removed use of quadVisibleRect, implemented SolidColorQuad, and moved shared code into drawQuad()
(In reply to comment #15) > > Source/WebCore/platform/graphics/chromium/cc/CCRendererSoftware.cpp:189 > > + SkRect vertexRect = toSkRect(quadVertexRect()); > > + SkIRect uvRect = toSkIRect(IntRect(quad->textureOffset(), quad->quadVisibleRect().size())); > > + > > + WebTransformationMatrix quadRectMatrix; > > + quadRectTransform(&quadRectMatrix, quad->quadTransform(), quad->quadRect()); > > I think this is wrong, because calculating the visible rect from the quad rect doesn't update the texture offset. You need to use IntRect(quad->textureOffset(), quad->quadRect().size()) if you're going to use quad->quadRect() later. > > Alternatively, you can pass quad->quadVisibleRect() to quadRectTransform and make the uvRect IntRect(quad->textureOffset() + quad->quadVisibleRect().location() - quad->quadRect.location(), quad->quadVisibleRect.size()) instead. OK, in getting it up and running a few weeks ago, I remember needing to use quadVisibleRect to get correct texture coordinates, but now just using quadRect in both places works just as well, so I switched to that. Created attachment 161304 [details]
Patch
Added support for scissoring and opacity
Created attachment 161325 [details]
Patch
Implemented CCTextureDrawQuad (needed for scrollbars)
Created attachment 161407 [details]
Patch
Implemented CCDebugBorderDrawQuad and added flipped TextureDrawQuad support
Any more thoughts about testing? I think pixel layout tests are the right way to test this, but it might take a few weeks before I have time to tackle the test infrastructure. Do you want to hold off on landing this until we have them? Comment on attachment 161407 [details]
Patch
R- as this will land in the Chromium tree.
|