Re-land: ui: Add custom stride support to GLImageMemory.

Re-land: ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests --gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/730fd9613f967271a1068c9c39b436fa13c35877
Cr-Commit-Position: refs/heads/master@{#358998}

[reveman, 2015-11-09 06:29:00]

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

[commit-bot: I haz the power, 2015-11-09 06:29:14]

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

[commit-bot: I haz the power, 2015-11-09 07:45:22]

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

[commit-bot: I haz the power, 2015-11-09 07:45:23]

Dry run: This issue passed the CQ dry run.

[reveman, 2015-11-09 13:04:33]

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

[piman, 2015-11-09 20:11:49]

lgtm

[reveman, 2015-11-09 20:16:46]

reveman@chromium.org changed reviewers:
+ fsamuel@chromium.org, kenrb@chromium.org

[reveman, 2015-11-09 20:16:46]

+fsamuel for components/mus/gles2/
+kenrb for content/common/child_process_messages.h

[Fady Samuel, 2015-11-10 05:42:18]

mus lgtm

[reveman, 2015-11-10 07:36:40]

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

[commit-bot: I haz the power, 2015-11-10 07:36:57]

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

[commit-bot: I haz the power, 2015-11-10 09:16:37]

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

[commit-bot: I haz the power, 2015-11-10 09:16:37]

Dry run: This issue passed the CQ dry run.

[reveman, 2015-11-10 15:55:26]

reveman@chromium.org changed reviewers:
+ wfh@chromium.org
- kenrb@chromium.org

[reveman, 2015-11-10 15:55:27]

+wfh for content/common/child_process_messages.h

[Will Harris, 2015-11-10 16:19:12]

we chatted in person about the size_t vs int32_t for stride and I'm satisfied
that these conversions are being handled correctly. lgtm on
content/common/child_process_messages.h

[reveman, 2015-11-10 16:20:20]

The CQ bit was checked by reveman@chromium.org

[reveman, 2015-11-10 16:20:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
fsamuel@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1403283007/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2015-11-10 16:20:38]

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

[commit-bot: I haz the power, 2015-11-10 16:25:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-11-10 16:26:36]

Patchset 3 (id:??) landed as
https://crrev.com/8b0062c4a359ee651b7fb23707bf3aaf20162990
Cr-Commit-Position: refs/heads/master@{#358835}

[Avi (use Gerrit), 2015-11-10 16:42:15]

avi@chromium.org changed reviewers:
+ avi@chromium.org

[Avi (use Gerrit), 2015-11-10 16:42:16]

https://codereview.chromium.org/1403283007/diff/40001/ui/gl/gl_image_shared_m...
File ui/gl/gl_image_shared_memory_unittest.cc (right):

https://codereview.chromium.org/1403283007/diff/40001/ui/gl/gl_image_shared_m...
ui/gl/gl_image_shared_memory_unittest.cc:74: size.width(), size.height(),
stride, gfx::BufferFormat::RGBA_8888,
This fails on Win x64 Builder (dbg) in
http://build.chromium.org/p/chromium.win/builders/Win%20x64%20Builder%20%28db...

c:\b\build\slave\win_x64_builder__dbg_\build\src\ui\gl\gl_image_shared_memory_unittest.cc(76)
:error C2220: warning treated as error - no 'object' file generated
c:\b\build\slave\win_x64_builder__dbg_\build\src\ui\gl\gl_image_shared_memory_unittest.cc(76)
: warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss
of data

[Avi (use Gerrit), 2015-11-10 16:42:53]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1427413004/ by avi@chromium.org.

The reason for reverting is: Broke Win x64 Builder (dbg).

[Will Harris, 2015-11-10 16:45:50]

hmm there goes my security street cred for spotting type conversion issues. :(

[Peter Mayo, 2015-11-10 16:47:52]

petermayo@chromium.org changed reviewers:
+ petermayo@chromium.org

[Peter Mayo, 2015-11-10 16:47:53]

https://codereview.chromium.org/1403283007/diff/40001/ui/gl/gl_image_shared_m...
File ui/gl/gl_image_shared_memory.cc (right):

https://codereview.chromium.org/1403283007/diff/40001/ui/gl/gl_image_shared_m...
ui/gl/gl_image_shared_memory.cc:74: format, stride)) {
FAILED: ninja -t msvc -e environment.x64 -- C:\b\build\goma/gomacc
"C:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64\cl.exe" /nologo
/showIncludes /FC
@obj\ui\gl\gl_unittests.gl_image_shared_memory_unittest.obj.rsp /c
..\..\ui\gl\gl_image_shared_memory_unittest.cc
/Foobj\ui\gl\gl_unittests.gl_image_shared_memory_unittest.obj
/Fdobj\ui\gl\gl_unittests.cc.pdb 
c:\b\build\slave\win_x64_builder__dbg_\build\src\ui\gl\gl_image_shared_memory_unittest.cc(76)
: error C2220: warning treated as error - no 'object' file generated
c:\b\build\slave\win_x64_builder__dbg_\build\src\ui\gl\gl_image_shared_memory_unittest.cc(76)
: warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss
of data
ninja: build stopped: subcommand failed.


on
https://build.chromium.org/p/chromium.win/builders/Win%20x64%20Builder%20%28d...

[Peter Mayo, 2015-11-10 16:48:27]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1437623003/ by petermayo@chromium.org.

The reason for reverting is: int versus size_t problem on Windows.

Please correct and re-land
.

[reveman, 2015-11-10 16:51:52]

Description was changed from

==========
ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests
--gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/8b0062c4a359ee651b7fb23707bf3aaf20162990
Cr-Commit-Position: refs/heads/master@{#358835}
==========

to

==========
ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests
--gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[reveman, 2015-11-10 16:52:08]

Description was changed from

==========
ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests
--gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Re-land: ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests
--gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[reveman, 2015-11-10 17:02:58]

The CQ bit was checked by reveman@chromium.org

[reveman, 2015-11-10 17:02:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
fsamuel@chromium.org, piman@chromium.org, wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/1403283007/#ps60001
(title: "add static_cast")

[commit-bot: I haz the power, 2015-11-10 17:03:20]

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

[commit-bot: I haz the power, 2015-11-10 18:13:00]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-10 18:13:47]

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

[Avi (use Gerrit), 2015-11-10 18:29:08]

https://codereview.chromium.org/1403283007/diff/60001/components/mus/gles2/mo...
File components/mus/gles2/mojo_gpu_memory_buffer.cc (right):

https://codereview.chromium.org/1403283007/diff/60001/components/mus/gles2/mo...
components/mus/gles2/mojo_gpu_memory_buffer.cc:90: handle.stride =
gfx::RowSizeForBufferFormat(size_.width(), format_, 0);
e:\b\build\slave\win_x64_gn__dbg_\build\src\components\mus\gles2\mojo_gpu_memory_buffer.cc(90)
:error C2220: warning treated as error - no 'object' file generated
e:\b\build\slave\win_x64_gn__dbg_\build\src\components\mus\gles2\mojo_gpu_memory_buffer.cc(90)
: warning C4267: '=' : conversion from 'size_t' to 'int32_t', possible loss of
data

[Avi (use Gerrit), 2015-11-10 18:29:29]

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

The reason for reverting is: Still failing to compile..

[reveman, 2015-11-10 18:41:25]

Description was changed from

==========
Re-land: ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests
--gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

to

==========
Re-land: ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests
--gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[robliao, 2015-11-10 18:44:41]

Description was changed from

==========
Re-land: ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests
--gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Re-land: ui: Add custom stride support to GLImageMemory.

This improves our support for shared memory pools by allowing
clients to create GLImages with a stride that is larger than
the row length of the image.

This makes it possible to import a buffer with a stride that
doesn't match the aligned row size. However, support for
mapping of such a buffer is not provided by this patch.

BUG=549781
TEST=gl_unittests
--gtest_filter=GLImageSharedMemoryPool/GLImageCopyTest/0.CopyTexImage
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[robliao, 2015-11-10 18:44:41]

robliao@chromium.org changed reviewers:
+ robliao@chromium.org

[robliao, 2015-11-10 19:00:13]

On 2015/11/10 18:29:29, Avi wrote:
> A revert of this CL (patchset #4 id:60001) has been created in
> https://codereview.chromium.org/1431333002/ by mailto:avi@chromium.org.
> 
> The reason for reverting is: Still failing to compile..

FYI, it appears the CQ doesn't run Win GN, which is what builds
mojo_gpu_memory_buffer.cc. I've added win_chromium_gn_x64_dbg to the previous
patchset to see if that fails similarly.

[reveman, 2015-11-10 19:08:10]

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

[commit-bot: I haz the power, 2015-11-10 19:08:39]

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

[robliao, 2015-11-10 19:15:26]

On 2015/11/10 19:08:39, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1403283007/80001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1403283007/80001

For reference http://crbug.com/486089 tracks the lack of GN in CQ.

[commit-bot: I haz the power, 2015-11-10 20:24:21]

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

[commit-bot: I haz the power, 2015-11-10 20:24:22]

Dry run: This issue passed the CQ dry run.

[reveman, 2015-11-11 01:07:03]

On 2015/11/10 at 20:24:22, commit-bot wrote:
> Dry run: This issue passed the CQ dry run.

I looked through the patch and I'm relatively sure that this is not going to
cause any windows build errors anymore. Re-landing and keeping an eye on the
bots.

[reveman, 2015-11-11 01:07:16]

The CQ bit was checked by reveman@chromium.org

[reveman, 2015-11-11 01:07:17]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org,
fsamuel@chromium.org, wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/1403283007/#ps80001
(title: "more missing static_casts")

[commit-bot: I haz the power, 2015-11-11 01:09:31]

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

[commit-bot: I haz the power, 2015-11-11 01:22:51]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-11-11 01:24:02]

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