Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(65)

Issue 7890046: Command to mark surface inactive, so gpu process can release resources. (Closed)

Created:
8 years, 2 months ago by mmocny
Modified:
8 years, 1 month ago
CC:
chromium-reviews, dpranke+watch-content_chromium.org, jam, joi+watch-content_chromium.org, darin-cc_chromium.org, PaweĊ‚ Hajdan Jr., mmocny
Visibility:
Public.

Description

Adding gl command to change surface visibility, so that the gpu process can appropriately release/reacquire resources. BUG=5175544 TEST=None Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105563

Patch Set 1 #

Patch Set 2 : changed gl command to setVisible(bool) and added webgraphicscontext3d implementation #

Patch Set 3 : Bugfixes, changed signature #

Patch Set 4 : updating with recent changes #

Total comments: 11

Patch Set 5 : . #

Patch Set 6 : Flush only on hide #

Total comments: 3

Patch Set 7 : . #

Patch Set 8 : removing changes to image xport header file #

Patch Set 9 : fixing mock gles2 cmd decoder #

Total comments: 2

Patch Set 10 : first cut of IPC version #

Total comments: 6

Patch Set 11 : Updated requested changes #

Total comments: 2

Patch Set 12 : final rebase and added comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -0 lines) Patch
M content/common/gpu/gpu_command_buffer_stub.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M content/common/gpu/gpu_command_buffer_stub.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +6 lines, -0 lines 0 comments Download
M content/common/gpu/gpu_messages.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M content/common/gpu/image_transport_surface_linux.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +10 lines, -0 lines 0 comments Download
M content/renderer/gpu/command_buffer_proxy.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -0 lines 0 comments Download
M content/renderer/gpu/command_buffer_proxy.cc View 1 2 3 4 5 6 7 8 9 1 chunk +9 lines, -0 lines 0 comments Download
M content/renderer/gpu/renderer_gl_context.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -0 lines 0 comments Download
M content/renderer/gpu/renderer_gl_context.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -0 lines 0 comments Download
M content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +6 lines, -0 lines 0 comments Download
M ui/gfx/gl/gl_surface.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M ui/gfx/gl/gl_surface.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +3 lines, -0 lines 0 comments Download
M webkit/gpu/webgraphicscontext3d_in_process_impl.h View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -0 lines 0 comments Download
M webkit/gpu/webgraphicscontext3d_in_process_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
jonathan.backer
I don't see where the ImageTransportSurface's use the framework on TOUCH_UI. Separate CL? http://codereview.chromium.org/7890046/diff/8001/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc File ...
8 years, 2 months ago (2011-09-27 16:53:14 UTC) #1
apatrick_chromium
http://codereview.chromium.org/7890046/diff/8001/gpu/command_buffer/build_gles2_cmd_buffer.py File gpu/command_buffer/build_gles2_cmd_buffer.py (right): http://codereview.chromium.org/7890046/diff/8001/gpu/command_buffer/build_gles2_cmd_buffer.py#newcode226 gpu/command_buffer/build_gles2_cmd_buffer.py:226: GL_APICALL void GL_APIENTRY glSetSurfaceVisibleCHROMIUM (GLboolean visible); Is there any ...
8 years, 2 months ago (2011-09-28 17:33:45 UTC) #2
mmocny
http://codereview.chromium.org/7890046/diff/8001/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc File content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc (right): http://codereview.chromium.org/7890046/diff/8001/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc#newcode282 content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc:282: gl_->Flush(); See discussion in other comment. On 2011/09/27 16:53:14, ...
8 years, 2 months ago (2011-09-28 21:37:33 UTC) #3
apatrick_chromium
http://codereview.chromium.org/7890046/diff/8001/gpu/command_buffer/build_gles2_cmd_buffer.py File gpu/command_buffer/build_gles2_cmd_buffer.py (right): http://codereview.chromium.org/7890046/diff/8001/gpu/command_buffer/build_gles2_cmd_buffer.py#newcode226 gpu/command_buffer/build_gles2_cmd_buffer.py:226: GL_APICALL void GL_APIENTRY glSetSurfaceVisibleCHROMIUM (GLboolean visible); On 2011/09/28 21:37:33, ...
8 years, 2 months ago (2011-09-28 22:29:45 UTC) #4
mmocny
I've updated to remove my debug code and change the flush to something less heavy ...
8 years, 2 months ago (2011-10-07 14:26:13 UTC) #5
jonathan.backer
I removed image_transport_surface_linux.h in an earlier CL. Forgot to delete it. http://codereview.chromium.org/7890046/diff/17001/content/common/gpu/image_transport_surface_linux.cc File content/common/gpu/image_transport_surface_linux.cc (right): ...
8 years, 2 months ago (2011-10-07 15:48:36 UTC) #6
apatrick_chromium
http://codereview.chromium.org/7890046/diff/8001/gpu/command_buffer/build_gles2_cmd_buffer.py File gpu/command_buffer/build_gles2_cmd_buffer.py (right): http://codereview.chromium.org/7890046/diff/8001/gpu/command_buffer/build_gles2_cmd_buffer.py#newcode226 gpu/command_buffer/build_gles2_cmd_buffer.py:226: GL_APICALL void GL_APIENTRY glSetSurfaceVisibleCHROMIUM (GLboolean visible); Yeah that's the ...
8 years, 2 months ago (2011-10-07 22:45:49 UTC) #7
mmocny
http://codereview.chromium.org/7890046/diff/19022/content/common/gpu/image_transport_surface.h File content/common/gpu/image_transport_surface.h (right): http://codereview.chromium.org/7890046/diff/19022/content/common/gpu/image_transport_surface.h#newcode110 content/common/gpu/image_transport_surface.h:110: void SurfaceVisible(bool visible); You are correct. First I thought ...
8 years, 2 months ago (2011-10-11 13:58:43 UTC) #8
mmocny
Replaced dropping inactive backbuffer with IPC implementation. I had a few choices as to how ...
8 years, 1 month ago (2011-10-13 18:54:17 UTC) #9
jonathan.backer
SGTM http://codereview.chromium.org/7890046/diff/31001/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc File content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc (right): http://codereview.chromium.org/7890046/diff/31001/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc#newcode275 content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc:275: context_->GetCommandBufferProxy()->SetSurfaceVisible(visible); I'm OK with it as is. There's ...
8 years, 1 month ago (2011-10-13 19:25:07 UTC) #10
apatrick_chromium
Thanks, this is much simpler. A few comments but no major objections. http://codereview.chromium.org/7890046/diff/31001/content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc File content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc ...
8 years, 1 month ago (2011-10-13 19:51:31 UTC) #11
mmocny
Updated with the changes requested. http://codereview.chromium.org/7890046/diff/35002/content/renderer/gpu/command_buffer_proxy.h File content/renderer/gpu/command_buffer_proxy.h (right): http://codereview.chromium.org/7890046/diff/35002/content/renderer/gpu/command_buffer_proxy.h#newcode73 content/renderer/gpu/command_buffer_proxy.h:73: bool SetSurfaceVisible(bool visible); TODO: ...
8 years, 1 month ago (2011-10-13 21:48:43 UTC) #12
apatrick_chromium
Looks fine. Just add the noted comments and then LGTM for my part.
8 years, 1 month ago (2011-10-13 21:53:05 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mmocny@chromium.org/7890046/41001
8 years, 1 month ago (2011-10-14 17:57:23 UTC) #14
commit-bot: I haz the power
8 years, 1 month ago (2011-10-14 20:35:55 UTC) #15
Change committed as 105563

Powered by Google App Engine
This is Rietveld 408576698