[Mesa-dev] [PATCH 1/4] gallium: add new pipe_screen::legal_resource_size() function

[Brian Paul]

On 09/17/2012 04:16 AM, Jose Fonseca wrote:
>
>
> ----- Original Message -----
>> From: Brian Paul<brianp at vmware.com>
>>
>> To implement proxy textures.  If a gallium driver doesn't implement
>> this function we'll just continue to use the core Mesa fallback code.
>>
>> Without this hook we really have no good way to implement OpenGL
>> proxy
>> textures with gallium drivers.
>
> Brian,
>
> This interface seems a good thing to add. A just have a few suggestion/remarks below.
>
>> ---
>>   src/gallium/include/pipe/p_screen.h |    8 ++++++++
>>   1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/gallium/include/pipe/p_screen.h
>> b/src/gallium/include/pipe/p_screen.h
>> index fdf6fa2..718a72b 100644
>> --- a/src/gallium/include/pipe/p_screen.h
>> +++ b/src/gallium/include/pipe/p_screen.h
>> @@ -138,6 +138,14 @@ struct pipe_screen {
>>                                            enum pipe_video_profile
>>                                            profile );
>>
>>      /**
>> +    * Check the size of the resource/texture given by 'res'.
>> +    * Used to implement proxy textures.
>> +    * \return TRUE if size is OK, FALSE if too large.
>> +    */
>> +   boolean (*legal_resource_size)(struct pipe_screen *screen,
>
> I think the name would benefit from a verb somewhere in it, e.g., is_resource_size_legal.
>
> I suspect this function will check more than size (e.g., number of samples, format). If so, then I'd suggest a more generic name, such as, "can_create_resource".
>
> Also, shouldn't this interface return an enum (e.g., pipe_error), so that one can distinguish between "can't create a texture because not enough memory now", vs "can't create this texture ever because of HW limitations", or is such distinction useless for GL proxy textures?
>
>> +                                  const struct pipe_resource *res);
>
> IIUC "res" is not a real resource but rather a template for a resource. If so, then it should be named "templat" for consistency.

OK, I'll change it to can_create_resource(screen, templat).

Thanks.

-Brian