On Thursday 13 February 2014, Francisco Jerez wrote:
> Fredrik Höglund <fred...@kde.org> writes:
>
> > ---
> > src/mesa/main/shaderimage.c | 164
> > +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 164 insertions(+)
> >
> > diff --git a/src/mesa/main/shaderimage.c b/src/mesa/main/shaderimage.c
> > index ce63bee..27695a3 100644
> > --- a/src/mesa/main/shaderimage.c
> > +++ b/src/mesa/main/shaderimage.c
> > @@ -33,6 +33,7 @@
> > #include "context.h"
> > #include "texobj.h"
> > #include "teximage.h"
> > +#include "enums.h"
> >
> > /*
> > * Define endian-invariant aliases for some mesa formats that are
> > @@ -481,6 +482,169 @@ _mesa_BindImageTexture(GLuint unit, GLuint texture,
> > GLint level,
> > void GLAPIENTRY
> > _mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint
> > *textures)
> > {
> > + GET_CURRENT_CONTEXT(ctx);
> > + int i;
> > +
> > + if (!ctx->Extensions.ARB_shader_image_load_store) {
> > + _mesa_error(ctx, GL_INVALID_OPERATION, "glBindImageTextures()");
> > + return;
> > + }
> > +
> > + if (first + count > ctx->Const.MaxImageUnits) {
> > + /* The ARB_multi_bind spec says:
> > + *
> > + * "An INVALID_OPERATION error is generated if <first> + <count>
> > + * is greater than the number of image units supported by
> > + * the implementation."
> > + */
> > + _mesa_error(ctx, GL_INVALID_OPERATION,
> > + "glBindImageTextures(first=%u + count=%u > the value of "
> > + "GL_MAX_IMAGE_UNITS=%u)",
> > + first, count, ctx->Const.MaxImageUnits);
> > + return;
> > + }
> > +
> > + /* Assume that at least one binding will be changed */
> > + FLUSH_VERTICES(ctx, 0);
> > + ctx->NewDriverState |= ctx->DriverFlags.NewImageUnits;
> > +
> > + if (!textures) {
> > + /* The ARB_multi_bind spec says:
> > + *
> > + * "If <textures> is NULL, each affected image unit from
> > + * <first> through <first>+<count>-1 will be reset to have
> > + * no bound texture object."
> > + */
> > + for (i = 0; i < count; i++) {
> > + struct gl_image_unit *u = &ctx->ImageUnits[first + i];
> > +
> > + u->Level = 0;
> > + u->Layered = GL_FALSE;
> > + u->Layer = 0;
> > + u->Access = GL_READ_ONLY;
> > + u->Format = GL_R8;
> > + u->_ActualFormat = MESA_FORMAT_R8;
> > + u->_Valid = GL_FALSE;
> > + _mesa_reference_texobj(&u->TexObj, NULL);
> > +
> > + if (ctx->Driver.BindImageTexture)
> > + ctx->Driver.BindImageTexture(ctx, u, NULL, 0, GL_FALSE,
> > + 0, GL_READ_ONLY, GL_R8);
> > + }
> > +
> > + return;
> > + }
>
> Can't you get rid of the whole block above by declaring 'const GLuint
> texture = (textures ? textures[i] : 0);' in the loop below, and then
> 's/textures[i]/texture/'?
I could, but I thought it might be worth optimizing this case. Note that
the hash table mutex is not locked in this block. I don't know if it
actually makes a difference for any real applications though.
But the actual unbinding code could be moved into a helper function.
> > +
> > + /* Note that the error semantics for multi-bind commands differ from
> > + * those of other GL commands.
> > + *
> > + * The Issues section in the ARB_multi_bind spec says:
> > + *
> > + * "(11) Typically, OpenGL specifies that if an error is generated by
> > + * a command, that command has no effect. This is somewhat
> > + * unfortunate for multi-bind commands, because it would
> > require
> > + * a first pass to scan the entire list of bound objects for
> > + * errors and then a second pass to actually perform the
> > + * bindings. Should we have different error semantics?
> > + *
> > + * RESOLVED: Yes. In this specification, when the parameters for
> > + * one of the <count> binding points are invalid, that binding
> > + * point is not updated and an error will be generated. However,
> > + * other binding points in the same command will be updated if
> > + * their parameters are valid and no other error occurs."
> > + */
> > +
> > + _mesa_begin_texture_lookups(ctx);
> > +
> > + for (i = 0; i < count; i++) {
> > + struct gl_image_unit *u = &ctx->ImageUnits[first + i];
> > +
> > + if (textures[i] != 0) {
> > + struct gl_texture_object *texObj;
> > + struct gl_texture_image *image;
> > + gl_format actualFormat;
> > +
> > + if (!u->TexObj || u->TexObj->Name != textures[i]) {
> > + texObj = _mesa_lookup_texture_without_locking(ctx,
> > textures[i]);
> > + if (!texObj) {
> > + /* The ARB_multi_bind spec says:
> > + *
> > + * "An INVALID_OPERATION error is generated if any value
> > + * in <textures> is not zero or the name of an existing
> > + * texture object (per binding)."
> > + */
> > + _mesa_error(ctx, GL_INVALID_OPERATION,
> > + "glBindImageTextures(textures[%u]=%u "
> > + "is not zero or the name of an existing texture
> > "
> > + "object)", i, textures[i]);
> > + continue;
> > + }
> > + } else {
> > + texObj = u->TexObj;
> > + }
> > +
> > + image = texObj->Image[0][0];
> > +
> > + if (image->Width == 0 || image->Height == 0 || image->Depth == 0)
> > {
> > + /* The ARB_multi_bind spec says:
> > + *
> > + * "An INVALID_OPERATION error is generated if the width,
> > + * height, or depth of the level zero texture image of
> > + * any texture in <textures> is zero (per binding)."
> > + */
> > + _mesa_error(ctx, GL_INVALID_OPERATION,
> > + "glBindImageTextures(the width, height or depth "
> > + "of the level zero texture image of "
> > + "textures[%u]=%u is zero)", i, textures[i]);
> > + continue;
> > + }
> > +
> > + actualFormat = get_image_format(image->InternalFormat);
> > +
> > + if (actualFormat == MESA_FORMAT_NONE) {
> > + /* The ARB_multi_bind spec says:
> > + *
> > + * "An INVALID_OPERATION error is generated if the internal
> > + * format of the level zero texture image of any texture
> > + * in <textures> is not found in table 8.33 (per binding)."
> > + */
> > + _mesa_error(ctx, GL_INVALID_OPERATION,
> > + "glBindImageTextures(the internal format %s of "
> > + "the level zero texture image of textures[%u]=%u "
> > + "is not supported)",
> > + _mesa_lookup_enum_by_nr(image->InternalFormat),
> > + i, textures[i]);
> > + continue;
> > + }
>
> I wish you could reuse validate_bind_image_texture() for this... Pity
> that that both extensions require returning different error codes for
> the same errors...
That's presumably because the values are not supplied in parameters.
> > +
> > + /* Update the texture binding */
> > + _mesa_reference_texobj(&u->TexObj, texObj);
> > + u->Level = 0;
> > + u->Layered = _mesa_tex_target_is_layered(texObj->Target);
> > + u->Layer = 0;
> > + u->Access = GL_READ_WRITE;
> > + u->Format = image->InternalFormat;
> > + u->_ActualFormat = actualFormat;
> > + u->_Valid = validate_image_unit(ctx, u);
> > + } else {
> > + /* Unbind the texture from the unit */
> > + _mesa_reference_texobj(&u->TexObj, NULL);
> > + u->Level = 0;
> > + u->Layered = GL_FALSE;
> > + u->Layer = 0;
> > + u->Access = GL_READ_ONLY;
> > + u->Format = GL_R8;
> > + u->_ActualFormat = MESA_FORMAT_R8;
> > + u->_Valid = GL_FALSE;
>
> You can just unref 'u->TexObj' and set 'u->_Valid' to GL_FALSE here.
Unfortunately the specification says that all parameters are reset to their
default values when a texture is unbound.
> > + }
> > +
> > + /* Pass the BindImageTexture call down to the device driver */
> > + if (ctx->Driver.BindImageTexture)
> > + ctx->Driver.BindImageTexture(ctx, u, u->TexObj, u->Level,
> > u->Layered,
> > + u->Layer, u->Access, u->Format);
> > + }
> > +
> > + _mesa_end_texture_lookups(ctx);
> > }
> >
> > void GLAPIENTRY
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
