Refactor glReadPixels validation.
Bug swiftshader:104
Change-Id: I2e5ab18f400035ec3f2125fb001643ca1125c565
Reviewed-on: https://swiftshader-review.googlesource.com/18148
Reviewed-by: Alexis Hétu <sugoi@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/OpenGL/common/Image.cpp b/src/OpenGL/common/Image.cpp
index 988b532..cdded3a 100644
--- a/src/OpenGL/common/Image.cpp
+++ b/src/OpenGL/common/Image.cpp
@@ -155,8 +155,7 @@
switch(type)
{
case GL_UNSIGNED_SHORT: return sw::FORMAT_D16;
- case GL_UNSIGNED_INT_24_8_OES: return sw::FORMAT_D24S8;
- case GL_UNSIGNED_INT: return sw::FORMAT_D32;
+ case GL_UNSIGNED_INT_24_8_OES: return sw::FORMAT_D24X8;
case GL_FLOAT: return sw::FORMAT_D32F_LOCKABLE;
default: UNREACHABLE(type);
}
diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp
index e257174..fbbb356 100644
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -3298,9 +3298,9 @@
return error(GL_INVALID_OPERATION);
}
- if(!IsValidReadPixelsFormatType(framebuffer, format, type))
+ if(!ValidateReadPixelsFormatType(framebuffer, format, type))
{
- return error(GL_INVALID_OPERATION);
+ return;
}
GLsizei outputWidth = (mState.packParameters.rowLength > 0) ? mState.packParameters.rowLength : width;
@@ -3335,14 +3335,12 @@
return error(GL_INVALID_OPERATION);
}
- sw::RectF rect((float)x, (float)y, (float)(x + width), (float)(y + height));
- sw::Rect dstRect(0, 0, width, height);
- rect.clip(0.0f, 0.0f, (float)renderTarget->getWidth(), (float)renderTarget->getHeight());
+ sw::SliceRectF srcRect((float)x, (float)y, (float)(x + width), (float)(y + height), 0);
+ sw::SliceRect dstRect(0, 0, width, height, 0);
+ srcRect.clip(0.0f, 0.0f, (float)renderTarget->getWidth(), (float)renderTarget->getHeight());
- sw::Surface *externalSurface = sw::Surface::create(width, height, 1, gl::ConvertReadFormatType(format, type), pixels, outputPitch, outputPitch * outputHeight);
- sw::SliceRectF sliceRect(rect);
- sw::SliceRect dstSliceRect(dstRect);
- device->blit(renderTarget, sliceRect, externalSurface, dstSliceRect, false, false, false);
+ sw::Surface *externalSurface = sw::Surface::create(width, height, 1, gl::ConvertReadFormatType(format, type), pixels, outputPitch, outputPitch * outputHeight);
+ device->blit(renderTarget, srcRect, externalSurface, dstRect, false, false, false);
externalSurface->lockExternal(0, 0, 0, sw::LOCK_READONLY, sw::PUBLIC);
externalSurface->unlockExternal();
delete externalSurface;
diff --git a/src/OpenGL/libGLESv2/Framebuffer.cpp b/src/OpenGL/libGLESv2/Framebuffer.cpp
index 4a9be7b..fd9cd10 100644
--- a/src/OpenGL/libGLESv2/Framebuffer.cpp
+++ b/src/OpenGL/libGLESv2/Framebuffer.cpp
@@ -354,7 +354,7 @@
}
else if(IsTextureTarget(mColorbufferType[i]))
{
- GLenum format = colorbuffer->getFormat();
+ GLint format = colorbuffer->getFormat();
if(!IsColorRenderable(format))
{
@@ -668,7 +668,7 @@
case GL_DEPTH_COMPONENT32_OES: return GL_UNSIGNED_INT;
case GL_DEPTH_COMPONENT32F: return GL_FLOAT;
case GL_DEPTH24_STENCIL8: return GL_UNSIGNED_INT_24_8_OES;
- case GL_DEPTH32F_STENCIL8: return GL_FLOAT;
+ case GL_DEPTH32F_STENCIL8: return GL_FLOAT_32_UNSIGNED_INT_24_8_REV;
default:
UNREACHABLE(depthbuffer->getFormat());
}
diff --git a/src/OpenGL/libGLESv2/utilities.cpp b/src/OpenGL/libGLESv2/utilities.cpp
index 246b71c..4108a79 100644
--- a/src/OpenGL/libGLESv2/utilities.cpp
+++ b/src/OpenGL/libGLESv2/utilities.cpp
@@ -623,7 +623,7 @@
return true;
}
- bool IsValidReadPixelsFormatType(const Framebuffer *framebuffer, GLenum format, GLenum type)
+ bool ValidateReadPixelsFormatType(const Framebuffer *framebuffer, GLenum format, GLenum type)
{
// GL_NV_read_depth
if(format == GL_DEPTH_COMPONENT)
@@ -632,25 +632,56 @@
if(!depthbuffer)
{
- return false;
+ return error(GL_INVALID_OPERATION, false);
}
+ GLint internalformat = depthbuffer->getFormat();
+
switch(type)
{
case GL_UNSIGNED_SHORT:
+ case GL_UNSIGNED_INT_24_8_OES:
+ switch(internalformat)
+ {
+ case GL_DEPTH_COMPONENT16:
+ case GL_DEPTH_COMPONENT24:
+ case GL_DEPTH_COMPONENT32_OES:
+ case GL_DEPTH24_STENCIL8:
+ break;
+ case GL_DEPTH_COMPONENT32F:
+ case GL_DEPTH32F_STENCIL8:
+ return error(GL_INVALID_OPERATION, false);
+ default:
+ UNREACHABLE(internalformat);
+ return error(GL_INVALID_OPERATION, false);
+ }
case GL_FLOAT:
- return true;
+ switch(internalformat)
+ {
+ case GL_DEPTH_COMPONENT32F:
+ case GL_DEPTH32F_STENCIL8:
+ break;
+ case GL_DEPTH_COMPONENT16:
+ case GL_DEPTH_COMPONENT24:
+ case GL_DEPTH_COMPONENT32_OES:
+ case GL_DEPTH24_STENCIL8:
+ return error(GL_INVALID_OPERATION, false);
+ default:
+ UNREACHABLE(internalformat);
+ return error(GL_INVALID_OPERATION, false);
+ }
default:
- UNIMPLEMENTED();
- return false;
+ return error(GL_INVALID_ENUM, false);
}
+
+ return true;
}
Renderbuffer *colorbuffer = framebuffer->getReadColorbuffer();
if(!colorbuffer)
{
- return false;
+ return error(GL_INVALID_OPERATION, false);
}
GLint internalformat = colorbuffer->getFormat();
@@ -730,14 +761,12 @@
}
}
- return false;
+ return error(GL_INVALID_OPERATION, false);
}
- bool IsDepthTexture(GLenum format)
+ bool IsDepthTexture(GLint format)
{
- return format == GL_DEPTH_COMPONENT ||
- format == GL_DEPTH_STENCIL_OES ||
- format == GL_DEPTH_COMPONENT16 ||
+ return format == GL_DEPTH_COMPONENT16 ||
format == GL_DEPTH_COMPONENT24 ||
format == GL_DEPTH_COMPONENT32_OES ||
format == GL_DEPTH_COMPONENT32F ||
@@ -745,12 +774,11 @@
format == GL_DEPTH32F_STENCIL8;
}
- bool IsStencilTexture(GLenum format)
+ bool IsStencilTexture(GLint format)
{
- return format == GL_STENCIL_INDEX_OES ||
- format == GL_DEPTH_STENCIL_OES ||
- format == GL_DEPTH24_STENCIL8 ||
- format == GL_DEPTH32F_STENCIL8;
+ return format == GL_DEPTH24_STENCIL8 ||
+ format == GL_DEPTH32F_STENCIL8 ||
+ format == GL_STENCIL_INDEX8;
}
bool IsCubemapTextureTarget(GLenum target)
@@ -1142,8 +1170,9 @@
case GL_UNSIGNED_INT_2_10_10_10_REV:
case GL_UNSIGNED_INT_10F_11F_11F_REV:
case GL_UNSIGNED_INT_5_9_9_9_REV:
- case GL_FLOAT_32_UNSIGNED_INT_24_8_REV:
return 4;
+ case GL_FLOAT_32_UNSIGNED_INT_24_8_REV:
+ return 8;
default:
UNREACHABLE(type);
break;
diff --git a/src/OpenGL/libGLESv2/utilities.h b/src/OpenGL/libGLESv2/utilities.h
index 00f577e..3b2704e 100644
--- a/src/OpenGL/libGLESv2/utilities.h
+++ b/src/OpenGL/libGLESv2/utilities.h
@@ -51,9 +51,9 @@
GLenum ValidateSubImageParams(bool compressed, bool copy, GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset,
GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, Texture *texture);
bool ValidateCopyFormats(GLenum textureFormat, GLenum colorbufferFormat);
- bool IsValidReadPixelsFormatType(const Framebuffer *framebuffer, GLenum format, GLenum type);
- bool IsDepthTexture(GLenum format);
- bool IsStencilTexture(GLenum format);
+ bool ValidateReadPixelsFormatType(const Framebuffer *framebuffer, GLenum format, GLenum type);
+ bool IsDepthTexture(GLint format);
+ bool IsStencilTexture(GLint format);
bool IsCubemapTextureTarget(GLenum target);
int CubeFaceIndex(GLenum cubeTarget);
bool IsTextureTarget(GLenum target);
diff --git a/src/Renderer/Blitter.cpp b/src/Renderer/Blitter.cpp
index 4245cd4..6522a13 100644
--- a/src/Renderer/Blitter.cpp
+++ b/src/Renderer/Blitter.cpp
@@ -389,7 +389,8 @@
c.x = Float(Int((*Pointer<UShort>(element))));
break;
case FORMAT_D24S8:
- c.x = Float(Int((*Pointer<UInt>(element))));
+ case FORMAT_D24X8:
+ c.x = Float(Int((*Pointer<UInt>(element) & UInt(0xFFFFFF00)) >> 8));
break;
case FORMAT_D32:
c.x = Float(Int((*Pointer<UInt>(element))));
@@ -755,7 +756,8 @@
*Pointer<UShort>(element) = UShort(RoundInt(Float(c.x)));
break;
case FORMAT_D24S8:
- *Pointer<UInt>(element) = UInt(RoundInt(Float(c.x)));
+ case FORMAT_D24X8:
+ *Pointer<UInt>(element) = UInt(RoundInt(Float(c.x)) << 8);
break;
case FORMAT_D32:
*Pointer<UInt>(element) = UInt(RoundInt(Float(c.x)));
@@ -1050,6 +1052,7 @@
scale = vector(0xFFFF, 0.0f, 0.0f, 0.0f);
break;
case FORMAT_D24S8:
+ case FORMAT_D24X8:
scale = vector(0xFFFFFF, 0.0f, 0.0f, 0.0f);
break;
case FORMAT_D32:
