Re: [v2 2/4] wined3d: make wined3d_device_get_front_buffer_data work with wined3d_texture and sub_resource_index
On 13 October 2015 at 18:45, Riccardo Bortolato <rikyz619(a)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.
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(a)gmail.com>:
On 13 October 2015 at 18:45, Riccardo Bortolato <rikyz619(a)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.
On 15 October 2015 at 13:30, Riccardo Bortolato <rikyz619(a)gmail.com> wrote:
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?
Well, that one doesn't work, you either need to check "sub_resource" against WINED3D_RTYPE_SURFACE, or "texture" against WINED3D_RTYPE_VOLUME_TEXTURE. But conceptually, sure, you can just check the resource type.
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)
I'd first get rid of wined3d_device_get_front_buffer_data(), then modify wined3d_swapchain_get_front_buffer_data().
participants (2)
-
Henri Verbeet -
Riccardo Bortolato