On 01/06/17 23:04, Samuel Pitoiset wrote:
Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
---
src/mesa/main/blit.c | 111 +++++++++++++++++++++++++++------------------------
1 file changed, 58 insertions(+), 53 deletions(-)
diff --git a/src/mesa/main/blit.c b/src/mesa/main/blit.c
index 2c0300eab3..207ce7d501 100644
--- a/src/mesa/main/blit.c
+++ b/src/mesa/main/blit.c
@@ -178,6 +178,62 @@ is_valid_blit_filter(const struct gl_context *ctx, GLenum
filter)
static void
+validate_stencil_buffer(struct gl_context *ctx, struct gl_framebuffer *readFb,
+ struct gl_framebuffer *drawFb, GLbitfield *mask,
+ bool no_error, const char *func)
I'd really really like to avoid passing no_error everywhere. Not only
does it result in less organized code, it potentially adds unnecessary
branches in both the no_error and regular code paths.
I think I'd rather if this and the following patches were structured
something like:
blit_framebuffer(...)
{
...
if (readRb == NULL || drawRb == NULL) {
*mask &= ~GL_STENCIL_BUFFER_BIT;
} else if (!no_error) {
if (!validate_stencil_buffer(...))
return;
}
...
}
This makes more sense to me also as the first check isn't technically a
validation but rather a case we ignore.
With that there should be minimal code in the no_error code path and no
reason for not marking blit_framebuffer() as ALWAYS_INLINE.
To avoid all the error code being inline twice into the
blit_framebuffer() callers you could simply wrap it in a static function
e.g.
void static
blit_framebuffer_err(...) {
/* We are wrapping the err variant of the always inlined
* blit_framebuffer() to avoid inlining it in every caller.
*/
blit_framebuffer(...);
}
We could create same type of wrapper for the no_error path also but it's
going to make less of a difference.
It might be interesting to compare the output from gcc before and after
to see if gcc is smart enough to do this all for us but being explicit
about it removes any doubt. I suspect gcc won't inline this code on it's
own.
+{
+ struct gl_renderbuffer *readRb =
+ readFb->Attachment[BUFFER_STENCIL].Renderbuffer;
+ struct gl_renderbuffer *drawRb =
+ drawFb->Attachment[BUFFER_STENCIL].Renderbuffer;
+
+ /* From the EXT_framebuffer_object spec:
+ *
+ * "If a buffer is specified in <mask> and does not exist in both
+ * the read and draw framebuffers, the corresponding bit is silently
+ * ignored."
+ */
+ if (readRb == NULL || drawRb == NULL) {
+ *mask &= ~GL_STENCIL_BUFFER_BIT;
+ } else if (!no_error) {
+ int read_z_bits, draw_z_bits;
+
+ if (_mesa_is_gles3(ctx) && (drawRb == readRb)) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "%s(source and destination stencil buffer cannot be the "
+ "same)", func);
+ return;
+ }
+
+ if (_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) !=
+ _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) {
+ /* There is no need to check the stencil datatype here, because
+ * there is only one: GL_UNSIGNED_INT.
+ */
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "%s(stencil attachment format mismatch)", func);
+ return;
+ }
+
+ read_z_bits = _mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS);
+ draw_z_bits = _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS);
+
+ /* If both buffers also have depth data, the depth formats must match
+ * as well. If one doesn't have depth, it's not blitted, so we should
+ * ignore the depth format check.
+ */
+ if (read_z_bits > 0 && draw_z_bits > 0 &&
+ (read_z_bits != draw_z_bits ||
+ _mesa_get_format_datatype(readRb->Format) !=
+ _mesa_get_format_datatype(drawRb->Format))) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "%s(stencil attachment depth format mismatch)", func);
+ return;
+ }
+ }
+}
+
+static void
blit_framebuffer(struct gl_context *ctx,
struct gl_framebuffer *readFb, struct gl_framebuffer *drawFb,
GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
@@ -317,59 +373,8 @@ blit_framebuffer(struct gl_context *ctx,
}
}
- if (mask & GL_STENCIL_BUFFER_BIT) {
- struct gl_renderbuffer *readRb =
- readFb->Attachment[BUFFER_STENCIL].Renderbuffer;
- struct gl_renderbuffer *drawRb =
- drawFb->Attachment[BUFFER_STENCIL].Renderbuffer;
-
- /* From the EXT_framebuffer_object spec:
- *
- * "If a buffer is specified in <mask> and does not exist in both
- * the read and draw framebuffers, the corresponding bit is silently
- * ignored."
- */
- if ((readRb == NULL) || (drawRb == NULL)) {
- mask &= ~GL_STENCIL_BUFFER_BIT;
- }
- else {
- int read_z_bits, draw_z_bits;
-
- if (_mesa_is_gles3(ctx) && (drawRb == readRb)) {
- _mesa_error(ctx, GL_INVALID_OPERATION,
- "%s(source and destination stencil "
- "buffer cannot be the same)", func);
- return;
- }
-
- if (_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) !=
- _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) {
- /* There is no need to check the stencil datatype here, because
- * there is only one: GL_UNSIGNED_INT.
- */
- _mesa_error(ctx, GL_INVALID_OPERATION,
- "%s(stencil attachment format mismatch)", func);
- return;
- }
-
- read_z_bits = _mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS);
- draw_z_bits = _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS);
-
- /* If both buffers also have depth data, the depth formats must match
- * as well. If one doesn't have depth, it's not blitted, so we should
- * ignore the depth format check.
- */
- if (read_z_bits > 0 && draw_z_bits > 0 &&
- (read_z_bits != draw_z_bits ||
- _mesa_get_format_datatype(readRb->Format) !=
- _mesa_get_format_datatype(drawRb->Format))) {
-
- _mesa_error(ctx, GL_INVALID_OPERATION,
- "%s(stencil attachment depth format mismatch)", func);
- return;
- }
- }
- }
+ if (mask & GL_STENCIL_BUFFER_BIT)
+ validate_stencil_buffer(ctx, readFb, drawFb, &mask, false, func);
if (mask & GL_DEPTH_BUFFER_BIT) {
struct gl_renderbuffer *readRb =
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
