Implemented copyTexSubImage3D WebGL2 entry point in Blink

Implemented copyTexSubImage3D WebGL2 entry point in Blink

BUG=295792
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201335

[bajones, 2015-08-26 21:32:48]

bajones@chromium.org changed reviewers:
+ kbr@chromium.org, zmo@chromium.org

[bajones, 2015-08-26 21:32:49]

PTAL. Conformance test is incoming. Need some command-buffer fixes to handle
initializing to zero before it will pass.

[Zhenyao Mo, 2015-08-26 23:44:38]

Looks good mostly with one small issue.

https://codereview.chromium.org/1318853003/diff/1/Source/modules/webgl/WebGLR...
File Source/modules/webgl/WebGLRenderingContextBase.cpp (right):

https://codereview.chromium.org/1318853003/diff/1/Source/modules/webgl/WebGLR...
Source/modules/webgl/WebGLRenderingContextBase.cpp:5646: if
(!isTexInternalFormatColorBufferCombinationValid(internalformat,
boundFramebufferColorFormat())) {
I don't think boundFramebufferColorFormat() does the right thing.  It always
obtain COLOR_ATTACHMENT0, not the read_buffer setting.

[Ken Russell (switch to Gerrit), 2015-08-27 02:01:03]

LGTM with TODO added (or, ideally, bug fixed, too -- though doesn't have to be
in this CL)

https://codereview.chromium.org/1318853003/diff/1/Source/modules/webgl/WebGLR...
File Source/modules/webgl/WebGLRenderingContextBase.cpp (right):

https://codereview.chromium.org/1318853003/diff/1/Source/modules/webgl/WebGLR...
Source/modules/webgl/WebGLRenderingContextBase.cpp:5646: if
(!isTexInternalFormatColorBufferCombinationValid(internalformat,
boundFramebufferColorFormat())) {
On 2015/08/26 23:44:38, Zhenyao Mo wrote:
> I don't think boundFramebufferColorFormat() does the right thing.  It always
> obtain COLOR_ATTACHMENT0, not the read_buffer setting.

Agreed, boundFramebufferColorFormat looks like it isn't taking into
consideration the ReadBuffer setting, and WebGLFramebuffer::colorBufferFormat
shouldn't be assuming GL_COLOR_ATTACHMENT0. Thanks for catching this.

Brandon, could you please file a bug and add a TODO? We'll need a WebGL
conformance test for this.

[bajones, 2015-08-27 18:37:25]

On 2015/08/27 02:01:03, Ken Russell wrote:
> LGTM with TODO added (or, ideally, bug fixed, too -- though doesn't have to be
> in this CL)
> 
>
https://codereview.chromium.org/1318853003/diff/1/Source/modules/webgl/WebGLR...
> File Source/modules/webgl/WebGLRenderingContextBase.cpp (right):
> 
>
https://codereview.chromium.org/1318853003/diff/1/Source/modules/webgl/WebGLR...
> Source/modules/webgl/WebGLRenderingContextBase.cpp:5646: if
> (!isTexInternalFormatColorBufferCombinationValid(internalformat,
> boundFramebufferColorFormat())) {
> On 2015/08/26 23:44:38, Zhenyao Mo wrote:
> > I don't think boundFramebufferColorFormat() does the right thing.  It always
> > obtain COLOR_ATTACHMENT0, not the read_buffer setting.
> 
> Agreed, boundFramebufferColorFormat looks like it isn't taking into
> consideration the ReadBuffer setting, and WebGLFramebuffer::colorBufferFormat
> shouldn't be assuming GL_COLOR_ATTACHMENT0. Thanks for catching this.
> 
> Brandon, could you please file a bug and add a TODO? We'll need a WebGL
> conformance test for this.

boundFramebufferColorFormat() is already overridden in WebGL2RenderingContext to
return the format of the ReadBuffer.

[bajones, 2015-08-27 18:38:02]

The CQ bit was checked by bajones@chromium.org

[commit-bot: I haz the power, 2015-08-27 18:38:14]

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

[Ken Russell (switch to Gerrit), 2015-08-27 18:44:41]

On 2015/08/27 18:37:25, bajones wrote:
> On 2015/08/27 02:01:03, Ken Russell wrote:
> > LGTM with TODO added (or, ideally, bug fixed, too -- though doesn't have to
be
> > in this CL)
> > 
> >
>
https://codereview.chromium.org/1318853003/diff/1/Source/modules/webgl/WebGLR...
> > File Source/modules/webgl/WebGLRenderingContextBase.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1318853003/diff/1/Source/modules/webgl/WebGLR...
> > Source/modules/webgl/WebGLRenderingContextBase.cpp:5646: if
> > (!isTexInternalFormatColorBufferCombinationValid(internalformat,
> > boundFramebufferColorFormat())) {
> > On 2015/08/26 23:44:38, Zhenyao Mo wrote:
> > > I don't think boundFramebufferColorFormat() does the right thing.  It
always
> > > obtain COLOR_ATTACHMENT0, not the read_buffer setting.
> > 
> > Agreed, boundFramebufferColorFormat looks like it isn't taking into
> > consideration the ReadBuffer setting, and
WebGLFramebuffer::colorBufferFormat
> > shouldn't be assuming GL_COLOR_ATTACHMENT0. Thanks for catching this.
> > 
> > Brandon, could you please file a bug and add a TODO? We'll need a WebGL
> > conformance test for this.
> 
> boundFramebufferColorFormat() is already overridden in WebGL2RenderingContext
to
> return the format of the ReadBuffer.

WebGL2RenderingContextBase::boundFramebufferColorFormat is doing the wrong
thing. It's returning the format of COLOR_ATTACHMENT0 of the bound read
framebuffer object, rather than COLOR_ATTACHMENTi where ReadBuffer specified the
i.

[commit-bot: I haz the power, 2015-08-27 20:33:38]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201335