RESOLVED WONTFIX 93761
[chromium] Implement software renderer
https://bugs.webkit.org/show_bug.cgi?id=93761
Summary [chromium] Implement software renderer
Alexandre Elias
Reported 2012-08-10 21:29:12 PDT
[chromium] Implement software renderer
Attachments
Patch (59.79 KB, patch)
2012-08-10 21:35 PDT, Alexandre Elias
no flags
Patch (24.30 KB, patch)
2012-08-13 23:05 PDT, Alexandre Elias
no flags
Patch (23.54 KB, patch)
2012-08-21 16:30 PDT, Alexandre Elias
no flags
Patch (22.01 KB, patch)
2012-08-27 17:03 PDT, Alexandre Elias
no flags
Patch (22.39 KB, patch)
2012-08-28 16:09 PDT, Alexandre Elias
no flags
Patch (22.92 KB, patch)
2012-08-29 12:40 PDT, Alexandre Elias
no flags
Patch (23.18 KB, patch)
2012-08-29 13:46 PDT, Alexandre Elias
no flags
Patch (24.54 KB, patch)
2012-08-29 15:08 PDT, Alexandre Elias
no flags
Patch (26.07 KB, patch)
2012-08-29 23:31 PDT, Alexandre Elias
aelias: review-
Alexandre Elias
Comment 1 2012-08-10 21:35:39 PDT
Alexandre Elias
Comment 2 2012-08-10 21:43:53 PDT
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?).
Adrienne Walker
Comment 3 2012-08-13 09:20:05 PDT
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>
Alexandre Elias
Comment 4 2012-08-13 13:12:45 PDT
I'm thinking CCDirectRenderer and CCDelegatingRenderer.
Antoine Labour
Comment 5 2012-08-13 13:21:33 PDT
CCDelegatingRenderer sounds best to me.
Alexandre Elias
Comment 6 2012-08-13 23:05:11 PDT
Created attachment 158224 [details] Patch Rebased on Bug 93927
Nat Duca
Comment 7 2012-08-15 10:40:28 PDT
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?
Alexandre Elias
Comment 8 2012-08-15 11:29:43 PDT
> 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.
Nat Duca
Comment 9 2012-08-15 11:36:49 PDT
Chosing between the delegating renderer and direct can be done by checking the hasParentCompositor capability on the output surface, right?
Alexandre Elias
Comment 10 2012-08-21 16:30:20 PDT
Created attachment 159794 [details] Patch Rebased
Alexandre Elias
Comment 11 2012-08-27 17:03:41 PDT
Alexandre Elias
Comment 12 2012-08-27 17:05:13 PDT
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?
WebKit Review Bot
Comment 13 2012-08-27 17:05:45 PDT
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.
Alexandre Elias
Comment 14 2012-08-28 16:09:26 PDT
Created attachment 161080 [details] Patch Rename everything to 'software'
Adrienne Walker
Comment 15 2012-08-29 10:41:38 PDT
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.
Adrienne Walker
Comment 16 2012-08-29 10:59:29 PDT
(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.
Alexandre Elias
Comment 17 2012-08-29 12:40:54 PDT
Created attachment 161288 [details] Patch Removed use of quadVisibleRect, implemented SolidColorQuad, and moved shared code into drawQuad()
Alexandre Elias
Comment 18 2012-08-29 12:43:32 PDT
(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.
Alexandre Elias
Comment 19 2012-08-29 13:46:50 PDT
Created attachment 161304 [details] Patch Added support for scissoring and opacity
Alexandre Elias
Comment 20 2012-08-29 15:08:07 PDT
Created attachment 161325 [details] Patch Implemented CCTextureDrawQuad (needed for scrollbars)
Alexandre Elias
Comment 21 2012-08-29 23:31:22 PDT
Created attachment 161407 [details] Patch Implemented CCDebugBorderDrawQuad and added flipped TextureDrawQuad support
Adrienne Walker
Comment 22 2012-09-05 14:40:49 PDT
Any more thoughts about testing?
Alexandre Elias
Comment 23 2012-09-05 14:46:04 PDT
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?
Alexandre Elias
Comment 24 2012-09-14 16:11:22 PDT
Comment on attachment 161407 [details] Patch R- as this will land in the Chromium tree.
Note You need to log in before you can comment on or make changes to this bug.