[v2 2/4] wined3d: make wined3d_device_get_front_buffer_data work with wined3d_texture and sub_resource_index

[Riccardo Bortolato]

Hi Henri,

in a later patch I modified wined3d_swapchain_get_front_buffer_data()
to work with texture + sub index (and that semplifies
wined3d_device_get_front_buffer_data() as well).
However the issue you mentioned still stands, would something like:

    if (!sub_resource || sub_resource->type ==
WINED3D_RTYPE_VOLUME_TEXTURE || !(swapchain =
wined3d_device_get_swapchain(device, swapchain_idx)))
        return WINED3DERR_INVALIDCALL;

be ok for you?

Or I could just submit a patch that calls directly the updated
wined3d_swapchain_get_front_buffer_data() while removing the
wined3d_device_get_front_buffer_data() function (either that or first
modify  wined3d_swapchain_get_front_buffer_data() and then remove
wined3d_device_get_front_buffer_data()). The check would be on the
same grounds though (check sub_resource type be a volume texture)

Ciao,
Riccardo

2015-10-15 12:29 GMT+02:00 Henri Verbeet <hverbeet at gmail.com>:
> On 13 October 2015 at 18:45, Riccardo Bortolato <rikyz619 at gmail.com> wrote:
>>  HRESULT CDECL wined3d_device_get_front_buffer_data(const struct wined3d_device *device,
>> -        UINT swapchain_idx, struct wined3d_surface *dst_surface)
>> +        UINT swapchain_idx, struct wined3d_texture *texture, unsigned int sub_resource_idx)
>>  {
>> +    struct wined3d_resource *sub_resource;
>>      struct wined3d_swapchain *swapchain;
>>
>> -    TRACE("device %p, swapchain_idx %u, dst_surface %p.\n", device, swapchain_idx, dst_surface);
>> +    TRACE("device %p, swapchain_idx %u, texture %p, sub_resource_idx %u.\n", device, swapchain_idx, texture, sub_resource_idx);
>>
>> -    if (!(swapchain = wined3d_device_get_swapchain(device, swapchain_idx)))
>> +    sub_resource = wined3d_texture_get_sub_resource(texture, sub_resource_idx);
>> +
>> +    if (!sub_resource || !(swapchain = wined3d_device_get_swapchain(device, swapchain_idx)))
>>          return WINED3DERR_INVALIDCALL;
>>
>> -    return wined3d_swapchain_get_front_buffer_data(swapchain, dst_surface);
>> +    return wined3d_swapchain_get_front_buffer_data(swapchain, surface_from_resource(sub_resource));
>>  }
> It's a little harder than that, I'm afraid. In particular, you can't
> assume the sub-resource is a surface here. (I.e., "texture" could be a
> volume texture.) In general, the rule is that anywhere you use
> surface_from_resource() you should make sure the resource is actually
> the surface in some way. In this specific case, the answer is probably
> that wined3d_device_get_front_buffer_data() should just go away, and
> callers should use wined3d_swapchain_get_front_buffer_data() directly
> instead. Of course that only slightly moves the problem, so
> wined3d_swapchain_get_front_buffer_data() should then verify that it
> gets an appropriate texture when you make the equivalent change there.
>
>