Show Compositor information in chrome://gpu

Show Compositor information in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
"Compositor information" displays "Tile Update Mode" and "Partial Raster" entry.

BUG=475633, 582438
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.

Committed: https://crrev.com/a2ee227780b0831c8fdf70cc60beda873b53ea19
Cr-Commit-Position: refs/heads/master@{#372549}

[dshwang, 2016-01-27 12:30:12]

Description was changed from

==========
Show Texture uploads mode in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
e.g. "Native GpuMemoryBuffers: Hardware accelerated"

"GpuMemoryBuffers Status" displays "Texture Uploads Mode" entry.
e.g. "Texture Uploads Mode: Software, Partial One-copy texture uploads"
     "Texture Uploads Mode: Native, Zero-copy texture uploads"

BUG=475633
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

to

==========
Show Texture uploads mode in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
e.g. "Native GpuMemoryBuffers: Hardware accelerated"

"GpuMemoryBuffers Status" displays "Texture Uploads Mode" entry.
e.g. "Texture Uploads Mode: Software, Partial One-copy texture uploads"
     "Texture Uploads Mode: Native, Zero-copy texture uploads"

BUG=475633
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

[dshwang, 2016-01-27 12:30:12]

dongseong.hwang@intel.com changed reviewers:
+ kbr@chromium.org, reveman@chromium.org, tiago.vignatti@intel.com

[dshwang, 2016-01-27 12:31:48]

kbr, reveman, could you review? It's follow-up CL of
https://codereview.chromium.org/1375663002

chrome://gpu shows only which buffer format and usage is supported. This CL
shows which texture upload mode is enabled. e.g. native zero-copy, software
partial one-copy, so on.

[inactive_dshwang_plz_cc_intel, 2016-01-27 20:02:45]

The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run

[inactive_dshwang_plz_cc_intel, 2016-01-27 20:03:22]

The CQ bit was unchecked by dongseong.hwang@chromium.org

[inactive_dshwang_plz_cc_intel, 2016-01-27 20:08:17]

The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-27 20:09:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1637423004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1637423004/1

[Ken Russell (switch to Gerrit), 2016-01-27 20:39:39]

I defer to reveman@ for review, though I'd like to comment that I think the
"GpuMemoryBuffer Status" section which is currently in about:gpu should be moved
below the version and driver information. It shouldn't be close to the first
thing the user sees upon visiting that page.

[dshwang, 2016-01-27 21:27:55]

On 2016/01/27 20:39:39, Ken Russell wrote:
> I defer to reveman@ for review, though I'd like to comment that I think the
> "GpuMemoryBuffer Status" section which is currently in about:gpu should be
moved
> below the version and driver information. It shouldn't be close to the first
> thing the user sees upon visiting that page.

I thought so. I'll move "GpuMemoryBuffer Status" section to below tomorrow
morning.

[commit-bot: I haz the power, 2016-01-27 21:45:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-27 21:45:07]

Dry run: This issue passed the CQ dry run.

[dshwang, 2016-01-28 09:23:00]

On 2016/01/27 21:27:55, dshwang wrote:
> On 2016/01/27 20:39:39, Ken Russell wrote:
> > I defer to reveman@ for review, though I'd like to comment that I think the
> > "GpuMemoryBuffer Status" section which is currently in about:gpu should be
> moved
> > below the version and driver information. It shouldn't be close to the first
> > thing the user sees upon visiting that page.
> 
> I thought so. I'll move "GpuMemoryBuffer Status" section to below tomorrow
> morning.

reveman, could you review?
I moved "GpuMemoryBuffer Status" section to below.

[reveman, 2016-01-29 15:43:50]

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/com...
File content/browser/gpu/compositor_util.cc (right):

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/com...
content/browser/gpu/compositor_util.cc:162: "Native GpuMemoryBuffer is not
supported.",
This is not necessarily true. As the default is still disabled, should we invert
this? "Native GpuMemoryBuffer are enabled"?

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_internals_ui.cc (right):

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:311: // Add texture uploads mode.
I'd prefer to call it "Tile Initialization Mode" as the use of textures might be
removed in some cases in the future.

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:313: if
(BrowserGpuMemoryBufferManager::IsNativeGpuMemoryBuffersEnabled()) {
This is going to be confusing as native being enabled doesn't mean it's used. It
depends on the platform, driver and what format is used. Is the GMB info below
not sufficient?

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:325: }
Can all this be two flags instead? 1. if zero-copy is enabled, 2. it partial
raster is enabled.

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:327:
gpu_memory_buffer_info->Append(NewDescriptionValuePair(
I don't think this belongs in gpu_memory_buffer_info. This is settings used by
the compositor for initializing tiles and not part of the GMB system.

[dshwang, 2016-01-29 16:46:14]

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/com...
File content/browser/gpu/compositor_util.cc (right):

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/com...
content/browser/gpu/compositor_util.cc:162: "Native GpuMemoryBuffer is not
supported.",
On 2016/01/29 15:43:50, reveman wrote:
> This is not necessarily true. As the default is still disabled, should we
invert
> this? "Native GpuMemoryBuffer are enabled"?

it's disabled_description.
"Problems Detected" in chrome://gpu shows as follows:
Native GpuMemoryBuffer is not supported.
Disabled Features: native_gpu_memory_buffers

Chrome Linux shows the disabled_description. I think it's correct description,
because it's why we don't turn it on in linux and windows.

let me copy&paste another example:
Accelerated rasterization has been disabled, either via about:flags or command
line.
Disabled Features: rasterization

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_internals_ui.cc (right):

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:311: // Add texture uploads mode.
On 2016/01/29 15:43:50, reveman wrote:
> I'd prefer to call it "Tile Initialization Mode" as the use of textures might
be
> removed in some cases in the future.

I feel a bit weird to "Initialization". one-copy and zero-copy determines how
raster pool updates tile, which affects initialization though.

How about "Tile Update Mode"?

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:313: if
(BrowserGpuMemoryBufferManager::IsNativeGpuMemoryBuffersEnabled()) {
On 2016/01/29 15:43:50, reveman wrote:
> This is going to be confusing as native being enabled doesn't mean it's used.
It
> depends on the platform, driver and what format is used.

It's true. If native is enabled but GMB info don't support any native format, a
user should understand native is not enabled.

or we can add logic that |texture_uploads_mode| is set to "Native, " only if
there is at least one native GMB format supported.

or we can add more complicated logic. query default tile format from cc, and
check the GBM format is supported.

What is better?

> Is the GMB info below
> not sufficient?

the GMB info below doesn't say whether chrome uses software one-copy, native
one-copy, or native zero-copy. the GMB info below says what native GBM format is
supported.

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:325: }
On 2016/01/29 15:43:50, reveman wrote:
> Can all this be two flags instead? 1. if zero-copy is enabled, 2. it partial
> raster is enabled.

currently chrome don't use partial raster even if using one-copy. I think you
want to know which one-copy or partial raster one-copy is enabled.

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:327:
gpu_memory_buffer_info->Append(NewDescriptionValuePair(
On 2016/01/29 15:43:50, reveman wrote:
> I don't think this belongs in gpu_memory_buffer_info. This is settings used by
> the compositor for initializing tiles and not part of the GMB system.

Ok, let me create new section: "Compositor Information"
Note it cannot belong to "Graphics Feature Status" because description must be
"Hardware accelerated", "Software only", or something.

[inactive_dshwang_plz_cc_intel, 2016-01-29 17:52:44]

The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-29 17:54:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1637423004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1637423004/40001

[dshwang, 2016-01-29 18:40:31]

In summary of new patch set
- Add "Compositor Information" section.
- Change "Texture upload mode" to "Tile Update Mode"

Now information is shown as follow:
Compositor Information
Tile Update Mode: Software, One-copy
Partial Raster: Disabled

About BrowserGpuMemoryBufferManager::IsNativeGpuMemoryBuffersEnabled(),
How about changing it to return true if at least one native format is supported.
https://codereview.chromium.org/1643293002

I think it's good for kEnableGpuMemoryBufferCompositorResources also.
It's not good to enable GpuMemoryBufferCompositorResources if at least one
native format isn't supported.
All GpuMemoryBuffer aren't backed on actual GL Image.
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/gp...

[commit-bot: I haz the power, 2016-01-29 19:16:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-29 19:16:20]

Dry run: This issue passed the CQ dry run.

[reveman, 2016-01-29 19:35:36]

My general recommendation is to keep these status fields simple and just match
the command line flags in combination with the default on the platform.

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_internals_ui.cc (right):

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:311: // Add texture uploads mode.
On 2016/01/29 at 16:46:14, dshwang wrote:
> On 2016/01/29 15:43:50, reveman wrote:
> > I'd prefer to call it "Tile Initialization Mode" as the use of textures
might be
> > removed in some cases in the future.
> 
> I feel a bit weird to "Initialization". one-copy and zero-copy determines how
raster pool updates tile, which affects initialization though.
> 
> How about "Tile Update Mode"?

Sgtm

https://codereview.chromium.org/1637423004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:313: if
(BrowserGpuMemoryBufferManager::IsNativeGpuMemoryBuffersEnabled()) {
On 2016/01/29 at 16:46:14, dshwang wrote:
> On 2016/01/29 15:43:50, reveman wrote:
> > This is going to be confusing as native being enabled doesn't mean it's
used. It
> > depends on the platform, driver and what format is used.
> 
> It's true. If native is enabled but GMB info don't support any native format,
a user should understand native is not enabled.
> 
> or we can add logic that |texture_uploads_mode| is set to "Native, " only if
there is at least one native GMB format supported.
> 
> or we can add more complicated logic. query default tile format from cc, and
check the GBM format is supported.
> 
> What is better?

These stats are more to help developers than users. I think it's fine having to
look at both GMB info and tile update info to determine what's actually used by
the compositor.

> 
> > Is the GMB info below
> > not sufficient?
> 
> the GMB info below doesn't say whether chrome uses software one-copy, native
one-copy, or native zero-copy. the GMB info below says what native GBM format is
supported.

Yes, and in combination with the value above you can determine how tiles are
updated.

[dshwang, 2016-01-29 19:40:09]

On 2016/01/29 19:35:36, reveman wrote:
> My general recommendation is to keep these status fields simple and just match
> the command line flags in combination with the default on the platform.
>  
> These stats are more to help developers than users. I think it's fine having
to
> look at both GMB info and tile update info to determine what's actually used
by
> the compositor.

Current patch set behaves as you explained. Do you think I need to change or
improve something?

[reveman, 2016-01-29 19:49:07]

https://codereview.chromium.org/1637423004/diff/40001/content/browser/gpu/com...
File content/browser/gpu/compositor_util.cc (right):

https://codereview.chromium.org/1637423004/diff/40001/content/browser/gpu/com...
content/browser/gpu/compositor_util.cc:162: "Native GpuMemoryBuffers is not
supported.",
nit: "Native GpuMemoryBuffers have been disabled, either via..." as this doesn't
represent if they are supported or not

https://codereview.chromium.org/1637423004/diff/40001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_internals_ui.cc (right):

https://codereview.chromium.org/1637423004/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:310: if
(BrowserGpuMemoryBufferManager::IsNativeGpuMemoryBuffersEnabled()) {
Don't include this here. This information is already made visible using
kNativeGpuMemoryBuffersFeatureName above.

[dshwang, 2016-01-29 19:58:40]

Thanks, Done.

https://codereview.chromium.org/1637423004/diff/40001/content/browser/gpu/com...
File content/browser/gpu/compositor_util.cc (right):

https://codereview.chromium.org/1637423004/diff/40001/content/browser/gpu/com...
content/browser/gpu/compositor_util.cc:162: "Native GpuMemoryBuffers is not
supported.",
On 2016/01/29 19:49:07, reveman wrote:
> nit: "Native GpuMemoryBuffers have been disabled, either via..." as this
doesn't
> represent if they are supported or not

Done.

https://codereview.chromium.org/1637423004/diff/40001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_internals_ui.cc (right):

https://codereview.chromium.org/1637423004/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_internals_ui.cc:310: if
(BrowserGpuMemoryBufferManager::IsNativeGpuMemoryBuffersEnabled()) {
On 2016/01/29 19:49:07, reveman wrote:
> Don't include this here. This information is already made visible using
> kNativeGpuMemoryBuffersFeatureName above.

Done.

[dshwang, 2016-01-29 20:00:01]

Patchset #4 (id:60001) has been deleted

[reveman, 2016-01-29 20:03:04]

lgtm

[dshwang, 2016-01-29 20:04:54]

Description was changed from

==========
Show Texture uploads mode in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
e.g. "Native GpuMemoryBuffers: Hardware accelerated"

"GpuMemoryBuffers Status" displays "Texture Uploads Mode" entry.
e.g. "Texture Uploads Mode: Software, Partial One-copy texture uploads"
     "Texture Uploads Mode: Native, Zero-copy texture uploads"

BUG=475633
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

to

==========
Show Compositor information in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
"Compositor information" displays "Tile Update Mode" and "Partial Raster" entry.

BUG=475633
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

[dshwang, 2016-01-29 20:05:16]

On 2016/01/29 20:03:04, reveman wrote:
> lgtm

Thank you for reviewing!

kbr, could you review again?

[dshwang, 2016-01-29 21:13:14]

Description was changed from

==========
Show Compositor information in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
"Compositor information" displays "Tile Update Mode" and "Partial Raster" entry.

BUG=475633
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

to

==========
Show Compositor information in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
"Compositor information" displays "Tile Update Mode" and "Partial Raster" entry.

BUG=475633, 582438
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

[Ken Russell (switch to Gerrit), 2016-01-30 01:59:42]

Thanks for the reorganization. lgtm

https://codereview.chromium.org/1637423004/diff/80001/content/browser/gpu/com...
File content/browser/gpu/compositor_util.cc (right):

https://codereview.chromium.org/1637423004/diff/80001/content/browser/gpu/com...
content/browser/gpu/compositor_util.cc:162: "Native GpuMemoryBuffers has been
disabled, either via about:flags"
has -> have

[dshwang, 2016-01-30 12:59:32]

On 2016/01/30 01:59:42, Ken Russell wrote:
> Thanks for the reorganization. lgtm

Thanks for reviewing!

[dshwang, 2016-01-30 12:59:37]

The CQ bit was checked by dongseong.hwang@intel.com

[dshwang, 2016-01-30 12:59:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/1637423004/#ps100001
(title: "has -> have")

[commit-bot: I haz the power, 2016-01-30 12:59:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1637423004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1637423004/100001

[commit-bot: I haz the power, 2016-01-30 14:12:32]

Description was changed from

==========
Show Compositor information in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
"Compositor information" displays "Tile Update Mode" and "Partial Raster" entry.

BUG=475633, 582438
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

to

==========
Show Compositor information in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
"Compositor information" displays "Tile Update Mode" and "Partial Raster" entry.

BUG=475633, 582438
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

[commit-bot: I haz the power, 2016-01-30 14:12:33]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-01-30 14:13:32]

Description was changed from

==========
Show Compositor information in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
"Compositor information" displays "Tile Update Mode" and "Partial Raster" entry.

BUG=475633, 582438
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.
==========

to

==========
Show Compositor information in chrome://gpu

"Graphics Feature Status" displays "Native GpuMemoryBuffers" entry.
"Compositor information" displays "Tile Update Mode" and "Partial Raster" entry.

BUG=475633, 582438
TEST=start chrome, open chrome://gpu on both desktop chrome and chrome os.

Committed: https://crrev.com/a2ee227780b0831c8fdf70cc60beda873b53ea19
Cr-Commit-Position: refs/heads/master@{#372549}
==========

[commit-bot: I haz the power, 2016-01-30 14:13:33]

Patchset 5 (id:??) landed as
https://crrev.com/a2ee227780b0831c8fdf70cc60beda873b53ea19
Cr-Commit-Position: refs/heads/master@{#372549}