WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(24.30 KB, patch)
2012-08-13 23:05 PDT
,
Alexandre Elias
no flags
Details
Formatted Diff
Diff
Patch
(23.54 KB, patch)
2012-08-21 16:30 PDT
,
Alexandre Elias
no flags
Details
Formatted Diff
Diff
Patch
(22.01 KB, patch)
2012-08-27 17:03 PDT
,
Alexandre Elias
no flags
Details
Formatted Diff
Diff
Patch
(22.39 KB, patch)
2012-08-28 16:09 PDT
,
Alexandre Elias
no flags
Details
Formatted Diff
Diff
Patch
(22.92 KB, patch)
2012-08-29 12:40 PDT
,
Alexandre Elias
no flags
Details
Formatted Diff
Diff
Patch
(23.18 KB, patch)
2012-08-29 13:46 PDT
,
Alexandre Elias
no flags
Details
Formatted Diff
Diff
Patch
(24.54 KB, patch)
2012-08-29 15:08 PDT
,
Alexandre Elias
no flags
Details
Formatted Diff
Diff
Patch
(26.07 KB, patch)
2012-08-29 23:31 PDT
,
Alexandre Elias
aelias
: review-
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Alexandre Elias
Comment 1
2012-08-10 21:35:39 PDT
Created
attachment 157857
[details]
Patch
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
Created
attachment 160860
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug