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

Issue 1376883004: Overlays: Remove special casing of primary overlay plane (Closed)

Created:
5 years, 2 months ago by ccameron
Modified:
5 years, 2 months ago
CC:
chromium-reviews, cc-bugs_chromium.org, Andre
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Overlays: Remove special casing of primary overlay plane Mac will soon want to skip drawing the primary overlay plane, which now can be accomplished by having the overlay strategy simply not emit it. BUG=533687 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/e532ed0710935ef66a860b08b8e2a1e36a426b27 Cr-Commit-Position: refs/heads/master@{#352186}

Patch Set 1 #

Patch Set 2 : Put back DCHECKs #

Patch Set 3 : Add missed file #

Total comments: 7

Patch Set 4 : Incorporate review feedback #

Patch Set 5 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+115 lines, -96 lines) Patch
M cc/output/direct_renderer.cc View 1 2 3 1 chunk +14 lines, -0 lines 0 comments Download
M cc/output/gl_renderer.cc View 1 1 chunk +0 lines, -4 lines 0 comments Download
M cc/output/gl_renderer_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M cc/output/output_surface.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M cc/output/output_surface.cc View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M cc/output/overlay_strategy_common.cc View 2 chunks +0 lines, -11 lines 0 comments Download
M cc/output/overlay_unittest.cc View 1 2 3 42 chunks +87 lines, -65 lines 0 comments Download
M content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.cc View 1 2 3 2 chunks +5 lines, -15 lines 0 comments Download

Messages

Total messages: 21 (4 generated)
ccameron
ptal
5 years, 2 months ago (2015-09-29 21:17:13 UTC) #2
alexst (slow to review)
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.cc&q=gpu_surface&sq=package:chromium&l=1 schedules the plane by itself, so it will need to be modified and directed ...
5 years, 2 months ago (2015-09-29 21:27:36 UTC) #3
ccameron
Uah, I too aggressively pruned my change there. Updated.
5 years, 2 months ago (2015-09-29 21:45:15 UTC) #4
alexst (slow to review)
lgtm, but please wait until Danny and/or Albert have a quick look to double check.
5 years, 2 months ago (2015-09-29 22:01:39 UTC) #6
ccameron
dnicoara, achaulk: any remarks, or is this good to go?
5 years, 2 months ago (2015-09-30 18:38:57 UTC) #7
achaulk
https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc File cc/output/overlay_processor.cc (left): https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc#oldcode28 cc/output/overlay_processor.cc:28: void OverlayProcessor::ProcessForOverlays( This doesn't always get called, because there ...
5 years, 2 months ago (2015-09-30 19:04:19 UTC) #8
ccameron
Thanks -- updated https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc File cc/output/overlay_processor.cc (right): https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc#newcode35 cc/output/overlay_processor.cc:35: if (strategies_.empty()) On 2015/09/30 19:04:19, achaulk ...
5 years, 2 months ago (2015-10-01 18:59:31 UTC) #9
achaulk
https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc File cc/output/overlay_processor.cc (right): https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc#newcode41 cc/output/overlay_processor.cc:41: primary_surface.display_rect = gfx::RectF(root_render_pass->output_rect); On 2015/10/01 18:59:31, ccameron wrote: > ...
5 years, 2 months ago (2015-10-01 19:41:46 UTC) #10
alexst (slow to review)
On 2015/10/01 19:41:46, achaulk wrote: > https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc > File cc/output/overlay_processor.cc (right): > > https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc#newcode41 > ...
5 years, 2 months ago (2015-10-01 19:46:37 UTC) #11
achaulk
On 2015/10/01 19:46:37, alexst wrote: > On 2015/10/01 19:41:46, achaulk wrote: > > > https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc ...
5 years, 2 months ago (2015-10-01 19:48:40 UTC) #12
ccameron
https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc File cc/output/overlay_processor.cc (right): https://codereview.chromium.org/1376883004/diff/40001/cc/output/overlay_processor.cc#newcode41 cc/output/overlay_processor.cc:41: primary_surface.display_rect = gfx::RectF(root_render_pass->output_rect); On 2015/10/01 19:41:46, achaulk wrote: > ...
5 years, 2 months ago (2015-10-01 21:08:48 UTC) #13
achaulk
lgtm
5 years, 2 months ago (2015-10-02 15:32:07 UTC) #14
ccameron
Thanks!
5 years, 2 months ago (2015-10-02 19:06:12 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1376883004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1376883004/80001
5 years, 2 months ago (2015-10-02 19:06:54 UTC) #18
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 2 months ago (2015-10-03 00:07:06 UTC) #19
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/e532ed0710935ef66a860b08b8e2a1e36a426b27 Cr-Commit-Position: refs/heads/master@{#352186}
5 years, 2 months ago (2015-10-03 00:09:13 UTC) #20
ccameron
5 years, 2 months ago (2015-10-05 17:47:19 UTC) #21
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1383293002/ by ccameron@chromium.org.

The reason for reverting is: Speculative revert because this might be related to
a browser crash rate uptick in crbug.com/539374..

Powered by Google App Engine
This is Rietveld 408576698