Re: [PATCH 03/10] d3d9: move GetDC check to actual d3d9_surface_GetDC call

[Henri Verbeet]

On 19 October 2015 at 19:07, Riccardo Bortolato <rikyz619(a)gmail.com> wrote:
> @@ -360,7 +379,6 @@ static const struct wined3d_parent_ops d3d9_surface_wined3d_parent_ops =
>  void surface_init(struct d3d9_surface *surface, struct wined3d_texture *wined3d_texture, unsigned int sub_resource_idx,
>          struct wined3d_surface *wined3d_surface, const struct wined3d_parent_ops **parent_ops)
>  {
> -    struct wined3d_resource_desc desc;
>      IDirect3DBaseTexture9 *texture;
>
> surface->IDirect3DSurface9_iface.lpVtbl = &d3d9_surface_vtbl;
> @@ -379,23 +397,6 @@ void surface_init(struct d3d9_surface *surface, struct wined3d_texture *wined3d_
>          IDirect3DBaseTexture9_Release(texture);
>      }
>
> -    wined3d_resource_get_desc(wined3d_surface_get_resource(wined3d_surface), &desc);
> -    switch (d3dformat_from_wined3dformat(desc.format))
> -    {
> -        case D3DFMT_A8R8G8B8:
> -        case D3DFMT_X8R8G8B8:
> -        case D3DFMT_R5G6B5:
> -        case D3DFMT_X1R5G5B5:
> -        case D3DFMT_A1R5G5B5:
> -        case D3DFMT_R8G8B8:
> -            surface->getdc_supported = TRUE;
> -            break;
> -
> -        default:
> -            surface->getdc_supported = FALSE;
> -            break;
> -    }
> -
>      *parent_ops = &d3d9_surface_wined3d_parent_ops;
>  }
> I guess you're moving this to avoid wined3d_texture_get_sub_resource()
> in surface_init(), but you don't really need that. You can get the
> format from the texture, since it's the same for all sub-resources.

[PATCH 03/10] d3d9: move GetDC check to actual d3d9_surface_GetDC call

[Riccardo Bortolato]

Yes, but since the only usage of getdc_supported is really in that
function, it feels pointless to store it anyway.

Ciao,
Riccardo

2015-10-20 16:37 GMT+02:00 Henri Verbeet <hverbeet(a)gmail.com>:
> On 19 October 2015 at 19:07, Riccardo Bortolato <rikyz619(a)gmail.com> wrote:
> > @@ -360,7 +379,6 @@ static const struct wined3d_parent_ops d3d9_surface_wined3d_parent_ops =
> >  void surface_init(struct d3d9_surface *surface, struct wined3d_texture *wined3d_texture, unsigned int sub_resource_idx,
> >          struct wined3d_surface *wined3d_surface, const struct wined3d_parent_ops **parent_ops)
> >  {
> > -    struct wined3d_resource_desc desc;
> >      IDirect3DBaseTexture9 *texture;
> >
> > surface->IDirect3DSurface9_iface.lpVtbl = &d3d9_surface_vtbl;
> > @@ -379,23 +397,6 @@ void surface_init(struct d3d9_surface *surface, struct wined3d_texture *wined3d_
> >          IDirect3DBaseTexture9_Release(texture);
> >      }
> >
> > -    wined3d_resource_get_desc(wined3d_surface_get_resource(wined3d_surface), &desc);
> > -    switch (d3dformat_from_wined3dformat(desc.format))
> > -    {
> > -        case D3DFMT_A8R8G8B8:
> > -        case D3DFMT_X8R8G8B8:
> > -        case D3DFMT_R5G6B5:
> > -        case D3DFMT_X1R5G5B5:
> > -        case D3DFMT_A1R5G5B5:
> > -        case D3DFMT_R8G8B8:
> > -            surface->getdc_supported = TRUE;
> > -            break;
> > -
> > -        default:
> > -            surface->getdc_supported = FALSE;
> > -            break;
> > -    }
> > -
> >      *parent_ops = &d3d9_surface_wined3d_parent_ops;
> >  }
> > I guess you're moving this to avoid wined3d_texture_get_sub_resource()
> > in surface_init(), but you don't really need that. You can get the
> > format from the texture, since it's the same for all sub-resources.

[PATCH 03/10] d3d9: move GetDC check to actual d3d9_surface_GetDC call

[Henri Verbeet]

On 20 October 2015 at 16:39, Riccardo Bortolato <rikyz619(a)gmail.com> wrote:
> Yes, but since the only usage of getdc_supported is really in that
> function, it feels pointless to store it anyway.

IIRC the original reason for storing the result in a flag was to avoid
recalculating it every time GetDC() is called. GetDC() is perhaps not
extremely performance critical, but it will still be called a fair
amount by applications that use it. It's possible to move it, but
without a clear need that would need some performance data to back it
up.