Re: [PATCH 3/6] d3drm: Implement IDirect3DRM*_CreateTexture. (v3)

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Am 2016-05-08 um 11:31 schrieb Aaryaman Vasishta:
> +    if (!d3drm_validate_image(image))
> +        return D3DRMERR_BADVALUE;
> Is this necessary? InitFromImage does it anyway I think.
>
> if (FAILED(hr = d3drm_texture_create(&object)))
>          return hr;
> You could call the public CreateObject method here, but I don't have a strong opinion on this.
> +    object->d3drm = &d3drm->IDirect3DRM_iface;
> Afaics InitFromImage does it anyway. It also causes problems when InitFromImage fails for some reason - you'll decref the d3drm object in the error path below.
> +    if (FAILED(hr = IDirect3DRMTexture3_InitFromImage(&object->IDirect3DRMTexture3_iface, image)))
> +    {
> +        d3drm_texture_destroy(object);
> +        return hr;
> +    }

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXL2HbAAoJEN0/YqbEcdMw9JcP/iXXJN5PuvyuLqki+eKgQHCv
fdRtBfdm57foN7+hfOcG9hZGgVWpdPMj5YFZmNhCaWKlnNjcxKDJE/ZsjsRaYXmD
pKPb6mLbNcZVBHyNeqbRcwlgJBiDLYIHM8BiT5DwssQK2lo8UmhVGJgmNZNPlGy8
sw/drdMg0lW11N6rOZR/fXcm5NY9F7oIAuI9945uElbOXg3byBvLAQdpPE95yJ5p
8rAQotCsbnqzGFEyRw3E1+0uKplBoaXd6ML+jhDrHK9Eqw0XWUtUsCuWiaYkTDGX
82XXWjhjsxQ84GkSKOweqtum6HjGDSnSZz12YTDMbspLywIpKzSMO5+O/dtxsolI
a5scAHlpYQ/bLK3TCWOkwNEsMkXBRjM8+j+PX1I3SQICdl3GmNxVo/b4ZU+0Wqt5
GwzyerUB2an5Dyf/+l7jqH0SxWgcrIiwa/ZTCxttAafDSooFhERwj402WhNQcZLN
BD9Jef52AYCJrduV3b3udvTyPuWCrPkkrm3NTSO6dbCIZG9EzMcujJZnA0D52y2E
qwfpfsfNNUJ7hkS1322u48i7GaSCZNoK6ISlA9RqtjVoRCGZKG/nGTwQgznrclB1
mvR24DUvAmogeTV0Akm20wUN3/n5sIkz47b0vb9Z5Jjg1v+90nTQ43pcMT0GCJhP
cN0YfnUAgpi7yD4yfA1E
=JJ9X
-----END PGP SIGNATURE-----

[PATCH 3/6] d3drm: Implement IDirect3DRM*_CreateTexture. (v3)

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Am 2016-05-08 um 16:57 schrieb Stefan Dösinger:
> > > +    object->d3drm = &d3drm->IDirect3DRM_iface;
> > > Afaics InitFromImage does it anyway. It also causes problems when 
> > > InitFromImage fails for some reason - you'll decref the d3drm 
> > > object in the error path below.
> > > Er, I misread this part - InitFromImage doesn't assign texture->d3drm,
> > > and the decref is tied to having an image, not having d3drm set.

Still it seems to be duplicated logic to set this here.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXL2paAAoJEN0/YqbEcdMwsiIP/0q9Hc50GEAB4UFxq0SxSz52
QQutknkDu/M/8ozGSr58xt/SzaSDO+Ij71nNRAf050bEBHuoxpZyqR8zYeW9oSDt
tMLHxm33MIsbb1U2AtqEdl6v/uQHS3weJwqCIUL9m1u6ZvkY119MsM41IWh/Kuim
yqspcGZAp4CPeKNvh5ynQAk49ElUJN1EDQ2fARlRSIAz9BZh2MZc30eIhc/pwQ3Q
sD7Y8TPCVIp8p7r+yVlvAKgdxMTNQOlPg52O04UK54o/UHFyeqv0c8ANxK5Zpa5a
4bHxvCMOXlZw67ML17VGI3tjABhC2fjYg+YeNLS98IACE7YZL/ui7SeRmN3KmTLJ
DwNyIFqjtistwDP8x5rPKnZZmQ09AjNy8tR5vW/XOUf3rmWbtIz+EWMZd/P4zeRc
88kI9JgDZ+niw9yUpcnkxCIdMKZO4cU0gtENYGDJBeL8WF+MTSomiDdTbThYufQs
4XPFgF7LBlbmp6fkOwyypyaKHr2/l7f/K0MTHGsdf8FKeYIOCKcJuKon8oYxGDEo
N3iuZAwqR9CVg2kgajwvddUMq75tz76XPSaXdwG+rMinrv1cNVUMNRjSuvlKosN3
y24Gav7P86WY8LPCAzqwULE1cb0Erule0kvSoMSXEmkIqez6lX41sQxXU0lWDrMy
lvPtRcV8JZhhjklqZ69l
=9sKX
-----END PGP SIGNATURE-----

[PATCH 3/6] d3drm: Implement IDirect3DRM*_CreateTexture. (v3)

[Aaryaman Vasishta]

On Sun, May 8, 2016 at 9:27 PM, Stefan Dösinger <stefandoesinger(a)gmail.com>
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Am 2016-05-08 um 11:31 schrieb Aaryaman Vasishta:
> > +    if (!d3drm_validate_image(image))
> > +        return D3DRMERR_BADVALUE;
> > Is this necessary? InitFromImage does it anyway I think.

CreateTexture doesn't leak a reference to d3drm1 if the image is invalid.
Although InitFromImage does. I could probably release d3drm1 on failure of
InitFromImage instead of validating the image again like this.
> > if (FAILED(hr = d3drm_texture_create(&object)))
> >          return hr;
>
> You could call the public CreateObject method here, but I don't have a
> strong opinion on this.

I just felt that it'd be better than going through multiple calls just to
get to d3drm_texture_create. Though the difference in performance does seem
neglibile. I'll use CreateObject if you want.


Cheers,
Aaryaman