diff options
author | zmo <zmo@chromium.org> | 2016-02-03 16:48:58 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-02-04 00:50:29 +0000 |
commit | c55a231a0021c476aec4b684dd88145870d6c03d (patch) | |
tree | 6ad96393bcb0064cba4272803dab4bf8e2d690c0 | |
parent | 0860565c23a4b67e53bd6f315fa7e74208766d6e (diff) | |
download | chromium_src-c55a231a0021c476aec4b684dd88145870d6c03d.zip chromium_src-c55a231a0021c476aec4b684dd88145870d6c03d.tar.gz chromium_src-c55a231a0021c476aec4b684dd88145870d6c03d.tar.bz2 |
Remove more validation related with fbo completeness and ReadPixels.
BUG=570453
TEST=webgl_conformance,webgl2_conformance
R=kbr@chromium.org,bajones@chromium.org
NOTRY=true
Review URL: https://codereview.chromium.org/1658703003
Cr-Commit-Position: refs/heads/master@{#373400}
10 files changed, 126 insertions, 192 deletions
diff --git a/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py b/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py index 3a062fc..35a86c1 100644 --- a/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py +++ b/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py @@ -84,6 +84,12 @@ class WebGL2ConformanceExpectations(WebGLConformanceExpectations): # Note that this test fails on ['win', 'intel'] with bug=483282 self.Fail('conformance2/buffers/uniform-buffers.html', bug=577368) + # Remove the following after roll in ToT WebGL conformance tests. + self.Fail('conformance2/reading/read-pixels-into-pixel-pack-buffer.html', + ['mac', 'linux'], bug=570453) + self.Fail('conformance2/reading/read-pixels-into-pixel-pack-buffer.html', + ['win', 'release'], bug=570453) + # Windows only. self.Fail('conformance2/textures/canvas/tex-image-and-sub-image-2d' + '-with-canvas-r8-red-unsigned_byte.html', diff --git a/gpu/command_buffer/build_gles2_cmd_buffer.py b/gpu/command_buffer/build_gles2_cmd_buffer.py index 62b8690..52cbed15 100755 --- a/gpu/command_buffer/build_gles2_cmd_buffer.py +++ b/gpu/command_buffer/build_gles2_cmd_buffer.py @@ -1628,6 +1628,9 @@ _NAMED_TYPE_INFO = { 'GL_RGB_INTEGER', 'GL_RGBA_INTEGER', ], + 'deprecated_es3': [ + 'GL_ALPHA', + ], }, 'PixelType': { 'type': 'GLenum', diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc index 5033e94..a53b9b0 100644 --- a/gpu/command_buffer/client/gles2_implementation.cc +++ b/gpu/command_buffer/client/gles2_implementation.cc @@ -3521,9 +3521,6 @@ void GLES2Implementation::ReadPixels( SetGLError(GL_INVALID_VALUE, "glReadPixels", "dimensions < 0"); return; } - if (width == 0 || height == 0) { - return; - } // glReadPixel pads the size of each row of pixels by an amount specified by // glPixelStorei. So, we have to take that into account both in the fact that @@ -3612,8 +3609,10 @@ void GLES2Implementation::ReadPixels( if (xoffset < 0) { skip_row_bytes = static_cast<uint32_t>(-xoffset) * group_size; } - while (remaining_rows) { - GLsizei desired_size = + do { + // Even if height == 0, we still need to trigger the service side handling + // in case invalid args are passed in and a GL errro needs to be generated. + GLsizei desired_size = remaining_rows == 0 ? 0 : service_padded_row_size * (remaining_rows - 1) + unpadded_row_size; ScopedTransferBufferPtr buffer(desired_size, helper_, transfer_buffer_); if (!buffer.valid()) { @@ -3641,6 +3640,9 @@ void GLES2Implementation::ReadPixels( if (!result->success) { break; } + if (remaining_rows == 0) { + break; + } const uint8_t* src = static_cast<const uint8_t*>(buffer.address()); if (padded_row_size == unpadded_row_size && (pack_row_length_ == 0 || pack_row_length_ == width) && @@ -3676,7 +3678,7 @@ void GLES2Implementation::ReadPixels( } y_index += num_rows; remaining_rows -= num_rows; - } + } while (remaining_rows); CheckGLError(); } diff --git a/gpu/command_buffer/common/gles2_cmd_utils.cc b/gpu/command_buffer/common/gles2_cmd_utils.cc index 4c7fcf7..0011053 100644 --- a/gpu/command_buffer/common/gles2_cmd_utils.cc +++ b/gpu/command_buffer/common/gles2_cmd_utils.cc @@ -1166,6 +1166,8 @@ uint32_t GLES2Util::GetChannelsNeededForAttachmentType( return kDepth; case GL_STENCIL_ATTACHMENT: return kStencil; + case GL_DEPTH_STENCIL_ATTACHMENT: + return kDepth | kStencil; default: if (type >= GL_COLOR_ATTACHMENT0 && type < static_cast<int>( diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc index d19a491..ed487d0 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc @@ -1251,14 +1251,6 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient { // Returns: true if glEnable/glDisable should actually be called. bool SetCapabilityState(GLenum cap, bool enabled); - // Check that the currently bound framebuffers are valid. - // Generates GL error if not. - bool CheckBoundFramebuffersValid(const char* func_name); - - // Check that the currently bound read framebuffer has a color image - // attached. Generates GL error if not. - bool CheckBoundReadFramebufferColorAttachment(const char* func_name); - // Infer color encoding from internalformat static GLint GetColorEncodingFromInternalFormat(GLenum internalformat); @@ -1270,9 +1262,12 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient { bool CheckFramebufferValid( Framebuffer* framebuffer, GLenum target, + bool clear_uncleared_images, const char* func_name); - bool CheckBoundDrawFramebufferValid(const char* func_name); + bool CheckBoundDrawFramebufferValid( + bool clear_uncleared_images, const char* func_name); + bool CheckBoundReadFramebufferValid(const char* func_name); // Check if the current valuebuffer exists and is valid. If not generates // the appropriate GL error. Returns true if the current valuebuffer is in @@ -3653,11 +3648,13 @@ void GLES2DecoderImpl::RestoreCurrentFramebufferBindings() { bool GLES2DecoderImpl::CheckFramebufferValid( Framebuffer* framebuffer, - GLenum target, const char* func_name) { + GLenum target, + bool clear_uncleared_images, + const char* func_name) { if (!framebuffer) { if (surfaceless_) return false; - if (backbuffer_needs_clear_bits_) { + if (backbuffer_needs_clear_bits_ && clear_uncleared_images) { glClearColor(0, 0, 0, BackBufferHasAlpha() ? 0 : 1.f); state_.SetDeviceColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); glClearStencil(0); @@ -3698,8 +3695,9 @@ bool GLES2DecoderImpl::CheckFramebufferValid( } // Are all the attachments cleared? - if (renderbuffer_manager()->HaveUnclearedRenderbuffers() || - texture_manager()->HaveUnclearedMips()) { + if (clear_uncleared_images && + (renderbuffer_manager()->HaveUnclearedRenderbuffers() || + texture_manager()->HaveUnclearedMips())) { if (!framebuffer->IsCleared()) { // Can we clear them? if (framebuffer->GetStatus(texture_manager(), target) != @@ -3723,63 +3721,27 @@ bool GLES2DecoderImpl::CheckFramebufferValid( } framebuffer_manager()->MarkAsComplete(framebuffer); } - - // NOTE: At this point we don't know if the framebuffer is complete but - // we DO know that everything that needs to be cleared has been cleared. return true; } -bool GLES2DecoderImpl::CheckBoundFramebuffersValid(const char* func_name) { - if (!features().chromium_framebuffer_multisample) { - bool valid = CheckFramebufferValid( - framebuffer_state_.bound_draw_framebuffer.get(), GL_FRAMEBUFFER_EXT, - func_name); - - if (valid) - OnUseFramebuffer(); - - return valid; - } - return CheckFramebufferValid(framebuffer_state_.bound_draw_framebuffer.get(), - GL_DRAW_FRAMEBUFFER_EXT, - func_name) && - CheckFramebufferValid(framebuffer_state_.bound_read_framebuffer.get(), - GL_READ_FRAMEBUFFER_EXT, - func_name); -} - -bool GLES2DecoderImpl::CheckBoundDrawFramebufferValid(const char* func_name) { - Framebuffer* framebuffer = framebuffer_state_.bound_draw_framebuffer.get(); - if (!framebuffer) { - // Assume the default back buffer is always complete. - return true; - } - if (!framebuffer_manager()->IsComplete(framebuffer)) { - if (framebuffer->GetStatus(texture_manager(), GL_DRAW_FRAMEBUFFER) != - GL_FRAMEBUFFER_COMPLETE) { - LOCAL_SET_GL_ERROR( - GL_INVALID_FRAMEBUFFER_OPERATION, func_name, - "framebuffer incomplete (check)"); - return false; - } - framebuffer_manager()->MarkAsComplete(framebuffer); - } - return true; +bool GLES2DecoderImpl::CheckBoundDrawFramebufferValid( + bool clear_uncleared_images, const char* func_name) { + GLenum target = features().chromium_framebuffer_multisample ? + GL_DRAW_FRAMEBUFFER : GL_FRAMEBUFFER; + Framebuffer* framebuffer = GetFramebufferInfoForTarget(target); + bool valid = CheckFramebufferValid( + framebuffer, target, clear_uncleared_images, func_name); + if (valid && !features().chromium_framebuffer_multisample) + OnUseFramebuffer(); + return valid; } -bool GLES2DecoderImpl::CheckBoundReadFramebufferColorAttachment( - const char* func_name) { - Framebuffer* framebuffer = features().chromium_framebuffer_multisample ? - framebuffer_state_.bound_read_framebuffer.get() : - framebuffer_state_.bound_draw_framebuffer.get(); - if (!framebuffer) - return true; - if (framebuffer->GetAttachment(GL_COLOR_ATTACHMENT0) == NULL) { - LOCAL_SET_GL_ERROR( - GL_INVALID_OPERATION, func_name, "no color image attached"); - return false; - } - return true; +bool GLES2DecoderImpl::CheckBoundReadFramebufferValid(const char* func_name) { + GLenum target = features().chromium_framebuffer_multisample ? + GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER; + Framebuffer* framebuffer = GetFramebufferInfoForTarget(target); + bool valid = CheckFramebufferValid(framebuffer, target, true, func_name); + return valid; } GLint GLES2DecoderImpl::GetColorEncodingFromInternalFormat( @@ -3814,7 +3776,7 @@ gfx::Size GLES2DecoderImpl::GetBoundReadFrameBufferSize() { GetFramebufferInfoForTarget(GL_READ_FRAMEBUFFER_EXT); if (framebuffer != NULL) { const Framebuffer::Attachment* attachment = - framebuffer->GetAttachment(GL_COLOR_ATTACHMENT0); + framebuffer->GetReadBufferAttachment(); if (attachment) { return gfx::Size(attachment->width(), attachment->height()); } @@ -5921,7 +5883,7 @@ error::Error GLES2DecoderImpl::HandleDeleteProgram(uint32_t immediate_data_size, error::Error GLES2DecoderImpl::DoClear(GLbitfield mask) { DCHECK(!ShouldDeferDraws()); - if (CheckBoundFramebuffersValid("glClear")) { + if (CheckBoundDrawFramebufferValid(true, "glClear")) { ApplyDirtyState(); // TODO(zmo): Filter out INTEGER/SIGNED INTEGER images to avoid // undefined results. @@ -5945,7 +5907,8 @@ error::Error GLES2DecoderImpl::DoClear(GLbitfield mask) { void GLES2DecoderImpl::DoClearBufferiv( GLenum buffer, GLint drawbuffer, const GLint* value) { - if (!CheckBoundDrawFramebufferValid("glClearBufferiv")) + // TODO(zmo): Set clear_uncleared_images=true once crbug.com/584059 is fixed. + if (!CheckBoundDrawFramebufferValid(false, "glClearBufferiv")) return; ApplyDirtyState(); @@ -5988,7 +5951,8 @@ void GLES2DecoderImpl::DoClearBufferiv( void GLES2DecoderImpl::DoClearBufferuiv( GLenum buffer, GLint drawbuffer, const GLuint* value) { - if (!CheckBoundDrawFramebufferValid("glClearBufferuiv")) + // TODO(zmo): Set clear_uncleared_images=true once crbug.com/584059 is fixed. + if (!CheckBoundDrawFramebufferValid(false, "glClearBufferuiv")) return; ApplyDirtyState(); @@ -6018,7 +5982,8 @@ void GLES2DecoderImpl::DoClearBufferuiv( void GLES2DecoderImpl::DoClearBufferfv( GLenum buffer, GLint drawbuffer, const GLfloat* value) { - if (!CheckBoundDrawFramebufferValid("glClearBufferfv")) + // TODO(zmo): Set clear_uncleared_images=true once crbug.com/584059 is fixed. + if (!CheckBoundDrawFramebufferValid(false, "glClearBufferfv")) return; ApplyDirtyState(); @@ -6061,7 +6026,8 @@ void GLES2DecoderImpl::DoClearBufferfv( void GLES2DecoderImpl::DoClearBufferfi( GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) { - if (!CheckBoundDrawFramebufferValid("glClearBufferfi")) + // TODO(zmo): Set clear_uncleared_images=true once crbug.com/584059 is fixed. + if (!CheckBoundDrawFramebufferValid(false, "glClearBufferfi")) return; ApplyDirtyState(); @@ -6468,7 +6434,8 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM( GLbitfield mask, GLenum filter) { DCHECK(!ShouldDeferReads() && !ShouldDeferDraws()); - if (!CheckBoundFramebuffersValid("glBlitFramebufferCHROMIUM")) { + if (!CheckBoundDrawFramebufferValid(true, "glBlitFramebufferCHROMIUM") || + !CheckBoundReadFramebufferValid("glBlitFramebufferCHROMIUM")) { return; } @@ -8013,7 +7980,7 @@ error::Error GLES2DecoderImpl::DoDrawArrays( LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "primcount < 0"); return error::kNoError; } - if (!CheckBoundFramebuffersValid(function_name)) { + if (!CheckBoundDrawFramebufferValid(true, function_name)) { return error::kNoError; } // We have to check this here because the prototype for glDrawArrays @@ -8135,7 +8102,7 @@ error::Error GLES2DecoderImpl::DoDrawElements(const char* function_name, return error::kNoError; } - if (!CheckBoundFramebuffersValid(function_name)) { + if (!CheckBoundDrawFramebufferValid(true, function_name)) { return error::kNoError; } @@ -9218,10 +9185,13 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size, return error::kNoError; } + if (!CheckBoundReadFramebufferValid("glReadPixels")) { + return error::kNoError; + } GLenum src_internal_format = GetBoundReadFrameBufferInternalFormat(); if (src_internal_format == 0) { - LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glReadPixels", - "no valid read buffer source"); + LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glReadPixels", + "no valid color image"); return error::kNoError; } std::vector<GLenum> accepted_formats; @@ -9326,7 +9296,7 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size, return error::kNoError; } - if (!CheckBoundFramebuffersValid("glReadPixels")) { + if (!CheckBoundReadFramebufferValid("glReadPixels")) { return error::kNoError; } @@ -11218,8 +11188,14 @@ void GLES2DecoderImpl::DoCopyTexImage2D( return; } - // Check we have compatible formats. GLenum read_format = GetBoundReadFrameBufferInternalFormat(); + if (read_format == 0) { + LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glCopyTexImage2D", + "no valid color image"); + return; + } + + // Check we have compatible formats. uint32_t channels_exist = GLES2Util::GetChannelsForFormat(read_format); uint32_t channels_needed = GLES2Util::GetChannelsForFormat(internal_format); @@ -11268,7 +11244,7 @@ void GLES2DecoderImpl::DoCopyTexImage2D( return; } - if (!CheckBoundReadFramebufferColorAttachment("glCopyTexImage2D")) { + if (!CheckBoundReadFramebufferValid("glCopyTexImage2D")) { return; } @@ -11279,10 +11255,6 @@ void GLES2DecoderImpl::DoCopyTexImage2D( return; } - if (!CheckBoundFramebuffersValid("glCopyTexImage2D")) { - return; - } - LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glCopyTexImage2D"); ScopedResolvedFrameBufferBinder binder(this, false, true); gfx::Size size = GetBoundReadFrameBufferSize(); @@ -11362,8 +11334,13 @@ void GLES2DecoderImpl::DoCopyTexSubImage2D( return; } - // Check we have compatible formats. GLenum read_format = GetBoundReadFrameBufferInternalFormat(); + if (read_format == 0) { + LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glCopyTexImage2D", + "no valid color image"); + return; + } + // Check we have compatible formats. uint32_t channels_exist = GLES2Util::GetChannelsForFormat(read_format); uint32_t channels_needed = GLES2Util::GetChannelsForFormat(format); @@ -11381,7 +11358,7 @@ void GLES2DecoderImpl::DoCopyTexSubImage2D( return; } - if (!CheckBoundReadFramebufferColorAttachment("glCopyTexSubImage2D")) { + if (!CheckBoundReadFramebufferValid("glCopyTexImage2D")) { return; } @@ -11392,10 +11369,6 @@ void GLES2DecoderImpl::DoCopyTexSubImage2D( return; } - if (!CheckBoundFramebuffersValid("glCopyTexSubImage2D")) { - return; - } - ScopedResolvedFrameBufferBinder binder(this, false, true); gfx::Size size = GetBoundReadFrameBufferSize(); GLint copyX = 0; diff --git a/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h b/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h index 6da0bb0..ab35382 100644 --- a/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h +++ b/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h @@ -779,6 +779,10 @@ static const GLenum valid_read_pixel_format_table_es3[] = { GL_RG_INTEGER, GL_RGB_INTEGER, GL_RGBA_INTEGER, }; +static const GLenum deprecated_read_pixel_format_table_es3[] = { + GL_ALPHA, +}; + static const GLenum valid_read_pixel_type_table[] = { GL_UNSIGNED_BYTE, GL_UNSIGNED_SHORT_5_6_5, GL_UNSIGNED_SHORT_4_4_4_4, GL_UNSIGNED_SHORT_5_5_5_1, @@ -1459,6 +1463,9 @@ void Validators::UpdateValuesES3() { pixel_type.AddValues(valid_pixel_type_table_es3, arraysize(valid_pixel_type_table_es3)); program_parameter.SetIsES3(true); + read_pixel_format.RemoveValues( + deprecated_read_pixel_format_table_es3, + arraysize(deprecated_read_pixel_format_table_es3)); read_pixel_format.AddValues(valid_read_pixel_format_table_es3, arraysize(valid_read_pixel_format_table_es3)); read_pixel_type.AddValues(valid_read_pixel_type_table_es3, diff --git a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp index 310449f..0553803 100644 --- a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp +++ b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp @@ -627,11 +627,14 @@ void WebGL2RenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLs return; } - // Need to validate whether the pixel pack buffer is mapped, - // if we decide to expose mapBufferRange() to web developers. + const char* reason = "framebuffer incomplete"; + WebGLFramebuffer* framebuffer = getReadFramebufferBinding(); + if (framebuffer && framebuffer->checkDepthStencilStatus(&reason) != GL_FRAMEBUFFER_COMPLETE) { + synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, "readPixels", reason); + return; + } long long size = buffer->getSize() - offset; - // If size is negative, or size is not large enough to store pixels, those cases // are handled by validateReadPixelsFuncParameters to generate INVALID_OPERATION. if (!validateReadPixelsFuncParameters(width, height, format, type, size)) @@ -639,9 +642,8 @@ void WebGL2RenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLs clearIfComposited(); - WebGLFramebuffer* readFramebufferBinding = getFramebufferBinding(GL_READ_FRAMEBUFFER); { - ScopedDrawingBufferBinder binder(drawingBuffer(), readFramebufferBinding); + ScopedDrawingBufferBinder binder(drawingBuffer(), framebuffer); webContext()->readPixels(x, y, width, height, format, type, reinterpret_cast<void*>(offset)); } } @@ -1049,7 +1051,7 @@ void WebGL2RenderingContextBase::copyTexSubImage3D(GLenum target, GLint level, G WebGLFramebuffer* readFramebufferBinding = nullptr; if (!validateCopyTexSubImage("copyTexSubImage3D", target, level, xoffset, yoffset, zoffset, x, y, width, height)) return; - if (!validateReadBufferAndGetInfo("copyTexSubImage3D", readFramebufferBinding, nullptr, nullptr)) + if (!validateReadBufferAndGetInfo("copyTexSubImage3D", readFramebufferBinding)) return; WebGLTexture* tex = validateTextureBinding("copyTexSubImage3D", target, true); ASSERT(tex); @@ -3128,6 +3130,11 @@ WebGLFramebuffer* WebGL2RenderingContextBase::getFramebufferBinding(GLenum targe } } +WebGLFramebuffer* WebGL2RenderingContextBase::getReadFramebufferBinding() +{ + return m_readFramebufferBinding.get(); +} + bool WebGL2RenderingContextBase::validateGetFramebufferAttachmentParameterFunc(const char* functionName, GLenum target, GLenum attachment) { if (!validateFramebufferTarget(target)) { diff --git a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h index 5e2a873..c5e71e9 100644 --- a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h +++ b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h @@ -229,6 +229,7 @@ protected: DOMArrayBufferView::ViewType readPixelsExpectedArrayBufferViewType(GLenum type) override; WebGLFramebuffer* getFramebufferBinding(GLenum target) override; + WebGLFramebuffer* getReadFramebufferBinding() override; GLint getMaxTextureLevelForTarget(GLenum target) override; void renderbufferStorageImpl(GLenum target, GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height, const char* functionName) override; GLenum boundFramebufferColorFormat() override; diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp index 7898a54..16521a5 100644 --- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp +++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp @@ -1763,6 +1763,11 @@ WebGLFramebuffer* WebGLRenderingContextBase::getFramebufferBinding(GLenum target return nullptr; } +WebGLFramebuffer* WebGLRenderingContextBase::getReadFramebufferBinding() +{ + return m_framebufferBinding.get(); +} + GLenum WebGLRenderingContextBase::checkFramebufferStatus(GLenum target) { if (isContextLost()) @@ -1963,7 +1968,7 @@ void WebGLRenderingContextBase::copyTexImage2D(GLenum target, GLint level, GLenu return; } WebGLFramebuffer* readFramebufferBinding = nullptr; - if (!validateReadBufferAndGetInfo("copyTexImage2D", readFramebufferBinding, nullptr, nullptr)) + if (!validateReadBufferAndGetInfo("copyTexImage2D", readFramebufferBinding)) return; if (!isTexInternalFormatColorBufferCombinationValid(internalformat, boundFramebufferColorFormat())) { synthesizeGLError(GL_INVALID_OPERATION, "copyTexImage2D", "framebuffer is incompatible format"); @@ -1986,7 +1991,7 @@ void WebGLRenderingContextBase::copyTexSubImage2D(GLenum target, GLint level, GL WebGLFramebuffer* readFramebufferBinding = nullptr; if (!validateCopyTexSubImage("copyTexSubImage2D", target, level, xoffset, yoffset, 0, x, y, width, height)) return; - if (!validateReadBufferAndGetInfo("copyTexSubImage2D", readFramebufferBinding, nullptr, nullptr)) + if (!validateReadBufferAndGetInfo("copyTexSubImage2D", readFramebufferBinding)) return; WebGLTexture* tex = validateTextureBinding("copyTexSubImage2D", target, true); ASSERT(tex); @@ -3712,17 +3717,16 @@ void WebGLRenderingContextBase::polygonOffset(GLfloat factor, GLfloat units) webContext()->polygonOffset(factor, units); } -bool WebGLRenderingContextBase::validateReadBufferAndGetInfo(const char* functionName, WebGLFramebuffer*& readFramebufferBinding, GLenum* format, GLenum* type) +bool WebGLRenderingContextBase::validateReadBufferAndGetInfo(const char* functionName, WebGLFramebuffer*& readFramebufferBinding) { - GLenum target = isWebGL2OrHigher() ? GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER; - readFramebufferBinding = getFramebufferBinding(target); + readFramebufferBinding = getReadFramebufferBinding(); if (readFramebufferBinding) { const char* reason = "framebuffer incomplete"; if (readFramebufferBinding->checkStatus(&reason) != GL_FRAMEBUFFER_COMPLETE) { synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, functionName, reason); return false; } - if (!readFramebufferBinding->getReadBufferFormatAndType(format, type)) { + if (!readFramebufferBinding->getReadBufferFormatAndType(nullptr, nullptr)) { synthesizeGLError(GL_INVALID_OPERATION, functionName, "no image to read from"); return false; } @@ -3732,11 +3736,6 @@ bool WebGLRenderingContextBase::validateReadBufferAndGetInfo(const char* functio synthesizeGLError(GL_INVALID_OPERATION, functionName, "no image to read from"); return false; } - // Obtain the default drawing buffer's format and type. - if (format) - *format = drawingBuffer()->getActualAttributes().alpha ? GL_RGBA : GL_RGB; - if (type) - *type = GL_UNSIGNED_BYTE; } return true; } @@ -3769,66 +3768,6 @@ bool WebGLRenderingContextBase::validateReadPixelsFormatAndType(GLenum format, G return true; } -bool WebGLRenderingContextBase::validateReadPixelsFormatTypeCombination(GLenum format, GLenum type, GLenum readBufferInternalFormat, GLenum readBufferType) -{ - GLenum acceptedFormat = 0, acceptedType = 0; - switch (readBufferInternalFormat) { // This is internalformat. - case GL_R8UI: - case GL_R16UI: - case GL_R32UI: - case GL_RG8UI: - case GL_RG16UI: - case GL_RG32UI: - // All the RGB_INTEGER formats are not renderable. - case GL_RGBA8UI: - case GL_RGB10_A2UI: - case GL_RGBA16UI: - case GL_RGBA32UI: - acceptedFormat = GL_RGBA_INTEGER; - acceptedType = GL_UNSIGNED_INT; - break; - case GL_R8I: - case GL_R16I: - case GL_R32I: - case GL_RG8I: - case GL_RG16I: - case GL_RG32I: - case GL_RGBA8I: - case GL_RGBA16I: - case GL_RGBA32I: - acceptedFormat = GL_RGBA_INTEGER; - acceptedType = GL_INT; - break; - default: - acceptedFormat = GL_RGBA; - switch (readBufferType) { - case GL_HALF_FLOAT: - case GL_HALF_FLOAT_OES: - case GL_FLOAT: - case GL_UNSIGNED_INT_10F_11F_11F_REV: - acceptedType = GL_FLOAT; - break; - default: - acceptedType = GL_UNSIGNED_BYTE; - break; - } - break; - } - - if (!(format == acceptedFormat && type == acceptedType) - && !(readBufferInternalFormat == GL_RGB10_A2 && format == GL_RGBA && type == GL_UNSIGNED_INT_2_10_10_10_REV)) { - // Check against the implementation color read format and type. - WGC3Dint implFormat = 0, implType = 0; - webContext()->getIntegerv(GL_IMPLEMENTATION_COLOR_READ_FORMAT, &implFormat); - webContext()->getIntegerv(GL_IMPLEMENTATION_COLOR_READ_TYPE, &implType); - if (!implFormat || !implType || format != static_cast<GLenum>(implFormat) || type != static_cast<GLenum>(implType)) { - synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "invalid format/type combination"); - return false; - } - } - return true; -} - DOMArrayBufferView::ViewType WebGLRenderingContextBase::readPixelsExpectedArrayBufferViewType(GLenum type) { switch (type) { @@ -3866,12 +3805,6 @@ bool WebGLRenderingContextBase::validateReadPixelsFuncParameters(GLsizei width, { if (!validateReadPixelsFormatAndType(format, type)) return false; - WebGLFramebuffer* readFramebufferBinding = nullptr; - GLenum readBufferInternalFormat = 0, readBufferType = 0; - if (!validateReadBufferAndGetInfo("readPixels", readFramebufferBinding, &readBufferInternalFormat, &readBufferType)) - return false; - if (!validateReadPixelsFormatTypeCombination(format, type, readBufferInternalFormat, readBufferType)) - return false; // Calculate array size, taking into consideration of pack parameters. unsigned totalBytesRequired = 0, totalSkipBytes = 0; @@ -3899,7 +3832,12 @@ void WebGLRenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLsi synthesizeGLError(GL_INVALID_VALUE, "readPixels", "no destination ArrayBufferView"); return; } - + const char* reason = "framebuffer incomplete"; + WebGLFramebuffer* framebuffer = getReadFramebufferBinding(); + if (framebuffer && framebuffer->checkDepthStencilStatus(&reason) != GL_FRAMEBUFFER_COMPLETE) { + synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, "readPixels", reason); + return; + } if (!validateReadPixelsFuncParameters(width, height, format, type, static_cast<long long>(pixels->byteLength()))) return; @@ -3913,10 +3851,8 @@ void WebGLRenderingContextBase::readPixels(GLint x, GLint y, GLsizei width, GLsi clearIfComposited(); void* data = pixels->baseAddress(); - GLenum target = isWebGL2OrHigher() ? GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER; - WebGLFramebuffer* readFramebufferBinding = getFramebufferBinding(target); { - ScopedDrawingBufferBinder binder(drawingBuffer(), readFramebufferBinding); + ScopedDrawingBufferBinder binder(drawingBuffer(), framebuffer); webContext()->readPixels(x, y, width, height, format, type, data); } } @@ -6565,7 +6501,7 @@ bool WebGLRenderingContextBase::validateDrawArrays(const char* functionName, GLe } const char* reason = "framebuffer incomplete"; - if (m_framebufferBinding && m_framebufferBinding->checkStatus(&reason) != GL_FRAMEBUFFER_COMPLETE) { + if (m_framebufferBinding && m_framebufferBinding->checkDepthStencilStatus(&reason) != GL_FRAMEBUFFER_COMPLETE) { synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, functionName, reason); return false; } @@ -6594,7 +6530,7 @@ bool WebGLRenderingContextBase::validateDrawElements(const char* functionName, G } const char* reason = "framebuffer incomplete"; - if (m_framebufferBinding && m_framebufferBinding->checkStatus(&reason) != GL_FRAMEBUFFER_COMPLETE) { + if (m_framebufferBinding && m_framebufferBinding->checkDepthStencilStatus(&reason) != GL_FRAMEBUFFER_COMPLETE) { synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, functionName, reason); return false; } diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h index 9b7928d..95875c7 100644 --- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h +++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h @@ -868,11 +868,10 @@ protected: // Generates GL error and returns false if parameters are invalid. bool validateTexFuncFormatAndType(const char* functionName, TexImageFunctionType, GLenum internalformat, GLenum format, GLenum type, GLint level); - // Helper function to check readbuffer validity for readPixels and copyTex{Sub}Image. - // If yes, obtains the readbuffer's format, type, the bound read framebuffer, returns true. + // Helper function to check readbuffer validity for copyTex{Sub}Image. + // If yes, obtains the bound read framebuffer, returns true. // If not, generates a GL error, returns false. - // Note: it's OK to pass format == nullptr and type == nullptr. - bool validateReadBufferAndGetInfo(const char* functionName, WebGLFramebuffer*& readFramebufferBinding, GLenum* format, GLenum* type); + bool validateReadBufferAndGetInfo(const char* functionName, WebGLFramebuffer*& readFramebufferBinding); // Helper function to check format/type enums for readPixels. // Generates INVALID_ENUM and returns false if parameters are invalid. @@ -881,10 +880,6 @@ protected: // Helper function to get expected ArrayBuffer view type for readPixels. virtual DOMArrayBufferView::ViewType readPixelsExpectedArrayBufferViewType(GLenum type); - // Helper function to check format/type combination for readPixels. - // Generates INVALID_OPERATION and returns false if the combination is unsupported. - bool validateReadPixelsFormatTypeCombination(GLenum format, GLenum type, GLenum readBufferInternalFormat, GLenum readBufferType); - // Helper function to check parameters of readPixels. Returns true if all parameters // are valid. Otherwise, generates appropriate error and returns false. bool validateReadPixelsFuncParameters(GLsizei width, GLsizei height, GLenum format, GLenum type, long long bufferSize); @@ -980,6 +975,8 @@ protected: // Get the framebuffer bound to given target virtual WebGLFramebuffer* getFramebufferBinding(GLenum target); + virtual WebGLFramebuffer* getReadFramebufferBinding(); + // Helper function to validate input parameters for framebuffer functions. // Generate GL error if parameters are illegal. bool validateFramebufferFuncParameters(const char* functionName, GLenum target, GLenum attachment); |