|
Arne Müller
|
 |
« on: 17. July 2007, 08:26:17 pm » |
|
So guys, just commited a rather big change to openmali, I hope this one goes ok, because I changed loads of the vecmath structure. If you want to konw, what I all changed, please read the commit comment I made (was rather long)
One issue actually crashes into the Point3f -> Vector3f problem. To be able to reduce GC-overhead at other places and to increase compability between the vector types, I had to make the Vector2f, 3f and 4f classes extend VectorNf (which actually makes pretty much sense, if you ask me) One such Ocassion would be the compability of MatrixMxNf with e.g. Vector3f.
As we're coding Java I cant make Vector3f extend both Tuple3f and VectorNf.
If we want to keep this change we'll have to think how we want to keep compability with the Point classes. One Option that might work would be to make the Point classes extend the Vector classes. Another Option would be to remove the Point-class at all. I never liked the distinction between the two anyways.
|
|
|
|
|
Logged
|
42.
|
|
|
Marvin Fröhlich
Xith Lord
Administrator
Guru
   
Offline
Posts: 4381
May the 4th, be with you...
|
 |
« Reply #1 on: 17. July 2007, 08:48:50 pm » |
|
Well, I think, it is very important (more important) to have Point3f and Vector3f extend Tuple3f. And it doesn't make sense to let Tuple3f extend VectorNf. And Point3f must stay, since there is an important difference, how Point3f and Vector3f are transformed (vector is not translated). And it just does not make much sense to have a vector for a vertex coord for example.
Therefore I would vote for reverting the class hierarchy change.
I don't have time to have a look at the changes today. But tomorrow I will.
There's another problem, I noticed yesterday. As You may have seen, I added readonly extensions of the basic vecmath classes, where all methods, which modify the instance are overridden with an exception thrown. Unfortunately an overridden method cannot be inlined (as it seems). And so the whole vecmath stuff is much slower if only an extension exists in the classpath, where a specific method is overridden.
So I need to find a better solution for that.
Marvin
|
|
|
|
|
Logged
|
|
|
|
Marvin Fröhlich
Xith Lord
Administrator
Guru
   
Offline
Posts: 4381
May the 4th, be with you...
|
 |
« Reply #2 on: 17. July 2007, 09:15:34 pm » |
|
Actually the vecmath implementation is pretty unusable for xith without Point3f and Vector3f extending Tuple3f.
And for logical reasons: What is the length of a vertex-coord? It's not defined. What is the distance of two vectors? It's not defined. What is the angle between two points? It's not defined. Tuple3f has all the stuff being common for both. So If you can't decide, which to use, use Tuple3f.
Where did you have GC problems with this class hierarchy?
Marvin
|
|
|
|
|
Logged
|
|
|
|
|
horati
|
 |
« Reply #3 on: 17. July 2007, 09:39:39 pm » |
|
There's another problem, I noticed yesterday. As You may have seen, I added readonly extensions of the basic vecmath classes, where all methods, which modify the instance are overridden with an exception thrown. Unfortunately an overridden method cannot be inlined (as it seems). And so the whole vecmath stuff is much slower if only an extension exists in the classpath, where a specific method is overridden.
Standard Java VMs can only inline methods determined to be final at runtime. So it follows that it is unimportant that a method be marked final; however, it IS important whether or not there are any overrides for a method. At such time as additional classes are loaded overriding a method within a VM, any inlined instances of that method are outlined just in case instances might require it. As I understand it from the VM devs, this is even the case when it can be determined that ALL instances passed are of a given class. I'm not sure this will work, but maybe you should try creating an abstract base class making the methods you intend to override abstract. Then, make all concrete subclasses final. You don't actually need to make the subclasses final; however, it will insure your intent is followed and at least force someone to read your Javadoc telling them why the class is final. In theory, any classes that use references to the final classes should be as fast as they were before. If your intent requires use of the abstract class, you won't be able to optimize it. Since we are mucking with these classes, would someone explain the difference between the Coord, Point, and TexCoord classes?
|
|
|
|
|
Logged
|
|
|
|
Marvin Fröhlich
Xith Lord
Administrator
Guru
   
Offline
Posts: 4381
May the 4th, be with you...
|
 |
« Reply #4 on: 17. July 2007, 10:06:34 pm » |
|
I'm not sure this will work, but maybe you should try creating an abstract base class making the methods you intend to override abstract. Then, make all concrete subclasses final. You don't actually need to make the subclasses final; however, it will insure your intent is followed and at least force someone to read your Javadoc telling them why the class is final. In theory, any classes that use references to the final classes should be as fast as they were before. If your intent requires use of the abstract class, you won't be able to optimize it.
No, this is not possible. I have a concrete class e.g. Point3f, which has getters and setters for it's fields. Now I want to have a "really" read-only version of this class but with the exact same interface, since it must be (RO-) usable just like a normal instance of this class. This is possible by overriding the appropriate methods, which prevents inlining. Another solution, I will check tomorrow is to query a final boolean in any of the setters. I will see, if this effects performance. Since we are mucking with these classes, would someone explain the difference between the Coord, Point, and TexCoord classes?
Well, Point3f is a 3-dimensional, spatial point. A Vector3f is a 3-dimensional vector. The Coord3f class combines them. It is transformed like a Point3f and has all the method of Vector3f. I don't know, if this calss is even needed. I think, it can be deleted, since nobody uses it except for one or two classes, which can be rewritten. Well, I will see. TexCoordf is something completely different. It is not part of the new vecmath lib (just as Colorf). The new implementation doesn't even extend Tuple3f/Tuple2f, but is completely indepentent. It has no fields x,y,z,w, but s,t,p,q as it is the standard naming in OpenGL. A TextureCoordinate is not a spatial point, but a normalized local position in a (curved) surface. Marvin
|
|
|
|
|
Logged
|
|
|
|
|
horati
|
 |
« Reply #5 on: 17. July 2007, 11:43:54 pm » |
|
No, this is not possible. I have a concrete class e.g. Point3f, which has getters and setters for it's fields. Now I want to have a "really" read-only version of this class but with the exact same interface, since it must be (RO-) usable just like a normal instance of this class. This is possible by overriding the appropriate methods, which prevents inlining. Another solution, I will check tomorrow is to query a final boolean in any of the setters. I will see, if this effects performance.
If that's the goal, add the writer methods to a RW class that extends the RO class. In this way, the VM can determine that all methods are final or you can enforce the performance enhancing constraint by declaring the methods final individually. If you get good locality of reference, the final boolean might be faster than overriding; however, it's unlikely to be much, much better. superclass:
protected Foo bar; public final Foo getBar() { ... }
subclass:
public final setBar( Foo bar ) { super.bar = bar; }
is the best solution for this case. Unfortunately, Java's protected modifier allows other classes in the same package to access the attribute; however, that's just something we have to live with. Well, Point3f is a 3-dimensional, spatial point. A Vector3f is a 3-dimensional vector. The Coord3f class combines them. It is transformed like a Point3f and has all the method of Vector3f. I don't know, if this calss is even needed. I think, it can be deleted, since nobody uses it except for one or two classes, which can be rewritten. Well, I will see. TexCoordf is something completely different. It is not part of the new vecmath lib (just as Colorf). The new implementation doesn't even extend Tuple3f/Tuple2f, but is completely indepentent. It has no fields x,y,z,w, but s,t,p,q as it is the standard naming in OpenGL. A TextureCoordinate is not a spatial point, but a normalized local position in a (curved) surface.
Ah, it sounds as if we really don't need Coord3f. I thought life was similar to what you described above; however, I wasn't able to assemble evidence confirming it. I'm not on the other projects or I would copy/paste the above description into package.html files in org.openmali.vecmath and org.jagatoo.vecmath. Perhaps somebody could?
|
|
|
|
|
Logged
|
|
|
|
|
Arne Müller
|
 |
« Reply #6 on: 17. July 2007, 11:46:22 pm » |
|
every point can be described by a vector from the Coordinate Origin to the Point itself.
The length of a Point is simply it's distance to the coordinate origin.
The distance of two vectors v1, v2 is the distance of two points, when one point is v1 from some point p away and the other one v2 away.
With Vector!=Point, we'll always have to copy data from Point to Vector or vice versa (if we don't want to implicitly rewrite those methods every time they are needed.
This especially gets evident, when writing math-methods. Points are nothing else than Vectors, they simply look a bit different.
maybe it would be possible to keep the old hierarchy, I don't know.
about that finalizing of the methods, I don't think I need to extend MatrixMxNf, I'll simply add the possibility of part-array access into the class itself. This makes it much easier, because else I would have had to override nearly all methods in there.
|
|
|
|
|
Logged
|
42.
|
|
|
|
Arne Müller
|
 |
« Reply #7 on: 18. July 2007, 12:37:15 am » |
|
Hi I just committed my pointer-replacement for joode into openmali. It actually consists of some changes in the array-access of the data in the Matrix. But if the existing methods are used to do that, one shouldn't be able to produce additional errors, only direct access is pretty nasty. The new visible methods will be something like public static MatrixMxNf sharedSubMatrixMxNf(int beginRow, int beginCol, int rows, int cols, float[] values) Also instances of Matrix3f or Matrix4f can be created this way. Because this change went so easily, I think, we could really keep the old class hierarchy with the Vector classes extending the Tuple classes (at least it seems possible). There would be only additional methods needed for MatrixMxNf.transform(Vector3f) and we won't be able to do any pooling with VectorNf (even if it has poolable sizes). But I would keep Vector4f extending VectorNf, because there's no Tule4f and also Quaternion4f extending Vector4f. cheers, Arne [AddOn]: the sharedSubMatrix stuff isn't yet GC-free, but it can bemade GC free with another seperate Pool. I would do the change right away, but I think it's better, if Marvin can look into my changes and correct the mess I made before that. 
|
|
|
|
« Last Edit: 18. July 2007, 06:33:34 am by Arne Müller »
|
Logged
|
42.
|
|
|
Marvin Fröhlich
Xith Lord
Administrator
Guru
   
Offline
Posts: 4381
May the 4th, be with you...
|
 |
« Reply #8 on: 18. July 2007, 10:15:29 am » |
|
Because this change went so easily, I think, we could really keep the old class hierarchy with the Vector classes extending the Tuple classes (at least it seems possible). There would be only additional methods needed for MatrixMxNf.transform(Vector3f) and we won't be able to do any pooling with VectorNf (even if it has poolable sizes).
But I would keep Vector4f extending VectorNf, because there's no Tule4f and also Quaternion4f extending Vector4f.
Ah. This is what I thought  . And I don't have a problem and even like the Vector4f extending VectorNf. But I don't like the Quaternion extending the vector. Is there any good reason for this except for code duplication? I would not want the quaternion to have get/setX/Y/Z/W methods or a length methods, etc. [AddOn]: the sharedSubMatrix stuff isn't yet GC-free, but it can bemade GC free with another seperate Pool. I would do the change right away, but I think it's better, if Marvin can look into my changes and correct the mess I made before that.  No problem for the GC-friendliness. This can be tuned later. Thanks for letting me clean up the stuff first. I will do it this evening. Marvin
|
|
|
|
|
Logged
|
|
|
|
|
Arne Müller
|
 |
« Reply #9 on: 18. July 2007, 12:11:02 pm » |
|
Because this change went so easily, I think, we could really keep the old class hierarchy with the Vector classes extending the Tuple classes (at least it seems possible). There would be only additional methods needed for MatrixMxNf.transform(Vector3f) and we won't be able to do any pooling with VectorNf (even if it has poolable sizes).
But I would keep Vector4f extending VectorNf, because there's no Tule4f and also Quaternion4f extending Vector4f.
Ah. This is what I thought  . And I don't have a problem and even like the Vector4f extending VectorNf. But I don't like the Quaternion extending the vector. Is there any good reason for this except for code duplication? I would not want the quaternion to have get/setX/Y/Z/W methods or a length methods, etc. If Quaternion4f would not extend Vector4f, Quaternion4f would need it's own length() and getNorm() methods. Btw. length() in Vector4f computes only the length of the x,y,z components, because mostly we're dealing in 3D-Space and w is just a helper component. If you think this is a bad decision, you're welcome to add the w-component into the computation, but then also change the method in Quaternion4f, which is using it. that Quaternions are Vector4f's just look at Vector4f.getLinearHyperPlaneNormal, it takes Vector4f as points (just another example, why vectors make good points), but it also gets used with Quaternion4f as parameters. So if Quaternion4f doesn't extend Vector4f, we'd probably have to change all or most uses of Vector4f to Quaternion4f, thus making Vector4f more or less redundant.
|
|
|
|
|
Logged
|
42.
|
|
|
Marvin Fröhlich
Xith Lord
Administrator
Guru
   
Offline
Posts: 4381
May the 4th, be with you...
|
 |
« Reply #10 on: 18. July 2007, 04:27:01 pm » |
|
If Quaternion4f would not extend Vector4f, Quaternion4f would need it's own length() and getNorm() methods.
Well, I don't think, length is defined for a Quaternion. And IIRC, the Quaternion4f class has (or at least had) a norm() method. Btw. length() in Vector4f computes only the length of the x,y,z components, because mostly we're dealing in 3D-Space and w is just a helper component. If you think this is a bad decision, you're welcome to add the w-component into the computation, but then also change the method in Quaternion4f, which is using it.
I would suggest to add a length3() method to Vector4f, that computes the length with only the forst three components. But the length() method itself should use all components of the vector. So if Quaternion4f doesn't extend Vector4f, we'd probably have to change all or most uses of Vector4f to Quaternion4f, thus making Vector4f more or less redundant.
Well, why are you using Vector4f so much? If you're using it like a quaternion, why not really use Quaternion4f  ? Vector4f is only used very, very rarely in Xith3D... only when OpenGL needs one. I will create an abstract class called Tuplef, which will hold the methods size() get( int ) set( int, float ) and some conversion getters/setters. It will be a superclass of Tuple2f, Tuple3f, VectorNf, etc. So you can use if for all these tuple classes. Marvin
|
|
|
|
|
Logged
|
|
|
|
Marvin Fröhlich
Xith Lord
Administrator
Guru
   
Offline
Posts: 4381
May the 4th, be with you...
|
 |
« Reply #11 on: 19. July 2007, 12:03:59 am » |
|
So, I think, I cleaned everything up now as described above.
We need to see, if we can move some of the new Quaternion4f methods to Vector4f, since they don't seem to really use a Quaternion, but in fact a Vector4f.
Marvin
|
|
|
|
|
Logged
|
|
|
|
Marvin Fröhlich
Xith Lord
Administrator
Guru
   
Offline
Posts: 4381
May the 4th, be with you...
|
 |
« Reply #12 on: 19. July 2007, 01:17:11 am » |
|
Arne and I looked through the latest changes, corrected some things and now it looks pretty well.
Marvin
|
|
|
|
|
Logged
|
|
|
|
|
'n ddrylliog
|
 |
« Reply #13 on: 19. July 2007, 09:15:42 am » |
|
Arne and I looked through the latest changes, corrected some things and now it looks pretty well.
Cool TMA few questions : 1) When will Xith3D switch to that new version ? 2) What does it change, user-side ? Do the users even need to change any line of their code ? And if yes, what are the main point changes ? I know these questions may have been answered above, but a quick summary is welcome at this point, I think.
|
|
|
|
|
Logged
|
|
|
|
Marvin Fröhlich
Xith Lord
Administrator
Guru
   
Offline
Posts: 4381
May the 4th, be with you...
|
 |
« Reply #14 on: 19. July 2007, 09:52:55 am » |
|
1) When will Xith3D switch to that new version ?
When the COLLADA loader is ready we should do a new Xith3D release. Then the users can fall back to this release, during we're having the one or the other bug in the new math lib. So, when this new release is out, we (I) can start porting Xith to the new OpenMaLi. 2) What does it change, user-side ? Do the users even need to change any line of their code ? And if yes, what are the main point changes ?
Oh, yes. The users will have to change some lines. I think, we will keep the pckage name of "org.openmali.vecmath2" for the new thing, so the imports will have to be changed (simple search-and-replace). The methods have stayed the same mostly. But the biggest change will be the use of getters/setters only for the fields (no public .x, .y, .z, etc.). So people will have to change vec.x = 10f;
to vec.setX( 10f );
There are convenience methods for things like this: vec.y += 3f;
This can be written as: vec.setY( vec.getY() + 3f );
but also as: vec.addY( 3f );
which is pretty convenient  . I think, everything else can stay the same. Marvin
|
|
|
|
|
Logged
|
|
|
|
|