On 10.05.2017 02:28, Eric Anholt wrote:
Timothy Arceri <tarc...@itsqueeze.com> writes:--- src/mapi/glapi/gen/ARB_framebuffer_object.xml | 2 +- src/mesa/main/fbobject.c | 31 +++++++++++++++++++++++++++ src/mesa/main/fbobject.h | 4 ++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/mapi/glapi/gen/ARB_framebuffer_object.xml b/src/mapi/glapi/gen/ARB_framebuffer_object.xml index ce5e45d..76114eb 100644 --- a/src/mapi/glapi/gen/ARB_framebuffer_object.xml +++ b/src/mapi/glapi/gen/ARB_framebuffer_object.xml @@ -239,21 +239,21 @@ <function name="FramebufferTexture3D" no_error="true"> <param name="target" type="GLenum"/> <param name="attachment" type="GLenum"/> <param name="textarget" type="GLenum"/> <param name="texture" type="GLuint"/> <param name="level" type="GLint"/> <param name="layer" type="GLint"/> <glx rop="4323"/> </function> - <function name="FramebufferTextureLayer" es2="3.0"> + <function name="FramebufferTextureLayer" es2="3.0" no_error="true"> <param name="target" type="GLenum"/> <param name="attachment" type="GLenum"/> <param name="texture" type="GLuint"/> <param name="level" type="GLint"/> <param name="layer" type="GLint"/> <glx rop="237"/> </function> <function name="FramebufferRenderbuffer" es2="2.0"> <param name="target" type="GLenum"/> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index ba01d0c..08e7347 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -3424,20 +3424,51 @@ void GLAPIENTRY _mesa_FramebufferTexture3D(GLenum target, GLenum attachment, GLenum textarget, GLuint texture, GLint level, GLint layer) { framebuffer_texture_with_dims(3, target, attachment, textarget, texture, level, layer, "glFramebufferTexture3D"); } void GLAPIENTRY +_mesa_FramebufferTextureLayer_no_error(GLenum target, GLenum attachment, + GLuint texture, GLint level, + GLint layer) +{ + GET_CURRENT_CONTEXT(ctx); + + /* Get the framebuffer object */ + struct gl_framebuffer *fb = get_framebuffer_target(ctx, target); + + /* Get the texture object */ + struct gl_texture_object *texObj = + get_texture_for_framebuffer(ctx, texture); + + struct gl_renderbuffer_attachment *att = + get_attachment(ctx, fb, attachment, NULL); + + GLenum textarget = 0; + if (texObj) { + if (texObj->Target == GL_TEXTURE_CUBE_MAP) { + assert(layer >= 0 && layer < 6); + textarget = GL_TEXTURE_CUBE_MAP_POSITIVE_X + layer; + layer = 0; + } + } + + _mesa_framebuffer_texture(ctx, fb, attachment, att, texObj, textarget, + level, layer, GL_FALSE); +}I've got stuck on reading this series a couple of times because this felt like too much code duplication. I understand that you're replicating what the current error-validating code does, but it seems like for no_error we can do a lot better, quite easily. What I'd like for these last 4 patches is to have two patches, each of which implements a Named and non-Named no_error function by doing the appropriate lookup of the fb and then calling a helper function that does the rest of their current bodies.
Not to be too annoying, but I still think that collapsing the functions entirely (and using ALWAYS_INLINE) is a good idea as well.
Cheers, Nicolai
Assuming patch 4.5 is to be squashed into 4, then other than the ';' comment patches 1-9 are: Reviewed-by: Eric Anholt <e...@anholt.net> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
