Fix framebuffer attachment validation. GL_BACK, GL_DEPTH, and GL_STENCIL are only valid for the default framebuffer, while GL_DEPTH_ATTACHMENT, GL_STENCIL_ATTACHMENT, GL_DEPTH_STENCIL_ATTACHMENT, and GL_COLOR_ATTACHMENTi are only valid for non-default framebuffer objects. Also implement the color encoding query. Change-Id: I153ae9407850a30ed14d9ae145ee3504ba71029a Reviewed-on: https://swiftshader-review.googlesource.com/14569 Tested-by: Nicolas Capens <nicolascapens@google.com> Reviewed-by: Alexis Hétu <sugoi@google.com> Reviewed-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/OpenGL/libGLESv2/libGLESv2.cpp b/src/OpenGL/libGLESv2/libGLESv2.cpp index 8f8ccdf..5198ada 100644 --- a/src/OpenGL/libGLESv2/libGLESv2.cpp +++ b/src/OpenGL/libGLESv2/libGLESv2.cpp
@@ -2756,121 +2756,86 @@ return error(GL_INVALID_ENUM); } - GLint clientVersion = context->getClientVersion(); + GLuint framebufferName = 0; - es2::Framebuffer *framebuffer = nullptr; if(target == GL_READ_FRAMEBUFFER) { - if(context->getReadFramebufferName() == 0) - { - if(clientVersion < 3) - { - return error(GL_INVALID_OPERATION); - } - else - { - switch(attachment) - { - case GL_BACK: - case GL_DEPTH: - case GL_STENCIL: - break; - default: - return error(GL_INVALID_ENUM); - } - } - } - - framebuffer = context->getReadFramebuffer(); + framebufferName = context->getReadFramebufferName(); } else { - if(context->getDrawFramebufferName() == 0) - { - if(clientVersion < 3) - { - return error(GL_INVALID_OPERATION); - } - else - { - switch(attachment) - { - case GL_BACK: - case GL_DEPTH: - case GL_STENCIL: - break; - default: - return error(GL_INVALID_ENUM); - } - } - } - - framebuffer = context->getDrawFramebuffer(); + framebufferName = context->getDrawFramebufferName(); } - GLenum attachmentType; - GLuint attachmentHandle; - GLint attachmentLayer; - Renderbuffer* renderbuffer = nullptr; + GLint clientVersion = context->getClientVersion(); + + if(framebufferName == 0) // Default framebuffer. + { + if(clientVersion < 3) + { + return error(GL_INVALID_OPERATION); + } + } + switch(attachment) { case GL_BACK: - if(clientVersion >= 3) - { - attachmentType = framebuffer->getColorbufferType(0); - attachmentHandle = framebuffer->getColorbufferName(0); - attachmentLayer = framebuffer->getColorbufferLayer(0); - renderbuffer = framebuffer->getColorbuffer(0); - } - else return error(GL_INVALID_ENUM); - break; - case GL_COLOR_ATTACHMENT0: - case GL_COLOR_ATTACHMENT1: - case GL_COLOR_ATTACHMENT2: - case GL_COLOR_ATTACHMENT3: - case GL_COLOR_ATTACHMENT4: - case GL_COLOR_ATTACHMENT5: - case GL_COLOR_ATTACHMENT6: - case GL_COLOR_ATTACHMENT7: - case GL_COLOR_ATTACHMENT8: - case GL_COLOR_ATTACHMENT9: - case GL_COLOR_ATTACHMENT10: - case GL_COLOR_ATTACHMENT11: - case GL_COLOR_ATTACHMENT12: - case GL_COLOR_ATTACHMENT13: - case GL_COLOR_ATTACHMENT14: - case GL_COLOR_ATTACHMENT15: - case GL_COLOR_ATTACHMENT16: - case GL_COLOR_ATTACHMENT17: - case GL_COLOR_ATTACHMENT18: - case GL_COLOR_ATTACHMENT19: - case GL_COLOR_ATTACHMENT20: - case GL_COLOR_ATTACHMENT21: - case GL_COLOR_ATTACHMENT22: - case GL_COLOR_ATTACHMENT23: - case GL_COLOR_ATTACHMENT24: - case GL_COLOR_ATTACHMENT25: - case GL_COLOR_ATTACHMENT26: - case GL_COLOR_ATTACHMENT27: - case GL_COLOR_ATTACHMENT28: - case GL_COLOR_ATTACHMENT29: - case GL_COLOR_ATTACHMENT30: - case GL_COLOR_ATTACHMENT31: - if((attachment - GL_COLOR_ATTACHMENT0) >= MAX_COLOR_ATTACHMENTS) - { - return error(GL_INVALID_ENUM); - } - attachmentType = framebuffer->getColorbufferType(attachment - GL_COLOR_ATTACHMENT0); - attachmentHandle = framebuffer->getColorbufferName(attachment - GL_COLOR_ATTACHMENT0); - attachmentLayer = framebuffer->getColorbufferLayer(attachment - GL_COLOR_ATTACHMENT0); - renderbuffer = framebuffer->getColorbuffer(attachment - GL_COLOR_ATTACHMENT0); - break; case GL_DEPTH: + case GL_STENCIL: if(clientVersion < 3) { return error(GL_INVALID_ENUM); } - // fall through + + if(framebufferName != 0) + { + return error(GL_INVALID_OPERATION); + } + break; + case GL_DEPTH_ATTACHMENT: + case GL_STENCIL_ATTACHMENT: + if(framebufferName == 0) + { + return error(GL_INVALID_OPERATION); + } + break; + case GL_DEPTH_STENCIL_ATTACHMENT: + if(clientVersion < 3) + { + return error(GL_INVALID_ENUM); + } + + if(framebufferName == 0) + { + return error(GL_INVALID_OPERATION); + } + break; + default: + if((unsigned int)(attachment - GL_COLOR_ATTACHMENT0) < MAX_COLOR_ATTACHMENTS) + { + if(framebufferName == 0) + { + return error(GL_INVALID_OPERATION); + } + } + else return error(GL_INVALID_ENUM); + } + + es2::Framebuffer *framebuffer = context->getFramebuffer(framebufferName); + + GLenum attachmentType; + GLuint attachmentHandle; + GLint attachmentLayer; + Renderbuffer *renderbuffer = nullptr; + switch(attachment) + { + case GL_BACK: + attachmentType = framebuffer->getColorbufferType(0); + attachmentHandle = framebuffer->getColorbufferName(0); + attachmentLayer = framebuffer->getColorbufferLayer(0); + renderbuffer = framebuffer->getColorbuffer(0); + break; + case GL_DEPTH: case GL_DEPTH_ATTACHMENT: attachmentType = framebuffer->getDepthbufferType(); attachmentHandle = framebuffer->getDepthbufferName(); @@ -2878,11 +2843,6 @@ renderbuffer = framebuffer->getDepthbuffer(); break; case GL_STENCIL: - if(clientVersion < 3) - { - return error(GL_INVALID_ENUM); - } - // fall through case GL_STENCIL_ATTACHMENT: attachmentType = framebuffer->getStencilbufferType(); attachmentHandle = framebuffer->getStencilbufferName(); @@ -2890,26 +2850,32 @@ renderbuffer = framebuffer->getStencilbuffer(); break; case GL_DEPTH_STENCIL_ATTACHMENT: - if(clientVersion >= 3) + attachmentType = framebuffer->getDepthbufferType(); + attachmentHandle = framebuffer->getDepthbufferName(); + attachmentLayer = framebuffer->getDepthbufferLayer(); + renderbuffer = framebuffer->getDepthbuffer(); + + if(attachmentHandle != framebuffer->getStencilbufferName()) { - attachmentType = framebuffer->getDepthbufferType(); - attachmentHandle = framebuffer->getDepthbufferName(); - attachmentLayer = framebuffer->getDepthbufferLayer(); - if(attachmentHandle != framebuffer->getStencilbufferName()) - { - // Different attachments to DEPTH and STENCIL, query fails - return error(GL_INVALID_OPERATION); - } - renderbuffer = framebuffer->getDepthbuffer(); + // Different attachments to DEPTH and STENCIL, query fails + return error(GL_INVALID_OPERATION); } - else return error(GL_INVALID_ENUM); break; default: - return error(GL_INVALID_ENUM); + ASSERT((unsigned int)(attachment - GL_COLOR_ATTACHMENT0) < MAX_COLOR_ATTACHMENTS); + attachmentType = framebuffer->getColorbufferType(attachment - GL_COLOR_ATTACHMENT0); + attachmentHandle = framebuffer->getColorbufferName(attachment - GL_COLOR_ATTACHMENT0); + attachmentLayer = framebuffer->getColorbufferLayer(attachment - GL_COLOR_ATTACHMENT0); + renderbuffer = framebuffer->getColorbuffer(attachment - GL_COLOR_ATTACHMENT0); + break; } GLenum attachmentObjectType = GL_NONE; // Type category - if(attachmentType == GL_NONE || Framebuffer::IsRenderbuffer(attachmentType)) + if(framebufferName == 0) + { + attachmentObjectType = GL_FRAMEBUFFER_DEFAULT; + } + else if(attachmentType == GL_NONE || Framebuffer::IsRenderbuffer(attachmentType)) { attachmentObjectType = attachmentType; } @@ -2927,7 +2893,7 @@ *params = attachmentObjectType; break; case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME: - if(Framebuffer::IsRenderbuffer(attachmentObjectType) || attachmentObjectType == GL_TEXTURE) + if(attachmentObjectType == GL_RENDERBUFFER || attachmentObjectType == GL_TEXTURE) { *params = attachmentHandle; } @@ -2939,7 +2905,7 @@ case GL_FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL: if(attachmentObjectType == GL_TEXTURE) { - *params = clientVersion < 3 ? 0 : renderbuffer->getLevel(); // FramebufferTexture2D will not allow level to be set to anything else in GL ES 2.0 + *params = clientVersion < 3 ? 0 : renderbuffer->getLevel(); // glFramebufferTexture2D does not allow level to be set to anything else in GL ES 2.0 } else { @@ -3024,7 +2990,7 @@ case GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING: if(clientVersion >= 3) { - *params = GL_LINEAR; // FIXME: GL_SRGB will also be possible, when sRGB is added + *params = GetColorEncoding(renderbuffer->getFormat()); } else return error(GL_INVALID_ENUM); break; @@ -3046,7 +3012,6 @@ case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE: *params = GL_NONE; break; - case GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME: if(clientVersion < 3) { @@ -3054,7 +3019,6 @@ } *params = 0; break; - default: if(clientVersion < 3) {
diff --git a/src/OpenGL/libGLESv2/utilities.cpp b/src/OpenGL/libGLESv2/utilities.cpp index 2b5d038..c6ad075 100644 --- a/src/OpenGL/libGLESv2/utilities.cpp +++ b/src/OpenGL/libGLESv2/utilities.cpp
@@ -1719,6 +1719,21 @@ return GetColorComponentType(internalformat) == GL_UNSIGNED_INT; } + GLenum GetColorEncoding(GLint internalformat) + { + switch(internalformat) + { + case GL_SRGB8: + case GL_SRGB8_ALPHA8: + return GL_SRGB; + default: + // [OpenGL ES 3.0.5] section 6.1.13 page 242: + // If attachment is not a color attachment, or no data storage or texture image + // has been specified for the attachment, params will contain the value LINEAR. + return GL_LINEAR; + } + } + std::string ParseUniformName(const std::string &name, unsigned int *outSubscript) { // Strip any trailing array operator and retrieve the subscript
diff --git a/src/OpenGL/libGLESv2/utilities.h b/src/OpenGL/libGLESv2/utilities.h index 6156097..7d04da2 100644 --- a/src/OpenGL/libGLESv2/utilities.h +++ b/src/OpenGL/libGLESv2/utilities.h
@@ -77,6 +77,7 @@ bool IsFloatFormat(GLint internalformat); bool IsSignedNonNormalizedInteger(GLint internalformat); bool IsUnsignedNonNormalizedInteger(GLint internalformat); + GLenum GetColorEncoding(GLint internalformat); // Parse the base uniform name and array index. Returns the base name of the uniform. outSubscript is // set to GL_INVALID_INDEX if the provided name is not an array or the array index is invalid.