Add workaround for glTex(Sub)Image3D on Intel Macs

Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

Work around this issue by uploading texture layer by layer.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[jiajia.qin, 2016-10-14 08:15:15]

Description was changed from

==========
Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
==========

to

==========
Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[jiajia.qin, 2016-10-14 08:17:47]

The CQ bit was checked by jiajia.qin@intel.com

[jiajia.qin, 2016-10-14 08:17:48]

The CQ bit was unchecked by jiajia.qin@intel.com

[jiajia.qin, 2016-10-14 08:17:48]

The CQ bit was checked by jiajia.qin@intel.com

[jiajia.qin, 2016-10-14 08:17:50]

The CQ bit was unchecked by jiajia.qin@intel.com

[jiajia.qin, 2016-10-14 08:17:55]

The CQ bit was unchecked by jiajia.qin@intel.com

[jiajia.qin, 2016-10-14 09:03:12]

jiajia.qin@intel.com changed reviewers:
+ kbr@chromium.org, qiankun.miao@intel.com, zmo@chromium.org

[jiajia.qin, 2016-10-14 09:03:13]

Please take a look. Thanks.

[qiankun, 2016-10-14 09:06:51]

It's a similar work to https://codereview.chromium.org/1963683002/.

[jiajia.qin, 2016-10-14 09:41:08]

Description was changed from

==========
Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

Work around this issue by uploading texture layer by layer.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[jiajia.qin, 2016-10-14 09:41:08]

jiajia.qin@intel.com changed reviewers:
- kbr@chromium.org, qiankun.miao@intel.com, zmo@chromium.org

[jiajia.qin, 2016-10-14 09:50:50]

Patchset #2 (id:20001) has been deleted

[jiajia.qin, 2016-10-14 09:53:09]

Patchset #1 (id:1) has been deleted

[jiajia.qin, 2016-10-14 09:53:17]

Patchset #1 (id:40001) has been deleted

[jiajia.qin, 2016-10-14 09:53:24]

Patchset #1 (id:60001) has been deleted

[jiajia.qin, 2016-10-14 09:56:54]

jiajia.qin@intel.com changed reviewers:
+ kbr@google.com, qiankun.miao@intel.com, zmo@chromium.org

[Zhenyao Mo, 2016-10-14 21:29:09]

Mostly looks good with minor optimization suggestion.

https://codereview.chromium.org/2421823002/diff/80001/gpu/command_buffer/serv...
File gpu/command_buffer/service/texture_manager.cc (right):

https://codereview.chromium.org/2421823002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/texture_manager.cc:3035: const GLubyte* image_pixels
=
You can move "const GLubyte* image_pixels = cast<const GLubyte*>(args.pixels);"
out of the loop, and the above two lines can be changed to "image_pixels +=
image_bytes;"

https://codereview.chromium.org/2421823002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/texture_manager.cc:3049: const GLubyte* row_pixels =
last_image_pixels + byte_offset;
You can move "const GLubyte* row_pixels = image_pixels + image_bytes;" out of
the loop, and the above two lines can be changed to "row_pixels += row_bytes;"

[jiajia.qin, 2016-10-17 05:38:57]

Mo, please take another look. Thanks.

https://codereview.chromium.org/2421823002/diff/80001/gpu/command_buffer/serv...
File gpu/command_buffer/service/texture_manager.cc (right):

https://codereview.chromium.org/2421823002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/texture_manager.cc:3035: const GLubyte* image_pixels
=
On 2016/10/14 21:29:09, Zhenyao Mo wrote:
> You can move "const GLubyte* image_pixels = cast<const
GLubyte*>(args.pixels);"
> out of the loop, and the above two lines can be changed to "image_pixels +=
> image_bytes;"

Done.

https://codereview.chromium.org/2421823002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/texture_manager.cc:3049: const GLubyte* row_pixels =
last_image_pixels + byte_offset;
On 2016/10/14 21:29:09, Zhenyao Mo wrote:
> You can move "const GLubyte* row_pixels = image_pixels + image_bytes;" out of
> the loop, and the above two lines can be changed to "row_pixels += row_bytes;"

Done.

[Zhenyao Mo, 2016-10-17 18:03:46]

lgtm

https://codereview.chromium.org/2421823002/diff/100001/gpu/command_buffer/ser...
File gpu/command_buffer/service/texture_manager.cc (right):

https://codereview.chromium.org/2421823002/diff/100001/gpu/command_buffer/ser...
gpu/command_buffer/service/texture_manager.cc:3053: // Restore unpack state
nit: comment move one line up

[jiajia.qin, 2016-10-17 23:02:52]

The CQ bit was checked by jiajia.qin@intel.com

[jiajia.qin, 2016-10-17 23:02:53]

The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/2421823002/#ps120001
(title: "rebase and fix a nit issue")

[commit-bot: I haz the power, 2016-10-17 23:03:39]

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, 2016-10-18 02:04:42]

Description was changed from

==========
Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

Work around this issue by uploading texture layer by layer.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

Work around this issue by uploading texture layer by layer.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-10-18 02:04:44]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2016-10-18 02:08:01]

Description was changed from

==========
Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

Work around this issue by uploading texture layer by layer.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add workaround for glTex(Sub)Image3D on Intel Macs

On macOS machines with Intel GPUs, glTexImage3D/glTexSubImage3D performs
incorrectly when uploading a texture from the bound GL_PIXEL_UNPACK_BUFFER
and glPixelStorei has set the GL_UNPACK_IMAGE_HEIGHT parameter to be
greater than zero but not equal to the texture's height. It seems that
glTexImage3D/glTexSubImage3D uses the texture's height, and not
GL_UNPACK_IMAGE_HEIGHT's value, during the upload.

Work around this issue by uploading texture layer by layer.

BUG=654258
TEST=WebGL 2 conformance tests
     conformance2/textures/misc/tex-unpack-params.html
     deqp/functional/gles3/texturespecification/teximage3d_pbo_params.html
     deqp/functional/gles3/texturespecification/texsubimage3d_pbo_params.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[commit-bot: I haz the power, 2016-10-18 02:08:02]

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