Re: [PATCH 2/6] d3drm: Implement IDirect3DRMTexture{2-3}_InitFromImage. (v4)

[Henri Verbeet]

On 11 May 2016 at 16:43, Aaryaman Vasishta  wrote:
> @@ -46,10 +46,27 @@ static inline struct d3drm_texture *impl_from_IDirect3DRMTexture3(IDirect3DRMTex
>
> void d3drm_texture_destroy(struct d3drm_texture *texture)
>  {
> - if (texture->image)
> -
>    IDirect3DRM_Release(texture->d3drm);
> d3drm_object_cleanup((IDirect3DRMObject*)&texture->IDirect3DRMTexture3_iface, &texture->obj);

It's somewhat hypothetical, but it seems safer to release the
IDirect3DRM interface after the d3drm_object_cleanup() call, in case
there are callbacks that (implicitly) depend on the Direct3DRM object
still existing. (E.g., is it possible to call
IDirect3DRMObject2::GetDirect3DRM() on textures?)
>  HeapFree(GetProcessHeap(), 0, texture);
>
> }
>
> +BOOL d3drm_validate_image(D3DRMIMAGE *image)
> +{
> - if (!image
> -
>        || !image->red_mask
> -
>        || !image->green_mask
> -
>        || !image->blue_mask
> -
>        || !image->buffer1
> -
>        || !(image->rgb || (image->palette && image->palette_size)))
> - {
> -
>    return FALSE;
> - }
> -
> - return TRUE;
>
> +}

Like d3drm_texture_destroy(), you're not using this outside texture.c.

[PATCH 2/6] d3drm: Implement IDirect3DRMTexture{2-3}_InitFromImage. (v4)

[Aaryaman Vasishta]

On Wed, May 11, 2016 at 9:13 PM, Henri Verbeet  wrote:
> On 11 May 2016 at 16:43, Aaryaman Vasishta 
> wrote:
> > @@ -46,10 +46,27 @@ static inline struct d3drm_texture
>
> *impl_from_IDirect3DRMTexture3(IDirect3DRMTex
> > void d3drm_texture_destroy(struct d3drm_texture *texture)
> >  {
> > - if (texture->image)
> > -
> >    IDirect3DRM_Release(texture->d3drm);
>
> d3drm_object_cleanup((IDirect3DRMObject*)&texture->IDirect3DRMTexture3_iface,
> &texture->obj);
> It's somewhat hypothetical, but it seems safer to release the
> IDirect3DRM interface after the d3drm_object_cleanup() call, in case
> there are callbacks that (implicitly) depend on the Direct3DRM object
> still existing. (E.g., is it possible to call
> IDirect3DRMObject2::GetDirect3DRM() on textures?)

Right, it makes sense once you take in future pathces in mind.
A quick test reveals that you it's possible to call
IDirect3DRMObject2::GetDirect3DRM() on textures (and probably any object
that inherits IDirect3DRMObject2) so this change does make sense for when
this interface is implemented.

I'll resent this patch as well.

Cheers,
Aaryaman