Re: [1/2] d3dx9: Implemented non-equal declaration support in CloneMesh.

[Matteo Bruni]

Hi Michael,

+    for (i = 0; declaration1[i].Stream != 0xff; i++)
+    {
+        if (memcmp(&declaration1[i], &declaration2[i], sizeof(*declaration1)))

Doesn't that loop need to stop at the first end marker of either
declaration? I.e. may the first vertex declaration be longer than the
second one?

[1/2] d3dx9: Implemented non-equal declaration support in CloneMesh.

[Michael Mc Donnell]

On Mon, Oct 24, 2011 at 9:13 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
> Hi Michael,
>
> +    for (i = 0; declaration1[i].Stream != 0xff; i++)
> +    {
> +        if (memcmp(&declaration1[i], &declaration2[i], sizeof(*declaration1)))
>
> Doesn't that loop need to stop at the first end marker of either
> declaration? I.e. may the first vertex declaration be longer than the
> second one?

The first vertex declaration may be longer than the second one, e.g.
to make room for more vertex attributes. The loop actually still
breaks if the second declaration is shorter because it will have an
end marker and the first one doesn't.

I must admit it is difficult to read. I guess It would be easier to
read the code if it first finds the size of each declaration and does
a full memcmp if they're equal:

static BOOL declaration_equals(CONST D3DVERTEXELEMENT9 *declaration1,
CONST D3DVERTEXELEMENT9 *declaration2)
{
    UINT size1, size2;

    /* Find the size of each declaration */
    for (size1 = 0; declaration1[size1].Stream != 0xff; size1++);

    for (size2 = 0; declaration2[size2].Stream != 0xff; size2++);

    /* If not same size then they are definately not equal */
    if (size1 != size2)
        return FALSE;

    /* Check that all components are the same */
    if (memcmp(declaration1, declaration2, size1*sizeof(*declaration1)) == 0)
        return TRUE;

    return FALSE;
}

I'll update the patch and send it again.

[1/2] d3dx9: Implemented non-equal declaration support in CloneMesh.

[Matteo Bruni]

2011/10/24 Michael Mc Donnell <michael(a)mcdonnell.dk>:
> On Mon, Oct 24, 2011 at 9:13 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
> > Hi Michael,
> >
> > +    for (i = 0; declaration1[i].Stream != 0xff; i++)
> > +    {
> > +        if (memcmp(&declaration1[i], &declaration2[i], sizeof(*declaration1)))
> >
> > Doesn't that loop need to stop at the first end marker of either
> > declaration? I.e. may the first vertex declaration be longer than the
> > second one?
>
> The first vertex declaration may be longer than the second one, e.g.
> to make room for more vertex attributes. The loop actually still
> breaks if the second declaration is shorter because it will have an
> end marker and the first one doesn't.
>
> I must admit it is difficult to read. I guess It would be easier to
> read the code if it first finds the size of each declaration and does
> a full memcmp if they're equal:
>
> static BOOL declaration_equals(CONST D3DVERTEXELEMENT9 *declaration1,
> CONST D3DVERTEXELEMENT9 *declaration2)
> {
>    UINT size1, size2;
>
>    /* Find the size of each declaration */
>    for (size1 = 0; declaration1[size1].Stream != 0xff; size1++);
>
>    for (size2 = 0; declaration2[size2].Stream != 0xff; size2++);
>
>    /* If not same size then they are definately not equal */
>    if (size1 != size2)
>        return FALSE;
>
>    /* Check that all components are the same */
>    if (memcmp(declaration1, declaration2, size1*sizeof(*declaration1)) == 0)
>        return TRUE;
>
>    return FALSE;
> }
>
> I'll update the patch and send it again.

You're right, sorry for the noise. I see that you have already resent
the patches, that's fine anyway.