content: Add overdraw feedback support for X11 ChromeOS builds.

content: Add overdraw feedback support for X11 ChromeOS builds.

This allows the overdraw feedback debugging feature to not only
be used on ChromeOS devices but also on Linux workstations.

BUG=678234
Review-Url: https://codereview.chromium.org/2643623004
Cr-Commit-Position: refs/heads/master@{#444653}
Committed: https://chromium.googlesource.com/chromium/src/+/db06141ebf2aa70c3ffdad7a112153568e93d8bc

[reveman, 2017-01-19 01:02:44]

reveman@chromium.org changed reviewers:
+ dcastagna@chromium.org

[Daniele Castagna, 2017-01-19 01:07:53]

LGTM

https://codereview.chromium.org/2643623004/diff/1/content/browser/compositor/...
File content/browser/compositor/mus_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/2643623004/diff/1/content/browser/compositor/...
content/browser/compositor/mus_browser_compositor_output_surface.cc:36: false),
nit: Can you add /* supports_stencil */

[reveman, 2017-01-19 01:16:52]

reveman@chromium.org changed reviewers:
+ jbauman@chromium.org

[reveman, 2017-01-19 01:16:53]

+jbauman for owner review

https://codereview.chromium.org/2643623004/diff/1/content/browser/compositor/...
File content/browser/compositor/mus_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/2643623004/diff/1/content/browser/compositor/...
content/browser/compositor/mus_browser_compositor_output_surface.cc:36: false),
On 2017/01/19 at 01:07:53, Daniele Castagna wrote:
> nit: Can you add /* supports_stencil */

Done.

[jbauman, 2017-01-19 01:53:59]

lgtm for now, though this may still not work on some systems.

https://codereview.chromium.org/2643623004/diff/20001/content/browser/composi...
File content/browser/compositor/gpu_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/2643623004/diff/20001/content/browser/composi...
content/browser/compositor/gpu_browser_compositor_output_surface.cc:37:
capabilities_.supports_stencil = support_stencil;
We should really try to grab this from the context, as there's no guarantee it's
supported even if we ask for it.

https://codereview.chromium.org/2643623004/diff/20001/content/browser/composi...
File content/browser/compositor/gpu_process_transport_factory.cc (right):

https://codereview.chromium.org/2643623004/diff/20001/content/browser/composi...
content/browser/compositor/gpu_process_transport_factory.cc:139:
attributes.stencil_size = need_stencil_bits ? 8 : 0;
On linux I don't think this actually affects whether or not the window gets a
stencil. We always try to choose a visual for every window that doesn't have a
stencil if there is a visual that doesn't have a stencil.

[reveman, 2017-01-19 04:15:07]

The CQ bit was checked by reveman@chromium.org

[reveman, 2017-01-19 04:15:08]

On 2017/01/19 at 01:53:59, jbauman wrote:
> lgtm for now, though this may still not work on some systems.

Agreed. But it works on the Linux systems I've aware of which I think goes a
long way for a debugging feature that I don't think we should make more
complicated than necessary.

https://codereview.chromium.org/2643623004/diff/20001/content/browser/composi...
File content/browser/compositor/gpu_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/2643623004/diff/20001/content/browser/composi...
content/browser/compositor/gpu_browser_compositor_output_surface.cc:37:
capabilities_.supports_stencil = support_stencil;
On 2017/01/19 at 01:53:59, jbauman wrote:
> We should really try to grab this from the context, as there's no guarantee
it's supported even if we ask for it.

I think what we really want to do is prefer an X visual with stencil bits and
only mark this as supported if one was found. That is if we're on X11 (only case
today when this will be true and we're not surfaceless). It happens that the
preferred visual on X always has stencil support and I considered adding some
code here to check that but it seemed overkill for this debugging feature. Added
a TODO here to latest patch that we can follow up on if needed.

https://codereview.chromium.org/2643623004/diff/20001/content/browser/composi...
File content/browser/compositor/gpu_process_transport_factory.cc (right):

https://codereview.chromium.org/2643623004/diff/20001/content/browser/composi...
content/browser/compositor/gpu_process_transport_factory.cc:139:
attributes.stencil_size = need_stencil_bits ? 8 : 0;
On 2017/01/19 at 01:53:59, jbauman wrote:
> On linux I don't think this actually affects whether or not the window gets a
stencil. We always try to choose a visual for every window that doesn't have a
stencil if there is a visual that doesn't have a stencil.

It doesn't but it effects if we allow some stencil operations or not. See:

https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_dec...
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_dec...

[reveman, 2017-01-19 04:15:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
dcastagna@chromium.org, jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2643623004/#ps40001
(title: "add todo")

[commit-bot: I haz the power, 2017-01-19 04:15:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 05:29:10]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484799307821570,
"parent_rev": "5f51a0f9f16fc3ff666e484a1047cadda952014d", "commit_rev":
"db06141ebf2aa70c3ffdad7a112153568e93d8bc"}

[commit-bot: I haz the power, 2017-01-19 05:30:09]

Description was changed from

==========
content: Add overdraw feedback support for X11 ChromeOS builds.

This allows the overdraw feedback debugging feature to not only
be used on ChromeOS devices but also on Linux workstations.

BUG=678234
==========

to

==========
content: Add overdraw feedback support for X11 ChromeOS builds.

This allows the overdraw feedback debugging feature to not only
be used on ChromeOS devices but also on Linux workstations.

BUG=678234

Review-Url: https://codereview.chromium.org/2643623004
Cr-Commit-Position: refs/heads/master@{#444653}
Committed:
https://chromium.googlesource.com/chromium/src/+/db06141ebf2aa70c3ffdad7a1121...
==========

[commit-bot: I haz the power, 2017-01-19 05:30:10]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/db06141ebf2aa70c3ffdad7a1121...