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

[Nicolai Hähnle]

On 26.05.2017 14:09, Samuel Pitoiset wrote:
> 
> 
> 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.

You're right, I missed that. This part is fine.

Cheers,
Nicolai


> 
>>
>> 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


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.