Re: [PATCH] d3dx9_36: Implement D3DXCheckTextureRequirements + tests

[Henri Verbeet]

On 19 April 2010 09:43, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
> +BOOL is_pow2(UINT num)
> +{
> +    UINT i;
> +
> +    if (!num) return FALSE;
> +    for (i = 0; num >> 1; i++) num >>= 1;
> +
> +    return (num << i) == num;
> +}
> This should at least be static. You can also just replace it with
> "return !(num & (num - 1));". You don't care about 0, since you
> already check that earlier in D3DXCheckTextureRequirements().
> make_pow2() can probably be simplified as well.

[PATCH] d3dx9_36: Implement D3DXCheckTextureRequirements + tests

[Ben Klein]

On 19 April 2010 19:43, Henri Verbeet <hverbeet(a)gmail.com> wrote:
> On 19 April 2010 09:43, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
> > +BOOL is_pow2(UINT num)
> > +{
> > +    UINT i;
> > +
> > +    if (!num) return FALSE;
> > +    for (i = 0; num >> 1; i++) num >>= 1;
> > +
> > +    return (num << i) == num;
> > +}
> > This should at least be static. You can also just replace it with
> > "return !(num & (num - 1));". You don't care about 0, since you
> > already check that earlier in D3DXCheckTextureRequirements().
> > make_pow2() can probably be simplified as well.

I guess it depends on how many times this function will be used
whether or not 0 should be checked as well.

[PATCH] d3dx9_36: Implement D3DXCheckTextureRequirements + tests

[Christian Costa]

Ben Klein a écrit :
> On 19 April 2010 19:43, Henri Verbeet <hverbeet(a)gmail.com> wrote:
> > On 19 April 2010 09:43, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
> > > +BOOL is_pow2(UINT num)
> > > +{
> > > +    UINT i;
> > > +
> > > +    if (!num) return FALSE;
> > > +    for (i = 0; num >> 1; i++) num >>= 1;
> > > +
> > > +    return (num << i) == num;
> > > +}
> >
> > This should at least be static. You can also just replace it with
> > "return !(num & (num - 1));". You don't care about 0, since you
> > already check that earlier in D3DXCheckTextureRequirements().
> > make_pow2() can probably be simplified as well.
>
> I guess it depends on how many times this function will be used
> whether or not 0 should be checked as well.

Not many. The check can be done. Especially if these functions are moved 
to util.c for reuse.
That said, I'm not sure if it makes any difference because 0 as input 
does not make sense either.

[PATCH] d3dx9_36: Implement D3DXCheckTextureRequirements + tests

[paulo lesgaz]

Maybe this function should be in util.c.

A+

David


--- En date de : Lun 19.4.10, Henri Verbeet <hverbeet(a)gmail.com> a écrit :

De: Henri Verbeet <hverbeet(a)gmail.com>
Objet: Re: [PATCH] d3dx9_36: Implement D3DXCheckTextureRequirements + tests
À: wine-devel(a)winehq.org
Date: Lundi 19 avril 2010, 11h43

On 19 April 2010 09:43, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
> +BOOL is_pow2(UINT num)
> +{
> +    UINT i;
> +
> +    if (!num) return FALSE;
> +    for (i = 0; num >> 1; i++) num >>= 1;
> +
> +    return (num << i) == num;
> +}
> This should at least be static. You can also just replace it with
> "return !(num & (num - 1));". You don't care about 0, since you
> already check that earlier in D3DXCheckTextureRequirements().
> make_pow2() can probably be simplified as well.

[PATCH] d3dx9_36: Implement D3DXCheckTextureRequirements + tests

[Christian Costa]

is_pow2 was in util.c, then remove. That's why I put then in texture.c. 
But that's better to put them in util.c
just in case there are used from another part.

BTW, is your name Paulo, Jéremie or David ? ;-)

paulo lesgaz a écrit :
> Maybe this function should be in util.c.
>
> A+
>
> David
>
> --- En date de : *Lun 19.4.10, Henri Verbeet /<hverbeet(a)gmail.com>/* a 
> écrit :
>
> De: Henri Verbeet <hverbeet(a)gmail.com>
>     Objet: Re: [PATCH] d3dx9_36: Implement
>     D3DXCheckTextureRequirements + tests
>     À: wine-devel(a)winehq.org
>     Date: Lundi 19 avril 2010, 11h43
>
> On 19 April 2010 09:43, Christian Costa <titan.costa(a)wanadoo.fr
>     </mc/compose?to=titan.costa(a)wanadoo.fr>> wrote:
>     > +BOOL is_pow2(UINT num)
>     > +{
>     > +    UINT i;
>     > +
>     > +    if (!num) return FALSE;
>     > +    for (i = 0; num >> 1; i++) num >>= 1;
>     > +
>     > +    return (num << i) == num;
>     > +}
>     This should at least be static. You can also just replace it with
>     "return !(num & (num - 1));". You don't care about 0, since you
>     already check that earlier in D3DXCheckTextureRequirements().
>     make_pow2() can probably be simplified as well.
>
> ------------------------------------------------------------------------

[PATCH] d3dx9_36: Implement D3DXCheckTextureRequirements + tests

[Christian Costa]

Henri Verbeet a écrit :
> On 19 April 2010 09:43, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
> > +BOOL is_pow2(UINT num)
> > +{
> > +    UINT i;
> > +
> > +    if (!num) return FALSE;
> > +    for (i = 0; num >> 1; i++) num >>= 1;
> > +
> > +    return (num << i) == num;
> > +}
>
> This should at least be static. You can also just replace it with
> "return !(num & (num - 1));". You don't care about 0, since you
> already check that earlier in D3DXCheckTextureRequirements().
> make_pow2() can probably be simplified as well.

make_pow2 can be simplified a bit but there will still be some iterations.