[Mesa-dev] [RFC PATCH 02/65] mesa: implement ARB_bindless_texture

[Samuel Pitoiset]

On 05/24/2017 12:10 PM, Nicolai Hähnle wrote:
> [snip]
>>>> +static GLuint64
>>>> +get_texture_handle(struct gl_context *ctx, struct gl_texture_object 
>>>> *texObj,
>>>> +                   struct gl_sampler_object *sampObj)
>>>> +{
>>>> +   struct gl_texture_handle_object *handleObj;
>>>> +   struct hash_entry *entry;
>>>> +   GLuint64 handle;
>>>> +
>>>> +   handleObj = CALLOC_STRUCT(gl_texture_handle_object);
>>>> +   if (!handleObj) {
>>>> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glGetTexture*HandleARB()");
>>>> +      return 0;
>>>> +   }
>>>> +
>>>> +   handleObj->texObj = texObj;
>>>> +   if (&texObj->Sampler != sampObj)
>>>> +      handleObj->sampObj = sampObj;
>>>> +
>>>> +   /* The ARB_bindless_texture spec says:
>>>> +    *
>>>> +    * "The handle for each texture or texture/sampler pair is 
>>>> unique; the same
>>>> +    *  handle will be returned if GetTextureHandleARB is called 
>>>> multiple times
>>>> +    *  for the same texture or if GetTextureSamplerHandleARB is 
>>>> called multiple
>>>> +    *  times for the same texture/sampler pair."
>>>> +    */
>>>> +   mtx_lock(&ctx->Shared->HandlesMutex);
>>>> +   entry = _mesa_hash_table_search(texObj->SamplerHandles, handleObj);
>>>> +   if (entry) {
>>>> +      mtx_unlock(&ctx->Shared->HandlesMutex);
>>>> +      free(handleObj);
>>>> +      return (uint64_t)entry->data;
>>>> +   }
>>>> +
>>>> +   /* Ask the driver for a new handle and store it. */
>>>> +   handle = ctx->Driver.NewTextureHandle(ctx, texObj, sampObj);
>>>> +   if (!handle) {
>>>> +      mtx_unlock(&ctx->Shared->HandlesMutex);
>>>> +      free(handleObj);
>>>> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glGetTexture*HandleARB()");
>>>> +      return 0;
>>>> +   }
>>>> +
>>>> +   _mesa_hash_table_insert(texObj->SamplerHandles, handleObj, (void 
>>>> *)handle);
>>>> +
>>>> +   if (&texObj->Sampler != sampObj) {
>>>> +      _mesa_hash_table_insert(sampObj->Handles, handleObj,
>>>> +                              (void *)handle);
>>>> +   }
>>>> +
>>>> +   /* When referenced by one or more handles, texture objects are 
>>>> immutable. */
>>>> +   texObj->HandleAllocated = true;
>>>> +   sampObj->HandleAllocated = true;
>>>> +   if (texObj->Target == GL_TEXTURE_BUFFER)
>>>> +      texObj->BufferObject->HandleAllocated = true;
>>>> +
>>>> +   /* Store the handle in the shared state for all contexts. */
>>>> +   _mesa_hash_table_insert(ctx->Shared->TextureHandles,
>>>> +                           (void *)handle, handleObj);
>>>
>>> This won't work on 32-bit systems (same for image handles).
>>
>> Because the handle is 64-bit? Mmh...
> 
> Right.
> 
> 
> [snip]
>>>> +void
>>>> +_mesa_make_texture_handles_non_resident(struct gl_context *ctx,
>>>> +                                        struct gl_texture_object 
>>>> *texObj)
>>>> +{
>>>> +   struct hash_entry *entry;
>>>> +   GLuint64 handle;
>>>> +
>>>> +   mtx_lock(&ctx->Shared->HandlesMutex);
>>>> +
>>>> +   hash_table_foreach(texObj->SamplerHandles, entry) {
>>>> +      struct gl_texture_handle_object *handleObj =
>>>> +         (struct gl_texture_handle_object *)entry->key;
>>>> +
>>>> +      handle = (uint64_t)entry->data;
>>>> +      if (is_texture_handle_resident(ctx, handle))
>>>> +         make_texture_handle_resident(ctx, handleObj, handle, false);
>>>> +   }
>>>> +
>>>> +   hash_table_foreach(texObj->ImageHandles, entry) {
>>>> +      struct gl_image_handle_object *handleObj =
>>>> +         (struct gl_image_handle_object *)entry->key;
>>>> +
>>>> +      handle = (uint64_t)entry->data;
>>>> +      if (is_image_handle_resident(ctx, handle))
>>>> +         make_image_handle_resident(ctx, handleObj, handle, 
>>>> GL_READ_ONLY, false);
>>>> +   }
>>>
>>> So... this also needs to loop through all other contexts and make the 
>>> handle non-resident in them, right? Otherwise you might end up with 
>>> dangling pointers (or at least dangling handles).
>>
>> No. Resident handles are per-context. Though, I'm not very happy 
>> myself with the way I managed the handles. I'm open to any better 
>> suggestions.
> 
> Right, resident handles are per-context, but the handles of a texture 
> might be resident in multiple contexts simultaneously. What happens if 
> they are, and then the texture object is deleted?

The ARB_bindless_texture spec says:

(7) What happens if you try to delete a texture or sampler object with a 
handle that is resident in another context?

RESOLVED:  Deleting the texture will remove the texture from the name 
space and make all handles using the texture non-resident in the current 
context.  However, texture or image handles for a deleted texture are
not deleted until the underlying texture or sampler object itself is 
deleted.  That deletion won't happen until the object is not bound 
anywhere and there are no handles using the object that are resident in
any context.

Currently, my implementation does exactly this. I think the app should 
handle this.

> 
> It seems to me that right now, you end up with dangling resident handles 
> in the "other" contexts. At the very least, this can cause incorrect 
> errors when the handle is re-used by the driver for another texture, so 
> *something* needs to loop over all contexts, it seems...
> 
> Cheers,
> Nicolai