Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(138)

Issue 10873099: Flag and adapter class for software compositing. (Closed)

Created:
8 years, 3 months ago by aelias_OOO_until_Jul13
Modified:
8 years, 3 months ago
Reviewers:
jamesr, nduca, piman
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, apatrick_chromium, darin (slow to review), danakj, enne (OOO), jamesr
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Flag and adapter class for software compositing. I'm adding a new CCRendererSkia class to CC, providing support for compositing a website to a software bitmap. However, the rest of the software compositing pipeline (delegating shared-memory resources and quadlists to the root compositor) is not yet done, so there's no way to get this bitmap onto the screen. As a stopgap, add an adapter class that uploads to a viewport-sized GL texture every frame, and a flag --force-software-compositing to activate it. BUG=124671 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158388

Patch Set 1 #

Total comments: 29

Patch Set 2 : Apply comments and rename everything to "Software" #

Total comments: 14

Patch Set 3 : Applied review comments #

Total comments: 1

Patch Set 4 : Final nit #

Patch Set 5 : Flip Y axis in vertex shader #

Patch Set 6 : Switch to using lockForWrite/lockForRead API #

Patch Set 7 : Adjust for new API #

Patch Set 8 : Rebase #

Patch Set 9 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+272 lines, -13 lines) Patch
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M content/content_renderer.gypi View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/common/content_switches.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_switches.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
M content/renderer/gpu/compositor_output_surface.h View 1 2 3 4 5 6 4 chunks +5 lines, -2 lines 0 comments Download
M content/renderer/gpu/compositor_output_surface.cc View 1 2 3 4 5 6 4 chunks +17 lines, -7 lines 0 comments Download
A content/renderer/gpu/compositor_software_output_device_gl_adapter.h View 1 2 3 4 5 6 1 chunk +54 lines, -0 lines 0 comments Download
A content/renderer/gpu/compositor_software_output_device_gl_adapter.cc View 1 2 3 4 5 6 1 chunk +178 lines, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 3 4 5 6 7 8 3 chunks +10 lines, -4 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
aelias_OOO_until_Jul13
This is the Chromium-side scaffolding to see the results of http://webk.it/93761
8 years, 3 months ago (2012-08-28 02:53:52 UTC) #1
piman
https://chromiumcodereview.appspot.com/10873099/diff/1/content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc File content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc (right): https://chromiumcodereview.appspot.com/10873099/diff/1/content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc#newcode25 content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc:25: , context3d_(context3D) { nit: style CompositorOutputCanvas3DTo2DAdapter::CompositorOutputCanvas3DTo2DAdapter( WebGraphicsContext3D* context3D) : ...
8 years, 3 months ago (2012-08-28 03:31:57 UTC) #2
nduca
In the future, force software compositing will be backed by a real skia device? How ...
8 years, 3 months ago (2012-08-28 06:17:06 UTC) #3
nduca
(For instance to be constructive, should we be factoring this as --enable-software-compositing --disable-gpu? That's closer ...
8 years, 3 months ago (2012-08-28 06:18:24 UTC) #4
aelias_OOO_until_Jul13
The behavior I implemented in this patch matches "force"; it uses software compositing even if ...
8 years, 3 months ago (2012-08-28 19:47:02 UTC) #5
nduca
Can we just fallback on the adapter if we can't do anything else? E.g. we'll ...
8 years, 3 months ago (2012-08-28 20:04:50 UTC) #6
aelias_OOO_until_Jul13
I don't much like the idea of auto-fallback to this adapter. It's really slow and ...
8 years, 3 months ago (2012-08-28 20:33:42 UTC) #7
nduca
On 2012/08/28 20:33:42, aelias wrote: > I don't much like the idea of auto-fallback to ...
8 years, 3 months ago (2012-08-28 20:37:05 UTC) #8
aelias_OOO_until_Jul13
OK, on second thought I renamed everything to use the word "software" for consistency. https://chromiumcodereview.appspot.com/10873099/diff/1/content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc ...
8 years, 3 months ago (2012-08-28 23:10:58 UTC) #9
piman
Mostly nits. https://chromiumcodereview.appspot.com/10873099/diff/1/content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc File content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc (right): https://chromiumcodereview.appspot.com/10873099/diff/1/content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc#newcode32 content/renderer/gpu/compositor_output_canvas_3d_to_2d_adapter.cc:32: if (!initialized_) On 2012/08/28 23:10:58, aelias wrote: ...
8 years, 3 months ago (2012-08-28 23:26:59 UTC) #10
jamesr
https://chromiumcodereview.appspot.com/10873099/diff/8002/content/renderer/gpu/compositor_output_surface.h File content/renderer/gpu/compositor_output_surface.h (right): https://chromiumcodereview.appspot.com/10873099/diff/8002/content/renderer/gpu/compositor_output_surface.h#newcode47 content/renderer/gpu/compositor_output_surface.h:47: const OVERRIDE; please do not OVERRIDE for implementations in ...
8 years, 3 months ago (2012-08-28 23:38:14 UTC) #11
aelias_OOO_until_Jul13
All nits applied. For the destructor, I have added a LOG(ERROR) and tried different tab-closing/navigation ...
8 years, 3 months ago (2012-08-28 23:53:11 UTC) #12
piman
LGTM+nit Lazy initialization prevents you from failing if, say, you fail to make current (this ...
8 years, 3 months ago (2012-08-29 00:21:51 UTC) #13
aelias_OOO_until_Jul13
To be clear, this isn't something I intend ever to be turned on in production. ...
8 years, 3 months ago (2012-08-29 00:24:53 UTC) #14
nduca
lgtm
8 years, 3 months ago (2012-08-29 00:30:52 UTC) #15
nduca
@aelias, are you landing this?
8 years, 3 months ago (2012-09-12 23:57:30 UTC) #16
aelias_OOO_until_Jul13
On 2012/09/12 23:57:30, nduca wrote: > @aelias, are you landing this? The CC side is ...
8 years, 3 months ago (2012-09-13 00:02:23 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aelias@chromium.org/10873099/28010
8 years, 3 months ago (2012-09-24 19:09:04 UTC) #18
commit-bot: I haz the power
8 years, 3 months ago (2012-09-24 21:26:48 UTC) #19
Change committed as 158388

Powered by Google App Engine
This is Rietveld 408576698