Timothy Arceri <tarc...@itsqueeze.com> writes:
> On 10/05/17 22:00, Nicolai Hähnle wrote:
>> 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.
>
> Yeah I've done that with the Tex*SubImage fuctions in an upcoming series
> and I think I've changed my mind. I still think the extra complexity is
> a little ugly, but I think its worth it.
>
> I'll redo these patches to share the inline function between the dsa/no
> error variants also.I've been thinking that too. Thanks!
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
