Re: [Warzone-dev] Fixed using wrong rev as base when changing piedraw.c
Am Sonntag, 28. Januar 2007 schrieb Dennis Schridde: Am Sonntag, 28. Januar 2007 schrieb The Watermelon: Ugly typo: Some notes: - Please DONT create patches which just comment out something. Remove it or don't remove it. We can allways revert using SVN and in the patch we see what you changed, too, so I see really no point to add huge amounts of commented out code. pgpMQQaYzhc6b.pgp Description: PGP signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] Fixed using wrong rev as base when changing piedraw.c
Am Montag, 29. Januar 2007 schrieb The Watermelon: On 1/29/07, Dennis Schridde [EMAIL PROTECTED] wrote: Am Sonntag, 28. Januar 2007 schrieb Dennis Schridde: Am Sonntag, 28. Januar 2007 schrieb The Watermelon: Ugly typo: Some notes: - Please DONT create patches which just comment out something. Remove it or don't remove it. We can allways revert using SVN and in the patch we see what you changed, too, so I see really no point to add huge amounts of commented out code. Removed commented-out lines and move a glMatrixPop and a glMatrixPush out of 'for loop' in pie_DrawRemainingTransShapes in piedraw.c(glLoadIdentity does top stack replacement,no need to do glMatrixPop and glMatrixPush inside 'for loop' imo). display3d.c @@ -1607,9 +1607,9 @@ Can't p[0].x = tileScreenInfo[i+0][j+0].sx; p[0].y = tileScreenInfo[i+0][j+0].sy; p[0].z = tileScreenInfo[i+0][j+0].sz; be replaced with p[0] = tileScreenInfo[i+0][j+0]; or is that not possible because of different data types? Similar things are done in several other locations, too. Maybe some more types can get their x,y,z and u,v coordinates replaced by Vector3 and Vector2 ? Another suggestion would also be to replace pos with position. But I don't exactly know if that's a correct naming anyway. Is the x,y,z in a vertex a position? This is nothing really important. The name position doesn't seem to fit for me, but it may very well be that I am totally wrong. Can iVertex and PIEVERTEX be merged? It seems to me that they are mostly the same. (What does the g field in iVertex mean?) There is also a PIEVECTORF, can't that be replaced by Vector3, too? And iVectorf? Replace it with Vector3d? Ohwowow... I just saw that you replaced iVectorf with Vector3f... That doesn't look good, since iVectorf used doubles, while Vector3f is about floats... Typedefinitions should go with the SDL types, if they are providing an API, IMO. Makes it more obvious what they are representing and in the long run I wanted to replace all Win types with SDL types anyway. iVertex is missing a space in the list. Looks better if it gets it. ;) (Yes, this is a bit nitpicking, but I would forget to change it myself before applying or after someone else applied it.) The name MakeVector3fInt should be improved IMO... Vector3_Float2Int maybe? Is there a commonly used name for such conversions? --Dennis pgpb5dXdLxWV8.pgp Description: PGP signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] Fixed using wrong rev as base when changing piedraw.c
Am Montag, 29. Januar 2007 schrieb The Watermelon: On 1/29/07, Dennis Schridde [EMAIL PROTECTED] wrote: display3d.c @@ -1607,9 +1607,9 @@ Can't p[0].x = tileScreenInfo[i+0][j+0].sx; p[0].y = tileScreenInfo[i+0][j+0].sy; p[0].z = tileScreenInfo[i+0][j+0].sz; be replaced with p[0] = tileScreenInfo[i+0][j+0]; or is that not possible because of different data types? tileScreenInfo is a SVMESH while p is a Vector3f... Yes, but it contains something that can be seen as a Vector3... Similar things are done in several other locations, too. Maybe some more types can get their x,y,z and u,v coordinates replaced by Vector3 and Vector2 ? Another suggestion would also be to replace pos with position. But I don't exactly know if that's a correct naming anyway. Is the x,y,z in a vertex a position? This is nothing really important. The name position doesn't seem to fit for me, but it may very well be that I am totally wrong. Can iVertex and PIEVERTEX be merged? It seems to me that they are mostly the same. (What does the g field in iVertex mean?) I dont know what the 'g' field is in iVertex either...havent looked at functions that use it...seems to be some color byte... There is also a PIEVECTORF, can't that be replaced by Vector3, too? yes i think it can be safely replaced by Vector3f too... And iVectorf? Replace it with Vector3d? Ohwowow... I just saw that you replaced iVectorf with Vector3f... That doesn't look good, since iVectorf used doubles, while Vector3f is about floats... iVectorf is useless because it will eventually get abused to integer in imdload.c,which is the only instance of iVectorf iirc. I am currently looking into the possibility to change all those x,y,z in PIEVERTEX and other pie structs to float from sint32 to increase the precision of pie point coords like someone suggested in forum,and change imdload function to use '%5.5f'(just a random number) to sscanf imd points in pie files,though all exist pie files will need to be changed to cope with such changes i think... I will change it to Vector3d for better readability anyways. Well, if that Vector3x stores floats, then it should be named Vector3f. My Vector3d was not because it is a vector in 3 dimensional space, but because it is a vector of 3 doubles. So if this vector is never used as a double, but only to store ints, then it should be removed and replaced by Vector3i and not Vector3f... But if it gets to store floats, then it should sure be named Vector3f, as you did it. My name was only for a third Vector3, which should store doubles, because the old iVectorf stored doubles, too. And I don't think it is safe to replace something that assumes to be double with a float. Typedefinitions should go with the SDL types, if they are providing an API, IMO. Makes it more obvious what they are representing and in the long run I wanted to replace all Win types with SDL types anyway. iVertex is missing a space in the list. Looks better if it gets it. ;) (Yes, this is a bit nitpicking, but I would forget to change it myself before applying or after someone else applied it.) The name MakeVector3fInt should be improved IMO... Vector3_Float2Int maybe? Is there a commonly used name for such conversions? --Dennis not sure about the naming stuff Me neither. ;) I just think that MakeVector3fInt is not a very good name. ;) pgpQRzH3hX8Lh.pgp Description: PGP signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] Fixed using wrong rev as base when changing piedraw.c
On 1/28/07, Dennis Schridde [EMAIL PROTECTED] wrote:
Am Sonntag, 28. Januar 2007 schrieb The Watermelon:
Idea is good.
Some notes:
- Please create patches which just comment out something. Remove it or
don't
remove it. We can allways revert using SVN and in the patch we see what
you
changed, too, so I see really no point to add huge amounts of commented
out
code.
- Why did you remove the GPL header?
- I am not sure whether changing those x,y,z parts to Vector3 is good.
You'd
have to change u,v to Vector2i, too. And if you don't change them, but
leave
them as is, you have the chance to directly pass them to GL, without
having
to fear that one or the other might include some additional fields which
would mess up the memory structure or alignment. Is there any benefit in
this
change? I mean, can we pass the pos part directly to GL in some place, is
the
code easier to read with this? Otherwise I vote for PIEVERTEX staying
independend from Vector3...
I failed to fix patch to piedraw.c in this one,I reverted back to a wrong
rev of piedraw.c when making those changes,and it removed the GPL header and
added some removed functions...
Vector3 is always easier to read than some non-standardized names like
'x,y,z' 'sx,sy,sz' and some weird 'd3dx,d3dy,d3dz',it also saves you a bunch
of Vector_Set functions in draw functions.Passing vector2 or vector3 struct
directly to glVertex2fv or glVertex3fv as glFloat[2]/glFloat[3] pointer is
generally faster than passing and copying individual values for x,y,z to
glVertex2f/glVertex3f.
Example:(block of function pie_Polygon)
sx,sy,sz version:
* fVector p1, p2, p3, v1, v2, n;
float l;*
* fVector_Set(p1, pVrts[0].sx, pVrts[0].sy, pVrts[0].sz);
fVector_Set(p2, pVrts[1].sx, pVrts[1].sy, pVrts[1].sz);
fVector_Set(p3, pVrts[2].sx, pVrts[2].sy, pVrts[2].sz);
fVector_Sub(v1, p3, p1);
fVector_Sub(v2, p2, p1);
fVector_CP(n, v1, v2);
l = 1.0;*
* glNormal3f(n.x*l, n.y*l, n.z*l);
}
}
for (i = 0; i numVerts; i++) {
glColor4ub(pVrts[i].light.byte.r, pVrts[i].light.byte.g,
pVrts[i].light.byte.b, pVrts[i].light.byte.a);
glTexCoord2f(pVrts[i].tu, pVrts[i].tv+texture_offset);
glVertex3f(pVrts[i].sx, pVrts[i].sy, pVrts[i].sz);
}*
vector3 version:
* Vector3f v1, v2, n;
float l;**
Vector3f_Sub(v1, pVrt[2].pos, pVrt[0].pos);
Vector3f_Sub(v2, pVrt[1].pos, pVrt[0].pos);
Vector3f_CP(n, v1, v2);
l = 1.0;*
* glNormal3f(n.x*l, n.y*l, n.z*l); }
}
for (i = 0; i numVerts; i++) {
glColor4ub(pVrts[i].light.byte.r, pVrts[i].light.byte.g,
pVrts[i].light.byte.b, pVrts[i].light.byte.a);
glTexCoord2f(pVrts[i].tu, pVrts[i].tv+texture_offset);
glVertex3fv(pVrts[i].pos);
}*
Texture coords u,v is not very suitable for Vector2i,cos it always appends
texture_offset to the 'v' when passing to gl functions.
I will fix these problems and resend the patch...
___
Warzone-dev mailing list
Warzone-dev@gna.org
https://mail.gna.org/listinfo/warzone-dev
