Re: Patchwatcher: patchwatcher bug: Try number 2 drawprim.c drawStridedSlow

[Chris Ahrendt]

Henri Verbeet wrote:
> 2008/9/11 Chris Ahrendt :
> > Henri Verbeet wrote:
> > > 2008/9/11 Chris Ahrendt :
> > > > Ok what now.... I did a git to get the latest tree... I generated the
> > > > patch off the latest tree.. and submitted it again... what I am failing.
> > > > so I can fix it =).. the code compiles just fine and runs on my machine
> > > > so not sure...
> > > >
> > > > chris
> > >
> > > The patch is not against current git, it's against an earlier version
> > > of your patch.
> > >
> > > More importantly though, the patch is completely unacceptable. You
> > > really need to try to get a better understanding of how D3D vertex
> > > declarations work, and what the different D3DDECLTYPE values mean.
> >
> > Henri,
> >
> > Actually it was based on the current Git tree not a back level.... but will
> > try and do a merge to see if there is an issue...
>
> Current git:
>  449                     /* The coords to supply depend completely on
> the fvf / vertex shader */
>  450                     switch (coordsToUse) {
>  451                     case 4: q = ptrToCoords[3]; /* drop through */
>  452                     case 3: r = ptrToCoords[2]; /* drop through */
>  453                     case 2: t = ptrToCoords[1]; /* drop through */
>  454                     case 1: s = ptrToCoords[0];
>  455                     }
>
> Your patch:
> >                 switch (coordsToUse) {
> > -
> >                case 16:
> > -
> >                case 15:
> > -
> >                case 14:
> > -
> >                case 17:
> >               case 13:
> > -
> >                case 12:
>
> Where do you see those lines you're removing in current git?
> > I disagree on your comment... I am following the MSDN definition of what the
> > function does.  I do know what they mean I have been discussing these with
> > stefan.
>
> Well, proof me wrong then:
> Did you test on a native Windows system if the code should behave the
> way you think it should? Why are you creating a switch statement based
> on numeric constants instead of the appropriate enum values? Do you
> understand what, for example, D3DDECLTYPE_UBYTE4N means? What is the
> size of a D3DDECLTYPE_UBYTE4N element? What is the range of a
> component of a D3DDECLTYPE_UBYTE4N element? What is the size of a
> float? What is it's range? What are the implications of all that for
> using ptrToCoords, which is a float pointer, for D3DDECLTYPE_UBYTE4N
> in combination with glMultiTexCoord4fARB(), which expects four
> normalized floating point values? How is all this handled for the
> other attributes, like diffuse, specular, normal and position? How
> does drawStridedSlowVs() handle this? What about
> drawStridedInstanced()?

Because the original code was set up with the numbers originally..
If you look at any of the pre say 1.1.3 code you will see only the
switch code with 4 - 1 in it.. and nothing more...
to get rid of an error I was getting in an application which was sending 
  a 7 for coordstouse which is a valid value (its a 6 the code adds 1 to 
it for some odd reason so I followed the present convention as last time 
I cleaned something up and did it with #defines or moved code I was told 
to leave it as it is if I see it that way).
I know the various types... I put the comment in there and was telling
stefan what they were so I am aware of it... however it does not seem to 
make a difference if its float or short). The values are not normalized 
in the code but by the application and passed into the call.

That I do not know how they are handled in other places...   the code 
was really simple when I started working with it to clear what I 
submitted up...

Chris