Move NumberOfPlanesForGpuMemoryBufferFormat to gfx

Move NumberOfPlanesForGpuMemoryBufferFormat to gfx

We will need to call it from media::GpuMemoryBufferVideoFramePool when we
add support for YUV_420_BIPLANAR.

BUG=510260
TEST=No behavior change, only moving code.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/85f29861d095a7c5dcf318053e8349d1efaddbab
Cr-Commit-Position: refs/heads/master@{#342942}

Committed: https://crrev.com/42febcc1b22c086482951ae830e2e3f785882dfc
Cr-Commit-Position: refs/heads/master@{#343053}

[Andre, 2015-08-07 23:53:47]

Patchset #2 (id:20001) has been deleted

[Andre, 2015-08-07 23:55:54]

Patchset #1 (id:1) has been deleted

[Andre, 2015-08-10 17:04:03]

andresantoso@chromium.org changed reviewers:
+ ccameron@chromium.org

[Andre, 2015-08-10 17:04:04]

PTAL

[Andre, 2015-08-10 17:46:30]

Patchset #3 (id:80001) has been deleted

[Andre, 2015-08-10 19:05:54]

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

[commit-bot: I haz the power, 2015-08-10 19:06:30]

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

[commit-bot: I haz the power, 2015-08-10 20:39:13]

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

[commit-bot: I haz the power, 2015-08-10 20:39:14]

Dry run: This issue passed the CQ dry run.

[Andre, 2015-08-10 22:56:51]

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

[Andre, 2015-08-10 22:56:52]

reveman@ PTAL

[reveman, 2015-08-10 23:08:28]

https://codereview.chromium.org/1281043006/diff/100001/ui/gfx/gpu_memory_buff...
File ui/gfx/gpu_memory_buffer.h (right):

https://codereview.chromium.org/1281043006/diff/100001/ui/gfx/gpu_memory_buff...
ui/gfx/gpu_memory_buffer.h:49: static size_t NumberOfPlanes(BufferFormat
format);
How about we add a gpu_memory_buffer_util.h file for this type of stuff?

The reason I prefer that is that it makes it more clear that it's just an
utility function and not something that has to be used. The format definition
itself includes all the information needed and these type of utility functions
are really just needed to write type generic code in a clean way. Most usage of
the gfx::GpuMemoryBuffer interface doesn't need it.

[Andre, 2015-08-11 03:50:04]

PTAL

https://codereview.chromium.org/1281043006/diff/100001/ui/gfx/gpu_memory_buff...
File ui/gfx/gpu_memory_buffer.h (right):

https://codereview.chromium.org/1281043006/diff/100001/ui/gfx/gpu_memory_buff...
ui/gfx/gpu_memory_buffer.h:49: static size_t NumberOfPlanes(BufferFormat
format);
On 2015/08/10 23:08:28, reveman (OOO Aug 10 - Aug 11) wrote:
> How about we add a gpu_memory_buffer_util.h file for this type of stuff?
> 
> The reason I prefer that is that it makes it more clear that it's just an
> utility function and not something that has to be used. The format definition
> itself includes all the information needed and these type of utility functions
> are really just needed to write type generic code in a clean way. Most usage
of
> the gfx::GpuMemoryBuffer interface doesn't need it.

Done.

[reveman, 2015-08-11 07:39:57]

lgtm

https://codereview.chromium.org/1281043006/diff/120001/ui/gfx/buffer_format_u...
File ui/gfx/buffer_format_util.h (right):

https://codereview.chromium.org/1281043006/diff/120001/ui/gfx/buffer_format_u...
ui/gfx/buffer_format_util.h:16: 
Would be nice to also move other utility functions (SubsamplingFactor,
RowSizeInBytes and BufferSizeInBytes) here. Either as part of this patch or as a
follow up.

[Andre, 2015-08-11 16:56:39]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2015-08-11 16:57:07]

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

[Andre, 2015-08-11 17:00:22]

Thanks. I can look into the other functions in a separate patch.
There are a few copies of those functions around. Do you prefer the ones the
return the value directly or
the ones that take an out parameter and return bool?
I think the ones that return the value directly is simpler to deal with, and for
the ones that return bool,
the call sites usually doesn't do much other than DCHECK the returned bool.
Better to keep it simple and
keep the DCHECKs inside the functions itself. WDYT?

[Andre, 2015-08-11 17:00:45]

The CQ bit was unchecked by andresantoso@chromium.org

[Andre, 2015-08-11 17:10:10]

andresantoso@chromium.org changed reviewers:
+ avi@chromium.org, jbauman@chromium.org, msw@chromium.org

[Andre, 2015-08-11 17:10:10]

+msw for components/view_manager.
+jbauman for gpu/command_buffer.
+avi for content/child and ui/gfx.

[Avi (use Gerrit), 2015-08-11 17:15:14]

Those LGTM

[msw, 2015-08-11 17:25:30]

components/view_manager/* lgtm

[reveman, 2015-08-11 17:46:36]

On 2015/08/11 at 17:00:22, andresantoso wrote:
> Thanks. I can look into the other functions in a separate patch.
> There are a few copies of those functions around. Do you prefer the ones the
return the value directly or
> the ones that take an out parameter and return bool?
> I think the ones that return the value directly is simpler to deal with, and
for the ones that return bool,
> the call sites usually doesn't do much other than DCHECK the returned bool.
Better to keep it simple and
> keep the DCHECKs inside the functions itself. WDYT?

The "checked" versions returning a bool are required on the browser side to
protect against a malicious renderer so I think those are needed. Maybe follow
the pattern in cc/resources/resource_util.h and provide both a Checked and an
Unchecked version of each function.

[jbauman, 2015-08-11 23:32:53]

gpu/command_buffer lgtm

[Andre, 2015-08-11 23:59:13]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2015-08-11 23:59:30]

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

[commit-bot: I haz the power, 2015-08-12 00:04:30]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2015-08-12 00:05:03]

Patchset 4 (id:??) landed as
https://crrev.com/85f29861d095a7c5dcf318053e8349d1efaddbab
Cr-Commit-Position: refs/heads/master@{#342942}

[Ken Rockot(use gerrit already), 2015-08-12 00:39:04]

A revert of this CL (patchset #4 id:120001) has been created in
https://codereview.chromium.org/1291643002/ by rockot@chromium.org.

The reason for reverting is: This breaks Win 64 builds:

http://build.chromium.org/p/chromium.win/builders/Win%20x64%20GN

.

[Andre, 2015-08-12 17:08:04]

Patchset #7 (id:180001) has been deleted

[Andre, 2015-08-12 17:08:13]

The CQ bit was checked by andresantoso@chromium.org

[Andre, 2015-08-12 17:08:14]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org,
avi@chromium.org, reveman@chromium.org, jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/1281043006/#ps160001
(title: "Rebase")

[commit-bot: I haz the power, 2015-08-12 17:08:42]

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

[commit-bot: I haz the power, 2015-08-12 18:11:38]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2015-08-12 18:12:11]

Patchset 6 (id:??) landed as
https://crrev.com/42febcc1b22c086482951ae830e2e3f785882dfc
Cr-Commit-Position: refs/heads/master@{#343053}