Jump to content

CFixedVector2D/3D proposed patch working, awaiting profiling...


Recommended Posts

Hi, I want to introduce this patch properly, first of all, and it's hard to do in IRC.

I think it was Stan who mentioned to me in another forum thread, that one of the things needing fixing in the engine, performance-wise, is the fixed point (64-bit) square root, which is found int Sqrt.cpp, named isqrt64().  This is a hand-coded routine, which I saw no immediate way of improving.  But what I thought of is that I could somehow reduce calls to it by cacheing results.  99% if not 100% of the use of this routine is courtesy of FixedVector2/3D objects' Length() fn, to obtain integer sqare root of ( x*x + y*y [+z*z] ).

Having done my share of programming and debugging in the past, I'm familiar with the fact that gazillions of calls to functions in most programs are kind of redundant, with the same variable read gazillions of times, or variables being "updated" a million times to the same value they were holding already.  There's an entire movement out there of people who believe in general cacheing of all results from all functions;  they want languages to begin adopting their philosophy...  I'm not subscribed to it, but it merits a thought or two.

In the case of FixedVectors, I wildly speculate that some of them have their length() function called many times, while other vectors might never hear a single call.  (Case in point, point2d is implemented using FixedVector2D under the hood;  but points don't have much of a length, do they?). 

Speaking of points and vectors, my next patch I'm thinking of is having non-class binary functions for point and vector math such that,

  • A point minus a point = a vector
  • The length of a vector is a length
  • The length of a point is a compiler error
  • A point plus or minus a vector = a point
  • A vector plus a point = a point
  • A vector minus a point is a compiler error
  • A point plus a point results in a compiler error (or in a point-group ;-)
  • Negative a vector is a (flipped) vector
  • Negative a point is a compiler error

It should prevent coding errors in the future;  --might it dig up some from the past?  I implemented all of this for my own engine 20 years ago.

But getting back to this patch, my educated guess is that many vectors' lengths will never be looked at, while some may be looked at ad-nauseum.  So I decided to experimentally try to

  • (only when needed) compute the length of a vector, if not already cached;  otherwise return the cached value
  • when computing the length, cache the result, in case it is needed again before the vector is modified
  • whenever the vector is modified, invalidate the cached length (but don't compute it again unless or until it is needed again)

Thus, I added a member to the class, L, also of type "fixed".  It holds the length of the vector when it does;  otherwise it reads zero, meaning that the length is not current.  Now, you might object to this saying "using a specific value to mean something is bad programming!", or "Isn't a zero length a valid length?!".  Yes, all of that is true, however, vectors of zero length are hopefully not numerous, and if we were to assume the worst, namely ALL our vectors are zero length, the penalty, in this code, is to have as many calls to isqrt64() as we do at present;  certainly not capital punishment;  and actually I test X Y and Z for being zero before calling the length computation.  That is, if I need the length, and see that the cached length is zero, I do look at X, Y and Z, and if all are zero I do nothing.  Secondly, having an extra bool to indicate currency of the length variable is a bit performance-wasteful in the case of a flyweight class like this.  In a big cyberpunk boilerplate class, I'd consider it, of course.

Note that this is effectively a cacheing, or "proxy", and as such it suffers from a minor design gottcha:  The function Length() looks like a pure function, and should be const as far as the outside world is concerned;  but, under the hood, the update of the cached length often happens inside this const function.  The right and only way to solve this issue, as is true of proxies in general, is to make L mutable.  This is the true reason "mutable" exists;  so let's use it.

 

The new code, compiles, links, passes the 351 tests, and plays nice and smooth like a charm, --at least at the Very Easy setting :P

I used to have momentary freezes of half a second every 5 minutes or so;  now I don't notice them anymore;  but I don't know if that's due to other changes happening on svn.

To find out if peformance is improved, I need to profile.  I followed the instructions in the Wiki for Profiler2 and tried it.  It looks pretty.  Now I will need to know how to count calls to isqrt64() and show them on the screen, or how to compare performance overall using a prerecorded game.  However, either way, the testing needs to play back a game by user input commands;  NOT by results;  as it needs to test pathfinding and AI, which are probably stripped back when a game is being re-played like a movie.  I will need help with all this.

For now, if anyone's interested in trying it, looking at it, and/or profiling it, here's my latest svn diff:

https://pastebin.com/nZt94JjJ

(File also attached below.)

Note that it seems nothing I've done is conflicting with any changes on svn;  it looks like there's nothing to resolve yet.

 

In the course of making these changes, I fixed a couple of other things about these classes:

  1. Dot() and CrossProduct() functions in FixedVector3D were not const
  2. X, Y (and Z) members were public, in both classes...  Privatized them, then dded getX(), getY()... const...;  and non-const Xref(), Yref(), etc. for external manipulators.

This latter change caused changes in many other files, where the members were being read or written directly.  That's why the diff is so big.

Note that some people believe that making members of a class public should make the compiled code run faster than having access functions.  That's the exact opposite of the truth:  The elephant in the room nobody talks about is that C++ code is generally far faster than any hand-coded C equivalent, and there's good reasons for this (which Soustrup eloquently explained in a recent interview):  The code optimizers of today have grown so smart that most assembler coders have a hard time competing with them;  but optimizations thrive in an atmosphere friendly to assumptions; and some of the most useful assumptions in compiler optimization are about what can and cannot change where and when.  C++ tries to limit access to the minimum necessary, limit scopes to the minimum necessary, hide whatever can be hidden...  In such an environment, crazy optimizations become possible.  But if you subversively defeat the best gifts C++ has to offer, such as by exposing class members, you kill many optimization possibilities at the same time.  These classes should already help optimized code run faster on the basis of having closed off public access to its members alone;  never mind the isqrt64() issue.  Plus, I fixed alignment issues with padding.  Compilers don't touch alignment with a ten-foot pole;  this is coder-responsibility.  (At least it was so 20 and 15 years ago;  I may be behind the curve;  if so, my apologies in advance.)

 

EDIT:  Speaking of elephants,  0ad elephants have gone on strike.  I can't get them to do any building or repair work for me, anymore.  In fact, even the hammer icon is gone.  I don't believe it has anything to do with my changes...  Ether they joined a union, or were accidentally given wooly mammoth AI, or both...

 

diff.txt

Edited by DanW58
  • Like 4
  • Thanks 2
Link to post
Share on other sites

Just a quick glance, so might have missed some of the quirks involved.

  • Why not use alignas instead of padding?
  • Are there enough vector3D "alive" that an additional bool actually matters?
  • L (length) seems to be singed, so why using zero for maybe invalid instead of <0 for invalid?
  • You are over commenting quite a bit me thinks.
Link to post
Share on other sites
1 hour ago, hyperion said:

Just a quick glance, so might have missed some of the quirks involved.

  • Why not use alignas instead of padding?
  • Are there enough vector3D "alive" that an additional bool actually matters?
  • L (length) seems to be singed, so why using zero for maybe invalid instead of <0 for invalid?
  • You are over commenting quite a bit me thinks.

He updated the patch here https://code.wildfiregames.com/D3454

1) Is fixed

4) too

 

Link to post
Share on other sites

I came around looking for where to update the patch I submitted a few days ago, but could not find it for the life of me.

I'm still working on this, though just for myself, (I may end up forking this), and I've finished a major milestone;  namely, all the Vector2D, Vector3D, FixedVector2D and FixedVector3D classes are now fairly cleaned up.  Well, there are still a few ugly things...

The cacheing of sqrt in FixedVector3D remains, and works beautifully.  The cacheing of sqrt in FixedVector2D is still there, but I'm not sure how much use it gets;  I might remove it.  In Vector3D I tried to do a similar cacheing, but the class is used structurally in many other classes, which changes their size, and in some cases it breaks their interfacing to other libraries that expect a fixed format.

The use of alignas() has proven extremely detrimental, precisely in that it breaks structures that include the aligned structures.  Everything I tried to do with alignas() I had to eventually undo.  It is a directive from Hell.

I also at least partially cleaned up the color classes.  CColor I renamed it to RGBAcolor.  I found cases where Vector4D was being used to carry color information, and I jumped to change interfaces to use RGBAcolor instead;  but the use of Vector4D was going too deep, into shaders and uniforms and whatnot, and I got cold feet and reversed course. But so what I ended up doing instead is adding a conversion operator in RGBAcolor to Vector4D, which simplified a lot of the code elsewhere.  Also, many places in the code had specially coded conversions from float to char color formats;  I put getR8(), getG8(), getB8() and getA8() functions in both RGBcolor and RGBAcolor classes and managed to simplify a lot of code elsewhere.

Getting back to vectors, not only am I cacheing the Length() results, but I also sped up the isqrt64() routine by using a double sqrt(), single instruction.  It was tricky to get it to past the tests, but it works.  It was just a matter of trimming overflows.

The end result is that the gameplay feels smoother, but of course this is a subjective thing, and I have no idea how to use Profile2.

I've no illusion anyone here is going to either look at my work or even try it for a minute, but just in case, I attach the diff, which is current (the last svn up I did was a 2 or 3 hours ago).  What I said in my first post about making points and vectors separate classes, I've done the work of defining CFIxedPoint2D, CFIxedPoint3D, CPoint2D and CPoint3D.  They are in the same files as where the vectors are declared, just below them.  But I have not used them yet...  That's what I'm about to do next.

So, the diff is attached.  Cheers!

diff.txt

Edited by DanW58
  • Like 3
Link to post
Share on other sites

To test if these changes actually perform better one can look at the profiler that is included ingame (press F9 irrc) and compare with/without changes. Given the changes are spread and measuring each one individually won't make much sense you can look at the parts of the profiler that includes changes form the diff and see if at least some differences (you can run pyrogenesis from command line to load a map with the same settings to better compare and all ... ).

  • Like 1
Link to post
Share on other sites
4 hours ago, DanW58 said:

I came around looking for where to update the patch I submitted a few days ago, but could not find it for the life of me.

It's the link I posted just above your post? D3454

4 hours ago, DanW58 said:

The end result is that the gameplay feels smoother, but of course this is a subjective thing, and I have no idea how to use Profile2.

I could explain it to you again, if you want I tried on IRC but I realised it might have been too much information.

Link to post
Share on other sites
17 hours ago, DanW58 said:

I've no illusion anyone here is going to either look at my work or even try it for a minute, but just in case, I attach the diff, which is current (the last svn up I did was a 2 or 3 hours ago).  What I said in my first post about making points and vectors separate classes, I've done the work of defining CFIxedPoint2D, CFIxedPoint3D, CPoint2D and CPoint3D.  They are in the same files as where the vectors are declared, just below them.  But I have not used them yet...  That's what I'm about to do next.

I've tried it but didn't get it to work. I will wait for your next phabricator patch :)

17 hours ago, DanW58 said:

The cacheing of sqrt in FixedVector3D remains, and works beautifully.  The cacheing of sqrt in FixedVector2D is still there, but I'm not sure how much use it gets;  I might remove it.  In Vector3D I tried to do a similar cacheing, but the class is used structurally in many other classes, which changes their size, and in some cases it breaks their interfacing to other libraries that expect a fixed format.

The use of alignas() has proven extremely detrimental, precisely in that it breaks structures that include the aligned structures.  Everything I tried to do with alignas() I had to eventually undo.  It is a directive from Hell.

I worked on Vector3D (https://code.wildfiregames.com/D2789) sometimes ago. The idea was to replace X, Y, Z by one __m128 SSE type and using SSE in the vector functions. Therefore @Imarok helped me replacing all direct accesses on the attributes by getters. At the end we gave up, because as you see, it breaks to many things. @Imarok can tell you more about it.

Link to post
Share on other sites

@OptimusShepardI applied the patch to my working copy SVN 24796. After running ./update-workspaces.sh (macOS), I only had to remove one curly bracket from each file before compiling the game:

/Users/paladin/0ad/source/maths/Vector3D.cpp
/Users/paladin/0ad/source/graphics/Color.cpp

I have compiled the game. Run a visual replay file with the normal SVN 24796 and a visual replay file with the patch applied. I saved two jsonp files.

  • 24796DAN patchprofile2.jsonp (SVN 24796 version with the above patch applied)
  • 24796profile2_SVNVersion.jsonp (normal SVN 24796 version no patch applied)
  • @Stan`recommended via IRC (27th Jan) to use your replay from Sahyadri Buttes, but I was not able to find it so I played one of my own: Acropolis Bay (2) commands.txt

PS: I hope I did everything right, otherwise please correct me, maybe next time it will be ok.

967105054_24796DANpatchprofile2.jsonp 24796profile2_SVNVersion.jsonp commands.txt

Edited by Langbart
Link to post
Share on other sites

It seems the two data are not in sync. Using source/tools/profiler2/profiler2.html and loading the two replays I get this You need to make it as identical as possible.

image.png

 

  • Did you set the camera to follow the same player?
  • Did you do it at x1? According to @OptimusShepard if you change speed you mess things up.

Here is @OptimusShepard's replay

optimus-sayahdri-replay.zip

 

Link to post
Share on other sites

@Stan` and @wraitii helped me making it right, this time non visual. I first compiled the svn version 24796 and run:

binaries/system/pyrogenesis -mod=public -replay=[path to Optimus Sahyadri Buttes command.txt file]

This automatically created the profile2.jsonp in my working directory. (renamed it to SVN24796profile2.jsonp)
I repeated the process this time with the applied patch from Dan to the SVN version 24796. (renamed it to SVNpatchDAN24796profile2.jsonp)

SVN24796profile2.jsonp SVNpatchDAN24796profile2.jsonp

Link to post
Share on other sites

Wow!  I came here to lament and despair, but found a lot of curious replies.

Optimus, I was starting to figure out the impact of SSE.  Wish I would have joined earlier;  I could have written all the SSE you needed and then some directly in Assembler, and bypass all the subversive interfaces.  My experience was with with 3DNow!, rather, but same thing;  --Intel just copied AMD, as always...

R.e. the profiling, thanks!, but I don't understand anything;  what's the conclusion?  Did it speed up the engine or not?  I can't tell from your words or your files or your graphs.  I see that "comparison" graph, but it has two graphs, one for frequency, but doesn't look like a typical speaker response ;-),   and the other is frame by frame graph...  of what I have no idea.  And there are red and green curves, but one says 0/frame - n = 1629 and the other 1/frame - n=5157, and I haven't the foggiest idea what that means.  I need a Rosetta Stone...

Anyways, I came to share my pain and yell for help understanding parts of the code.  Just so you understand what I'm doing, first of all, these are my FOR's (Frames of Reference) declarations:

First I declare a common ancestor for FOR's, not sure for what reason exactly, I'm not sure how to specify that a template param should be derived from class X...

Then I declare my template functor for Transform, which will declare needed transforms, leaving only the work of implementing them.

And I follow that by a whole list of FOR structs, full of useful typedefs.  They follow a model space to screen pipeline approximately from top to bottom.

Below that I have sugar typedefs, and vector and point declarations...

//////////////////////////////////////////////////////////////
////     "Frames Of Reference" common abstract type:      ////
//////////////////////////////////////////////////////////////
struct FOR // --Frames Of Reference' common abstract type
{
protected:
	virtual ~FOR() = 0;
};
//////////////////////////////////////////////////////////////
template< typename FOR_in_repr_t, typename FOR_out_repr_t >
struct Transform
{
	void operator()(FOR_in_repr_t const &, FOR_out_repr_t &) const;
};
//////////////////////////////////////////////////////////////
/// Abstract Frames of Reference to be used as template params
//////////////////////////////////////////////////////////////
// The first FOR is a null stub for the second to reference.
struct ____NO____FOR : public FOR  //The "NO" frame of reference
{
	typedef            ____NO____FOR                         FOR_t;  //
	typedef            ____NO____FOR                    otherFOR_t;
	typedef            ____NO____FOR                   represent_t;  // --none--
	typedef            ____NO____FOR                  other_repr_t;  // --none--
	typedef            ____NO____FOR                    fwdXform_t;  //NO transform
	typedef            ____NO____FOR                    bakXform_t;  //NO transform
private:  virtual  ~____NO____FOR() = 0;      //can't be instanced or inherited
};
struct ModelSpaceFOR : public FOR  //No need to id models; only one used at a time
{
	typedef  ModelSpaceFOR                                   FOR_t;  //
	typedef  ____NO____FOR                             other_FOR_t;  // preceded by nothing
	typedef  CVector3D<FOR_t>                          represent_t;  //float vec 3
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //NO transform
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //NO transform
private:  virtual  ~ModelSpaceFOR() = 0;
};
struct WorldSpaceFOR : public FOR  //This is the virtual world's 3D space
{
	typedef  WorldSpaceFOR                                   FOR_t;  //
	typedef  ModelSpaceFOR                             other_FOR_t;  // preceded by Models' space
	typedef  CFixedVector3D<FOR_t>                     represent_t;  //fixed-point vec 3
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //unit movements
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //impact queries?
private:  virtual  ~WorldSpaceFOR() = 0;
};
struct WorldSpeedFOR : public FOR  //This is virtual world velocity vectors for units
{
	typedef  WorldSpeedFOR                                   FOR_t;  //
	typedef  ModelSpaceFOR                             other_FOR_t;  // preceded by Models' space
	typedef  CVector3D<FOR_t>                          represent_t;  //fixed-point vec 3
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //obstacles affecting speed
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //motion (speed accumulation)
private:  virtual  ~WorldSpeedFOR() = 0;
};
struct WorldRotatFOR : public FOR  //This is virtual world spin vectors for units ///PROBABLY TO BE AXED
{
	typedef  WorldRotatFOR                                   FOR_t;  //
	typedef  ModelSpaceFOR                             other_FOR_t;  // preceded by Models' space
	typedef  CVector3D<FOR_t>                          represent_t;  //fixed-point vec 3
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //NO transform
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //NO transform
private:  virtual  ~WorldRotatFOR() = 0;
};
struct Entty2SpaceFOR : public FOR  //Same as world, but with no height info
{
	typedef  Entty2SpaceFOR                                  FOR_t;  //
	typedef  WorldSpaceFOR                             other_FOR_t;  // preceded by World space
	typedef  CFixedVector2D<FOR_t>                     represent_t;  //fixed-point vec 3
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  // X,Y <= X,..,Z
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  // X,Y,Z <= X,H,Y
private:  virtual  ~Entty2SpaceFOR() = 0;
};
#if 0
struct EnttyOrientFOR : public FOR  //Fwd and Up vectors;   ... to become unit vectors
{
	typedef  EnttyOrientFOR                                  FOR_t;  //
	typedef  WorldSpaceFOR                             other_FOR_t;  // preceded by Models' space
	typedef  CUnitVector2D                             represent_t;  //fixed-point vec 3
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //not sure how this works
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //not sure how this works
private:  virtual  ~EnttyOrientFOR() = 0;
};
#else
struct EnttyOrientFOR : public FOR  //Fwd and Up vectors;   FixedVector2D for now...
{
	typedef  EnttyOrientFOR                                  FOR_t;  //
	typedef  WorldSpaceFOR                             other_FOR_t;  // preceded by Models' space
	typedef  CFixedVector2D<FOR_t>                     represent_t;  //fixed-point vec 3
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //not sure how this works
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //not sure how this works
private:  virtual  ~EnttyOrientFOR() = 0;
};
#endif
struct ViewFrustrFOR : public FOR  //This is view frustrum space, CPU-GPU standard Xform
{
	typedef  ViewFrustrFOR                                   FOR_t;  //
	typedef  WorldSpaceFOR                             other_FOR_t;  // preceded by world space
	typedef  CVector3D<FOR_t>                          represent_t;  //float vec 3
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //standard perspective matrix
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //inverse matrix for mouse ray queries
private:  virtual ~ViewFrustrFOR() = 0;
};
struct MiniMapCoorFOR : public FOR  //Mini-Map coordinate representation
{
	typedef  MiniMapCoorFOR                                  FOR_t;  //
	typedef  WorldSpaceFOR                             other_FOR_t;  // preceded by World
	typedef  CFixedVector2D<FOR_t>                     represent_t;  //fixed-point vec 2
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //simple scaling from World X,Z
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //simple scaling to World X,Z
private:  virtual ~MiniMapCoorFOR() = 0;
};
struct ScreenPerspFOR : public FOR  //Not sure this is needed at all; probably Frustrum covers it
{
	typedef  ScreenPerspFOR                                  FOR_t;  //
	typedef  ViewFrustrFOR                             other_FOR_t;
	typedef  CFixedVector2D<FOR_t>                     represent_t;  //fixed-point vec 2
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //NO transform
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //NO transform
private: virtual ~ScreenPerspFOR() = 0;
};
struct ScrnOverlayFOR : public FOR  //screen overlays, & maybe post-proc effects
{
	typedef  ScrnOverlayFOR                                  FOR_t;  //
	typedef  MiniMapCoorFOR                            other_FOR_t;
	typedef  CFixedVector2D<FOR_t> (or u16?)           represent_t;  //fixed-point vec 2
	typedef  other_FOR_t::represent_t                 other_repr_t;  //representation
	typedef  Transform<other_repr_t,represent_t>        fwdXform_t;  //NO transform
	typedef  Transform<represent_t,other_repr_t>        bakXform_t;  //NO transform
private: virtual ~ScrnOverlayFOR() = 0;
};
///sweet nicknames:
typedef   ____NO____FOR   UNKN;  //temp use to pacify compiler when FOR is hard to figure out
typedef   ModelSpaceFOR  MODEL;
typedef   WorldSpaceFOR  WORLD;
typedef   WorldSpeedFOR   VELO;
typedef   WorldRotatFOR   SPIN;  //Probably not an FOR but a helper type like CEulerRadians
typedef   Entty2SpaceFOR  ENTP;  //for position, same as world (x,z)
typedef   EnttyOrientFOR  ENTO;  //for orientation, used in Fixed fwd & up unit vectors
typedef   EnttyOrientFOR  UNIT;  //for orientation, used in float fwd & up unit vectors
typedef   ViewFrustrFOR   VIEW;
typedef   MiniMapCoorFOR  MINI;
typedef   ScreenPerspFOR  SCRN;
typedef   ScrnOverlayFOR  OVRL;

 //convenient forward declarations:
template< typename FOR > class CFixedVector2D;
template< typename FOR > class CFixedPoint2D;
template< typename FOR > class CVector2D;
template< typename FOR > class CPoint2D;
template< typename FOR > class CFixedVector3D;
template< typename FOR > class CFixedPoint3D;
template< typename FOR > class CVector3D;
template< typename FOR > class CPoint3D;
class CUnitVector2D;  //This vector maintains unit-length automagically
class CUnitVector3D;  //This vector maintains unit-length automagically
class CEulerDegrees;  //float x,y,z
class CEulerRadians;  //float x,y,z

  (NOTE: All of the above is at the bottom of MathUtils.h, for now, as I don't know how to add new files to the build system.)

Note that none of this "code bloat" is going to have negative consequences whether in code size or performance.  They share a lot of code, and the compiler will merge them as much as it can;  the FOR distinctions are compile-time only.

Here's an example of two template transform implementations:

(For now these are at the bottom of Vector3D.cpp.)

//place these in the cpp files where they are most heavily called:
template Transform::operator() const (CFixedPoint3D<WORLD> const & in, CFixedPoint2D<ENTP> & out)
{
	out.X = in.X;  out.Y = in.Z;
}
template Transform::operator() const (CFixedPoint2D<ENTP> const & in, CFixedPoint3D<WORLD> & out)
{
	out.Y = mapheight_at(in.X, in.Y); //the name of the function is unsure, but it surely exists
	out.X = in.X;  out.Z = in.Y;
}

So, the system basically declares all the needed transforms, and even specifies the input and output types;  all I have to do then is copy-paste the code into the implementation, and then I can just push objects from one FOR to the other, and they go through the appropriate transformers.  Anyone starting to like this?

But then I get stuck on the stupidest problem...

I have the following conversions:

	CCEulerDegrees::CCEulerDegrees( CEulerRadians const & r ) :
	  X(RADTODEG(r.getX())), Y(RADTODEG(r.getY())), Z(RADTODEG(r.getZ())) {}
	CEulerRadians::CEulerRadians( CCEulerDegrees const & d ) :
	  X(DEGTORAD(d.getX())), Y(DEGTORAD(d.getY())), Z(DEGTORAD(d.getZ())) {}

Nothing too rocket-science here.  These are three float classes that all they do is hold Euler angles.

The problem is I don't know where in the code to use which.  There was one place, in one file, where I knew what was what because there was code like the above to translate degrees to radians.  But elsewhere in the code there's no such luck.  There's often not even a comment to say if it's degrees or radians.  Furthermore, in some places in the code, the representation is Vector3D (floats), while in other places it is FixedVector3D, but both are called "rotation".

In file NUSpline.h there's a struct declaration with a rotation.  If I make it a FixedVec, like,

struct SplineData  ///// this struct needs HEAVY refactoring...
{
	// Should be fixed, because used in the simulation
	CFixedPoint3D<WORLD> Position;
	CVector3D<VELO> Velocity;
	// TODO: make rotation as other spline
	CFixedVector3D<UNKN> Rotation; //could be radians, degrees... why FixedVector?  <<<<<<<<<<<<<<<<
	// Time interval to the previous node, should be 0 for the first node
	fixed Distance;
};

I get an error in file CCmpCinemaManager.cpp, "cannot convert ‘CFixedVector3D<____NO____FOR>’ to ‘const CEulerDegrees&’"...

[331]   pathSpline.AddNode(node.Position, node.Rotation, node.Distance);

And if I make it a float solution, namely,

struct SplineData  ///// this struct needs HEAVY refactoring...
{
	// Should be fixed, because used in the simulation
	CFixedPoint3D<WORLD> Position;
	CVector3D<VELO> Velocity;
	// TODO: make rotation as other spline
	CEulerDegrees Rotation; //could be radians, degrees... why FixedVector? <<<<<<<<<<<<<<<<<<<<<<<
	// Time interval to the previous node, should be 0 for the first node
	fixed Distance;
};

Then I get an error also in file CCmpCinemaManager.cpp, "cannot convert ‘float’ to ‘fixed’", but a different line:

[282]    serializer.NumberFixed_Unbounded("RotationX", nodes.Rotation.getX());

What makes navigating this part of the code particularly difficult is a lot of the functions are pure virtual;  clicking on go to implementation does nothing.

Too many hours resolving conflicts is having a toll on me...

Link to post
Share on other sites

Hey DanW58,

Nice to see you haven't given up! The errors you get seem to me like you are not sure with there is Fixed and Floats. As you may know the game must run the same on all machines, operating systems we support because all clients compute everything (google lockstep model) Since floating point calculation are not really precise and and can change from one machine to the next we cannot rely on them to be them hence why everything that needs to be synchronised uses CFixed instead of a normal float.

I suppose that when you used your macros (which might be better as templates) you didn't make that distinction?

Stuff that's serialized usually needs to be a CFixed because one wants the state to be exactly the same when loading a game for instance.

The state is based on what's serialized so we can compare hashes with other players.

42 minutes ago, DanW58 said:

 

R.e. the profiling, thanks!, but I don't understand anything;  what's the conclusion?  Did it speed up the engine or not?  I can't tell from your words or your files or your graphs.  I see that "comparison" graph, but it has two graphs, one for frequency, but doesn't look like a typical speaker response ;-),   and the other is frame by frame graph...  of what I have no idea.  And there are red and green curves, but one says 0/frame - n = 1629 and the other 1/frame - n=5157, and I haven't the foggiest idea what that means.  I need a Rosetta Stone...

Not really actually. To understand those graphs better you need to play with the Profiler2 UI in source/tools/profiler2/ and load those two jsonP files.

The colors depend on the order files are loaded here I believe green is your patch and red is vanilla. The y axis is the time it took to compute a single frame.

 

Link to post
Share on other sites
39 minutes ago, Stan` said:

The errors you get seem to me like you are not sure with there is Fixed and Floats.

I think I know exactly why there's Fixed.  As I mentioned before in some forum thread, I was once working on my own engine, and I had a moment of enlightenment, and saw that float's are not good as accumulators (e.g. counting votes), and not good for complex terrains, due to their absolute precision decreasing with the magnitude of the value they hold.  Near the origin, floats can give you extraordinary fine detail;  but if you go away from the origin a few miles in any direction, the float steps get coarser and coarser, from cm to dm, to meters to tens of meters.  So I began to work on an engine that would use ints for the world.  At 0.1 mm per step, 2 billion steps is 200 million mm, or 200,000 m, or 200 km (don't you love metric?);  good enough for a whole city and suburbs, and a few cows, all at hair-width precision;  same precision everywhere.  Who needs float's?

But that only applies to representing positions (points).  Not that there's really anything wrong with expressing angles as integers, too.  I mean, divide the circle into 4 billion (or rather 2^32) parts, and you have ultra-fine angular control, also of uniform precision, and naturally circular as numerics go (overflows just right).  And who needs float's, again?  So I REALLY have no complaints, EXCEPT the fact that the use of Fixed and Float for rotation seems inconsistent in the engine.  THAT is what's driving me crazy:  Sometimes it's floats, sometimes it's fixed, sometimes it's radians, and sometimes it's degrees, and there's often no way to know.  If there seemed to be some "official" way to represent rotation, I would reflect it in my classes;  I just don't know what it is.

Same goes for speed; I find it is often float, even if it accumulates over fixed terrain...  And there's no acceleration, no mass, no moment of inertia, no angular torque or acceleration... But that's my NEXT COMPLAINT :D

And the macros are not mine;  they were in the engine already;  I just used them.

Regarding the graph, the green line is my patch... so you're saying that my patch proved to be faster?  I thought it was...  I mean, I can feel it runs more smoothly.  I used to have occasional freezes, or mixed frames, but since the changes I made, all that went away.

Edited by DanW58
Link to post
Share on other sites

The reason there are both is this one. If you have a camera you don't care about precision cause it's not synchronised so we use floats. I guess that applies to most of the visuals.

But units have rotations too. And those needs to be syncrhonised cause units need to face their targets to be able to shoot. So stuff in graphics/ will probably be floats while stuff in simulation2 will probably be fixed. Does that make sense ?

Link to post
Share on other sites

AHHHHHHHHHHHHHHHHHHHHHH....  I get you now!!!!

 

Alright, time for sleepy pies;  but I think tomorrow is going to be a productive day.  Many Thanks!

 

EDIT:

By the way, the changes I'm working on now are NOT to improve performance.  They are striclty code refactoring that should have NO impact on performance, negative or positive.  So, if you guys are interested in performance but not interested in refactoring, I'd recommend you incorporate that patch you just profiled, and NOT wait for my next one.

My version will eventually get MUCH faster, as once there's clear divisions in the code it's easy to threadify it;  but it's going to take a lot of changes to get there.

And just to be clear, I'm sharing my changes with you in the spirit of GPL, because I intend to fork this engine, make my own game with it, and I don't have a server to put the code on, at the moment, and don't want you to be able to say I violated GPL.  But whether you use my work or not it's your business.  (I know I can put it on sourceforge;  it takes work tho...)

 

Edited by DanW58
Link to post
Share on other sites
1 hour ago, DanW58 said:

 

By the way, the changes I'm working on now are NOT to improve performance.  They are striclty code refactoring that should have NO impact on performance, negative or positive.  So, if you guys are interested in performance but not interested in refactoring, I'd recommend you incorporate that patch you just profiled, and NOT wait for my next one.

The point is your sqrt patch doesn't seem to improve performance which probably means the performance we get from not recomputing the length is lost because the "proper" usage of getter and setters. So maybe something is wrong there ? (I'm just making assumptions I didn't try) I guess an option would be to try first without the getters and setters and then without the optimization. Also langbart tested on Mac which might yield different results because we use SSE3 there (windows is stuck with SSE2)

1 hour ago, DanW58 said:

My version will eventually get MUCH faster, as once there's clear divisions in the code it's easy to threadify it;  but it's going to take a lot of changes to get there.

Looking forward to seeing it :) Promises Promises.
 

 

1 hour ago, DanW58 said:

And just to be clear, I'm sharing my changes with you in the spirit of GPL, because I intend to fork this engine, make my own game with it, and I don't have a server to put the code on, at the moment, and don't want you to be able to say I violated GPL.  But whether you use my work or not it's your business.  (I know I can put it on sourceforge;  it takes work tho...)

You can just fork it on github you know using branches :)

I don't particularly like the idea of spread efforts but one of the goals of 0 A D. is to have fun if that suits you better then I have nothing to say.

I will continue to help if I can but you might soon reach my limits.

Link to post
Share on other sites

Stan, the use of getters and setters DOES NOT impact performance negatively.  This is a huge misconception.   It is a compile-time, stylistic change.

How does code read a public variable in a struct?  It does so "by reference".  So explicitly writing a function that gets at it "by reference" boils down to the same code.

People often think that references are like pointers.  They are NOT.  When you use explicit referencing, in C++, you are basically telling the compiler, "do this however is best;  I trust you".  The concept of reference is intentionally vague, for this reason.  The point is to get SMALLER AND FASTER code, by telling the compiler to optimize the heck out of it.  And where you use a const ref, you are further informing the compiler that this call won't be modifying the thing, which allows the compiler to make wilder assumptions leading to even better performance improvements.  In many cases, getters and setters produce NO CODE;  and where they do, it is often for the better.

Regarding sqrt(), if my patch did not improve performance, the only possible reason for it is that isqrt64() is not being used as much as we may have thought.  I was 50-50 on whether the cacheing idea was going to make a difference;  but my other change, namely using double precision sqrt() inside isqrt64(), for sure should have a measurable impact, otherwise.

Re. "Looking forward to seeing it :) Promises Promises."

I sense something I do not like. 

Re. "I will continue to help if I can but you might soon reach my limits."

Please don't let me reach your limits, then.  I appreciate all the help I got from you.

 

Link to post
Share on other sites
24 minutes ago, DanW58 said:

Stan, the use of getters and setters DOES NOT impact performance negatively.  This is a huge misconception.   It is a compile-time, stylistic change.

How does code read a public variable in a struct?  It does so "by reference".  So explicitly writing a function that gets at it "by reference" boils down to the same code.

Well if you call a function pointer you need more time to resolve things no? Maybe it doesn't take more time in practise but it's still an extra step. Also if the setter assignation is non trivial it might affect optimizations.

I suppose I need to compare assembly :)

24 minutes ago, DanW58 said:

Regarding sqrt(), if my patch did not improve performance, the only possible reason for it is that isqrt64() is not being used as much as we may have thought.  I was 50-50 on whether the cacheing idea was going to make a difference;  but my other change, namely using double precision sqrt() inside isqrt64(), for sure should have a measurable impact, otherwise.

Maybe. I used the perf command on linux to check for such functions like this

perf record 0ad/binaries/system/pyrogenesis \
        -autostart="skirmishes/acropolis_bay_2p" \
        -autostart-ai=1:petra \
        -autostart-ai=2:petra \
        -autostart-aidiff=1:5 \
        -autostart-aidiff=2:5 \
        -autostart-victory="conquest"

then

perf report

You can see there which code gets the most called (doesn't mean it's slow just that it's called a lot)

24 minutes ago, DanW58 said:

Re. "Looking forward to seeing it :) Promises Promises."

I sense something I do not like. 

It's my strange sense of humor, I have heard a lot of things like this in the past ten years, and I hope you'll surprise me :)

Quote

Re. "I will continue to help if I can but you might soon reach my limits."

Please don't let me reach your limits, then.  I appreciate all the help I got from you.

You can quote my comments by hovering over the text in my posts :)

Glad to hear, I always try to help as much as I can. But you will soon be reaching my C++ limits, I'm by no mean an expert, and @wraitii and @vladislavbelov know so much more than I do. I only "fake it until I make it" which sadly can only go that far. I feel ashamed just writing it

 

Link to post
Share on other sites

Stan;  I'm forced to repeat myself:  A reference IS NOT A POINTER.  I repeat:    A reference IS NOT A POINTER.

You are talking about accessing something by pointer as an "extra step", but I did not write getters and setters using pointers;  I wrote them using references, and a reference IS NOT A POINTER.

Let me elaborate:  If we are talking about access to a variable in memory, that access ALWAYS needs an address, of course.  The question is whether that address is embedded with the instruction (access "by reference"), or needs to be pulled from another variable containing it (access by pointer).  To write a get function that gets a private variable by reference is IDENTICAL to making that variable public and accessing it "directly".  In fact, a const int & getX() const is FASTER than using a public ".X" in the sense that it informs the compiler that the call will not be modifying X, which allows the compiler to make additional optimizations in the code, generally.

Can a reference ever be a pointer?  Yes, in some cases it may compile to something like pointer code, but rarely.  Usually it will compile to NO CODE.  But if you tell the compiler to count calls to the get by reference function, then you are FORCING the compiler to implement it as a function, and we have a case of observation affecting the result of an experiment, as in Quantum Mechanics.

But if you don't believe me, indeed, ask wraitii;  he knows what I'm talking about and can probably explain it to you more clearly.  I have my differences with him in terms of taste: he hates to see .getX() functions all over the code;  whereas I love them;  but note that he never made any claims to get functions affecting performance.

EDIT:  Where get by reference functions DO compile to code is if you compile for debug.  Then the compiler HAS to turn the get function into code that the debugger can enter into and exit from, otherwise it would look as if the function failed to compile or something;  which is the truth in nodebug.

Edited by DanW58
Link to post
Share on other sites

Interesting, thanks for explaining it to me.  What about setters ?

I'm prudent about those things because I've seen compilers do weird stuff even for trivial things. Nowadays I run things through Godbolt.com

On 30/01/2021 at 8:47 PM, DanW58 said:

But if you don't believe me, indeed, ask wraitii;  he knows what I'm talking about and can probably explain it to you more clearly.  I have my differences with him in terms of taste: he hates to see .getX() functions all over the code;  whereas I love them;  but note that he never made any claims to get functions affecting performance.

I hope someday I can help you both work out your differences. I am somewhat of an idealist as you may have seen.

EDIT2: Note the command above can also be run in non visual mode, so you only get the the cfixedvector stuff

perf record 0ad/binaries/system/pyrogenesis \
        -autostart="skirmishes/acropolis_bay_2p" \
        -autostart-ai=1:petra \
        -autostart-ai=2:petra \
        -autostart-aidiff=1:5 \
        -autostart-aidiff=2:5 \
        -autostart-nonvisual \
        -autostart-victory="conquest"

EDIT3: Here is some extract as you can see isqrt64 is there (it's vanilla not your patch)

  3,75%  main             libstdc++.so.6.0.28       [.] std::_Rb_tree_increment                                                                                       ◆
   3,33%  main             pyrogenesis               [.] CComponentManager::BroadcastMessage                                                                           ▒
   3,12%  main             pyrogenesis               [.] CComponentManager::QueryInterface                                                                             ▒
   2,30%  main             pyrogenesis               [.] VertexPathfinder::ComputeShortPath                                                                            ▒
   2,28%  main             pyrogenesis               [.] CComponentManager::PostMessage                                                                                ▒
   1,78%  main             pyrogenesis               [.] CCmpRangeManager::PerformQuery                                                                                ▒
   1,38%  main             libmozjs78-ps-release.so  [.] js::AtomizeString                                                                                             ▒
   1,32%  main             libc-2.31.so              [.] __memmove_avx_unaligned_erms                                                                                  ▒
   1,15%  JS Helper        libmozjs78-ps-release.so  [.] js::jit::BacktrackingAllocator::tryAllocateRegister                                                           ▒
   1,13%  main             pyrogenesis               [.] isqrt64                                                                                                       ▒
   1,00%  main             libmozjs78-ps-release.so  [.] js::NativeSetProperty<(js::QualifiedBool)1>                                                                   ▒
   0,93%  main             libc-2.31.so              [.] _int_malloc                                                                                                   ▒
   0,92%  main             pyrogenesis               [.] CCmpPosition::HandleMessage                                                                                   ▒
   0,88%  main             libmozjs78-ps-release.so  [.] js::jit::GetNativeDataPropertyPure<false>                                                                     ▒
   0,86%  main             libc-2.31.so              [.] __memset_avx2_erms                                                                                            ▒
   0,84%  main             libmozjs78-ps-release.so  [.] JSRuntime::tracePersistentRoots                                                                               ▒
   0,82%  main             pyrogenesis               [.] CCmpObstructionManager::TestLine                                                                              ▒
   0,79%  main             libmozjs78-ps-release.so  [.] js::MapObject::has                                                                                            ▒
   0,78%  main             pyrogenesis               [.] CCmpTerritoryManager::GetOwner                                                                                ▒
   0,78%  main             pyrogenesis               [.] CCmpRangeManager::LosUpdateHelperIncremental                                                                  ▒
   0,75%  main             libmozjs78-ps-release.so  [.] js::NativeObject::addDataPropertyInternal                                                                     ▒
   0,68%  main             libmozjs78-ps-release.so  [.] js::MapGCThingTyped<js::ApplyGCThingTyped<js::InternalBarrierMethods<JS::Value>::preBarrier(JS::Value const&):▒
   0,63%  main             libmozjs78-ps-release.so  [.] js::NativeLookupOwnProperty<(js::AllowGC)1>                                                                   ▒
   0,56%  main             pyrogenesis               [.] CCmpRangeManager::ComputeLosVisibility                                                                        ▒
   0,55%  main             pyrogenesis               [.] LongPathfinder::HasJumpedVert                                                                                 ▒
   0,52%  main             pyrogenesis               [.] FastSpatialSubdivision::GetInRange                                                                            ▒
   0,51%  main             libmozjs78-ps-release.so  [.] js::jit::GetNativeDataPropertyByValuePure<false>                                                              ▒
   0,51%  main             libmozjs78-ps-release.so  [.] js::NativeObject::lookupPure                                                                                  ▒
   0,51%  main             libmozjs78-ps-release.so  [.] JS::Zone::getOrCreateUniqueId                                                                                 ▒
   0,51%  main             pyrogenesis               [.] CComponentManager::LookupEntityHandle                                                                         ▒
   0,48%  main             libmozjs78-ps-release.so  [.] js::MapObject::get                                                                                            ▒
   0,48%  main             libmozjs78-ps-release.so  [.] js::NativeGetProperty                                                                                         ▒
   0,44%  main             libc-2.31.so              [.] __memcmp_avx2_movbe                                                                                           ▒
   0,44%  main             libc-2.31.so              [.] malloc                                                                                                        ▒
   0,44%  main             pyrogenesis               [.] CCmpRangeManager::UpdateVisibilityData                   

 

Link to post
Share on other sites
On 30/01/2021 at 4:03 PM, Stan` said:

Interesting, thanks for explaining it to me.  What about setters ?

I'm prudent about those things because I've seen compilers do weird stuff even for trivial things. Nowadays I run things through Godbolt.com

Well, it depends on the type of setter.  What I've done is BAD, admittedly;  I simply wrote functions that expose the members directly, e.g.,

    float & Xref() { return X; }

That boils down to the same thing as making X public;  it prevents the compiler from making its best optimizations.  Note that through float& the variable can be read as well as written, so that some no-aliasing assumptions cannot be made.  What I should have written is something like

    void setX( float const & f ){ X = f; }

But then I would have to work hard for two months fixing code to call this function, as sometimes the code has "foo.X += 5;", which indeed reads and writes X.  I intend to do it, but not just yet;  I want to pick the low-hanging fruit for now.

The first column in Perf output is total time spent?  If 1.13% of the time is spent in isqrt64() vanilla, I would expect my code to spend half as much time, such that there should be about 0.6% performance gain, which is small, but not entirely insignificant.

I have good news.  This new approach is starting to bear fruit:

Spoiler


(In H file:)

template < typename T >
class CEulerDegrees
{
	T X, Y, Z;
public:
	CEulerDegrees() : X(0), Y(0), Z(0) {}
	CEulerDegrees( T const & x, T const & y, T const & z ) : X(x), Y(y), Z(z) {}
	CEulerDegrees( CEulerRadians<T> const & r );
	template < typename U >
	CEulerDegrees( CEulerDegrees<U> const & r );
	T getX() const { return X; }
	T getY() const { return Y; }
	T getZ() const { return Z; }
	T& Xref() { return X; }
	T& Yref() { return Y; }
	T& Zref() { return Z; }
};
template < typename T >
class CEulerRadians
{
	T X, Y, Z;
public:
	CEulerRadians() : X(0), Y(0), Z(0) {}
	CEulerRadians( T const & x, T const & y, T const & z ) : X(x), Y(y), Z(z) {}
	CEulerRadians( CEulerDegrees<T> const & d );
	template < typename U >
	CEulerRadians( CEulerRadians<U> const & r );
	T getX() const { return X; }
	T getY() const { return Y; }
	T getZ() const { return Z; }
	T& Xref() { return X; }
	T& Yref() { return Y; }
	T& Zref() { return Z; }
};

(in CPP file:)

template <> CEulerDegrees::CEulerDegrees<float>( CEulerRadians<float> const & r ) :
  X(RADTODEG(r.getX())), Y(RADTODEG(r.getY())), Z(RADTODEG(r.getZ())) {}
template <> CEulerDegrees::CEulerDegrees<float>( CEulerDegrees<fixed> const & r ) :
  X(r.getX().ToFloat()), Y(r.getY().ToFloat()), Z(r.getZ().ToFloat()) {}
template <> CEulerDegrees::CEulerDegrees<fixed>( CEulerDegrees<float> const & r )
{
	X.FromFloat(r.getX());
	Y.FromFloat(r.getY());
	Z.FromFloat(r.getZ());
}
template <> CEulerRadians::CEulerRadians<float>( CEulerDegrees<float> const & r ) :
  X(DEGTORAD(r.getX())), Y(DEGTORAD(r.getY())), Z(DEGTORAD(r.getZ())) {}
template <> CEulerRadians::CEulerRadians<float>( CEulerRadians<fixed> const & r ) :
  X(r.getX().ToFloat()), Y(r.getY().ToFloat()), Z(r.getZ().ToFloat()) {}
template <> CEulerRadians::CEulerRadians<fixed>( CEulerRadians<float> const & r )
{
	X.FromFloat(r.getX());
	Y.FromFloat(r.getY());
	Z.FromFloat(r.getZ());
}

(USE EXAMPLE:)
  
class TNSpline : public SNSpline
{
public:
	virtual ~TNSpline();
	void AddNode(const CFixedPoint3D<WORLD>& pos, const CEulerRadians<fixed>& rotation, fixed timePeriod);
.....................

 

Isn't that clearer and safer than,

     void AddNode(const CVector3D& pos, const CVector3D& rotation, fixed timePeriod);

?

Now the problem I'm having is with source/simulation2/components/CCmpCinemaManager.cpp:282...

The problem is, Cinema refers to the camera, doesn't it?  But this is under simulation2 !!!

EDIT:

Another seeming inconsistency... In file Render.h,

struct SDashedLine
{
	/// Packed array of consecutive dashes' points. Use m_StartIndices to navigate it.
	std::vector< CVector2D > m_Points;
.................

Why is this not CFixedVector2D ?   Isn't this world ground coordinates?

EDIT2:

Never mind;  I think I got it:  Entities positions are given in CFixedVector2D, but these dash lines and stuff are not crucial to making games deterministic on input so they don't matter as much and float is used then.  Ok, this kind of breaks my neat scheme of having a unique representation in each frame of reference.  So, there's an entities representation and a non-entities representation in each FOR...

 

--------------------------------------------------------------------------------------------------------------------------------------------------------

Edited by DanW58
Link to post
Share on other sites

 Could someone help me understand this piece of code?  I've been stuck for two days here.

It's in the file source/simulation2/components/CCmpFootprint.cpp, the function PickSpawnPoint(), half way down, where it reads,

 				if (m_Shape == CIRCLE)
 					// Multiply the point by 2pi / 6*(distX+r) to get the angle.
					pos += u.Rotate(fixed::Pi() * (offsetPoints[r] + k) / (3 * (distX + r))).Multiply(halfSize.X + gap * r );
 				else
 				{

The problem is that I'm precisely immersed in making clear separations between what is a point and what is a vector and what is a unit-vector, etceteras.

In my system, a point can be added to a vector, to get another point, for example;  but a point cannot be added to another point.  Much less can a point be multiplied, unless there is a way to end up with a "point cluster".  But here in this code pos is referred to as a point, in comments, but implemented as a vector;  and this comment says that we are going to "Multiply the point..." (literally! How does one "multiply a point"?) by what looks to me like an angle,  sort of like dividing a cat's tail by a magic flavor, allegedly to obtain yet another angle....  So this "point" is some kind of number that multiplied by an angle yields yet another angle?!?!  2pi/6 (or pi/3) is an angle, alright, (60 degrees in radians);  that much I know.  The (distX+r) is as clear as mud even though I know r is some kind of "row number".  And then this act of magical multiplication is implemented by operator+=();  yep...   But the worst part is I can't figure out what pos is, or what it is supposed to be.  A point or a vector?  If I make it a CFixedVector3D<WORLD>, a dozen things break;  and if I make it a CFixedPoint3D<WORLD> about two and a half dozen things break...

Thanks in advance.

Link to post
Share on other sites
33 minutes ago, DanW58 said:

Could someone help me understand this piece of code?  I've been stuck for two days here.

It's in the file source/simulation2/components/CCmpFootprint.cpp

What you could do is look up the file ( https://trac.wildfiregames.com/browser/ps/trunk/source/simulation2/components/CCmpFootprint.cpp ) and use the svn blame and revision log history. No guarantees, but you might find something (or someone) that could help you further.

Link to post
Share on other sites

It's interesting that that exists, but it doesn't answer my question about what pos is.

I think a clue is in the lines...

                // Scale and convert the perimeter coordinates of the point to its offset within the row.
236	                int x = (offsetPos.Dot(u) * distX / halfSize.X).ToInt_RoundToNearest();
237	                int y = (offsetPos.Dot(v) * distY / halfSize.Y).ToInt_RoundToNearest();

I've no idea what that means, really, but I have a feeling that pos is really a vector;  NOT a position;  a vector at the center of the spawning building, that can be rotated to identify a spawning point, or something of the sort.  But I need positive confirmation and technical details of how all of this works.

Specially the lines I showed in my previous post, with the abominably confusing comment...

 

Edited by DanW58
Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...