[PATCHv4 2/3] wined3d: Use an rbtree for storing shaders for texture format conversion/blitting

[Martin Storsjö]

On Fri, 14 Feb 2014, Henri Verbeet wrote:

> On 13 February 2014 13:05, Martin Storsjo <martin at martin.st> wrote:
>>  static HRESULT arbfp_blit_alloc(struct wined3d_device *device)
>>  {
>> +    struct arbfp_blit_priv *priv;
>>      device->blit_priv = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct arbfp_blit_priv));
>>      if(!device->blit_priv) {
>>          ERR("Out of memory\n");
>>          return E_OUTOFMEMORY;
>>      }
>> +    priv = device->blit_priv;
>> +
>> +    if (wine_rb_init(&priv->shaders, &wined3d_arbfp_blit_rb_functions) == -1)
>> +    {
>> +        ERR("Failed to initialize rbtree.\n");
>> +        HeapFree(GetProcessHeap(), 0, device->blit_priv);
>> +        device->blit_priv = NULL;
>> +        return E_OUTOFMEMORY;
>> +    }
>> +
>>      return WINED3D_OK;
>>  }
>>
> The following is probably slightly nicer:
>
> static HRESULT arbfp_blit_alloc(struct wined3d_device *device)
> {
> -    device->blit_priv = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
> sizeof(struct arbfp_blit_priv));
> -    if(!device->blit_priv) {
> -        ERR("Out of memory\n");
> +    struct arbfp_blit_priv *priv;
> +
> +    if (!(priv = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*priv))))
>         return E_OUTOFMEMORY;
> +
> +    if (wine_rb_init(&priv->shaders, &wined3d_arbfp_blit_rb_functions) == -1)
> +    {
> +        ERR("Failed to initialize rbtree.\n");
> +        HeapFree(GetProcessHeap(), 0, priv);
> +        return E_FAIL;
>     }
> +
> +    device->blit_priv = priv;
> +
>     return WINED3D_OK;
> }

That's nicer indeed, updated the patch accordingly locally.

>> +    if (desc)
>> +        HeapFree(GetProcessHeap(), 0, desc);
> There's no point in checking for NULL before HeapFree().

MSDN says "If this pointer is NULL, the behavior is undefined" about the 
lpMem parameter to HeapFree.

> Perhaps more importantly, I think it makes more sense to insert the 
> rbtree entries in arbfp_blit_set(), instead of duplicating the code 
> between gen_p8_shader() and gen_yuv_shader().

Good point, I've updated my patch accordingly.

I'll resend all three patches in a little while unless you or Stefan have 
got more comments at the moment.

// Martin