chenynCV / bullet

Automatically exported from code.google.com/p/bullet
Other
0 stars 0 forks source link

Complete float/double serialization #734

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago

We are using bullet in double precision mode (BT_USE_DOUBLE_PRECISION is 
defined) and would have liked to use bullet's serialization/deserialization 
mechanism.

Unfortunately it seems that separating float / double serialization correctly 
in bullet is not finished. Furthermore it also seems that the current structure 
can not be fixed without breaking compatibility with older versions.
Because of these reasons we decided to implement a parallel 
serialization/deserialization mechanism in bullet where special care was taken 
to distinguish float and double serialization structures.
The name of this new mechanism was postfixed with the DFF abbreviation that 
stands for double-float fix (foundation :-) ). It should be FDF as float-double 
fix, but FDF keyword is not unique in bullet (searching for FDF in bullet 
sources gives too many results, but we wanted something well-distinguishable).
To bring DNA and C++ structure layout closer together double-s are 8 byte 
aligned in the new serialization structures (makesdna is changed accordingly), 
CONDITIONAL_ALIGN is introduced to be able to handle 8 byte alignment problems 
arising from pointer size changes (from 4 byte to 8 byte) and most of the new 
serialization structure sizes are rounded to 8 byte boundary.

The btDefaultSerializer is extended to be able to change the version number 
from 281. This is a little hack to be able to distinguish .bullet files (e.g. 
when you need to know if file was saved using the DFF mechanism)

There is an additional change that is not cruical but was very convenient for 
us. Makesdna now calculates structure sizes with 4 byte and 8 byte long 
pointers at the same time to be able to give more information on how to change 
the serialization structures.

Makesdna currently does not handle alphalen for the new DFF structures as we 
considered it to be too much effort and increase in size compared to the gain 
(supporting an old architecture).

Please take a look at our attached patch and consider either changing the 
current serialization (more or less) similarly or adding this new serialization 
parallel to the old one.
Thank you.

What version of the product are you using? On what operating system?
We are using bullet 2.81.2613 on W7 64 bit, but our builds are usually 32 bit.
The patch is provided for http://bullet.googlecode.com/svn/trunk revision 2645.

The patch also includes some small changes in the original btWorldImporter.cpp.

Original issue reported on code.google.com by nsark...@gmail.com on 26 Aug 2013 at 9:37

Attachments:

GoogleCodeExporter commented 9 years ago
"Unfortunately it seems that separating float / double serialization correctly 
in bullet is not finished"

Can you explain that briefly (short and focussed on issues) ? As far as I know, 
double and float serialization should work fine.

Original comment by erwin.coumans on 26 Aug 2013 at 3:40

GoogleCodeExporter commented 9 years ago
E.g. we are using btGeneric6DofConstraint. Its serialize function uses the 
btGeneric6DofConstraintData structure. Please note that this is a real 
structure and not a #define for a float or double implementation depending on 
the BT_USE_DOUBLE_PRECISION. This structure contains btTransformFloatData-s and 
btVector3FloatData-s, so actually only the float implementation exists.

Short and focused that was the motive of our implementation, but I am glad to 
give more information if needed.

Original comment by nsark...@gmail.com on 27 Aug 2013 at 6:43

GoogleCodeExporter commented 9 years ago
I'm not convinced in the need for introduction of "double-float fix"/FDF, 
CONDITIONAL_ALIGN and a lot of code duplication.

The btGeneric6DofConstraint could be simply fixed to add double/single 
precision, but is that really necessary? 

Does the serialization of btGeneric6DofConstraint not work in double precision, 
or does your application have issues that double precision values in 
btGeneric6DofConstraint are stored in single-precision float values, and hence 
loosing precision?

If you really need double-precision serialization of btGeneric6DofConstraint, 
and possibly other structures, please report the structures that you need 
serialized.

One of the main features of the Bullet 2.x file format is that it is 100% 
backwards and forwards compatible, and we are not going to break that. Also, we 
are currently focusing on Bullet 3.x. At some stage we will introduce a Bullet 
3.x file format.

Original comment by erwin.coumans on 29 Aug 2013 at 5:47

GoogleCodeExporter commented 9 years ago
We are developing an engineering application. So double precision is important 
for us as, because with this it is visibly much accurate and with the current 
code serializing and loading back the scene alter the results.

Another thing: we tried to serialize the RagDoll of RagdollDemo in bullet and 
after deserializing it back we received an assert in 
btSequentialImpulseConstraintSolver::solveGroupCacheFriendlySetup ( 
btAssert(fsum > SIMD_EPSILON); ) when the BT_USE_DOUBLE_PRECISION was defined 
and the original serialization methods were used; while this problem 
disappeared with the new serialization. We checked it was because of the 
clamped precision. (Though we are not saying this error couldn't be solved in 
the old serialization.)

You are right that currently we do not need to be able to serialize all the 
bullet objects in double precision. We only need
-btGeneric6DofConstraintDoubleData and thus btTypedConstraintDoubleData
-btRigidBodyDoubleData and btCollisionObjectDoubleData but those are already 
done
-btGImpactMeshShapeDoubleData and thus btStridingMeshInterfaceDoubleData and 
btMeshPartDoubleData
-probably btDynamicsWorldDoubleData and btContactSolverInfoDoubleData but those 
are already done
But the reason for making a complete solution was that we might need the double 
serialization of more objects in the future and we thought it will be more 
acceptable this way.

I would like to emphasize again that we could not find a solution where the 
100% forward and backward compatibility can be achieved and it would also need 
code doubling to achive partial compatibility.
For examples:
 - There is no double version of btConeTwistConstraintData. So after creation of for example a btConeTwistConstraintDoubleData an older version will not be able to load/create ConeTwistConstraint-s.
 - btHingeConstraintDoubleData in the original serialization system contains floats. So in this case we would need to create a "btHingeConstraintReallyDoubleData" and everwhere where the original serializer code contains different code to float and double we would have to create a third part for the reallydouble structs. Also we would need a new flag (for example FD_REALLY_DOUBLE_PRECISION) so we can recognize that this new struct should be used. With this we would again lose the compatibility with and older version.

We thought it is much nicer to create a clean solution. 
Even if you reject this patch please note for the next release that the current 
version of the serialization has issues regarding to handling double precision 
and it can't be solved trivially. Thank you.

Original comment by nsark...@gmail.com on 2 Sep 2013 at 4:14

GoogleCodeExporter commented 9 years ago
We can add the missing parts for double precision, and fix the 
btHingeConstraintDoubleData issue, while maintaining backwards/forwards 
compatibility.

The main point is that a Bullet 2.x loader should be able to load the .bullet 
file. It is OK to introduce new structures, the old importer will ignore/skip 
them, without crashing.

It is good you point our all specific structures/classes that have double 
precision issues. Once I have time, I'll look into it.

Original comment by erwin.coumans on 2 Sep 2013 at 4:24

GoogleCodeExporter commented 9 years ago
Ok. Thank you.
We are looking forward your fixes.

Original comment by nsark...@gmail.com on 3 Sep 2013 at 8:21

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
>>btHingeConstraintDoubleData in the original serialization system contains 
floats.
>> So in this case we would need to create a 
"btHingeConstraintReallyDoubleData" and 
>>everwhere where the original serializer code contains different code to float
>> and double we 

No, that is not how it works.

You simply replace the 'float' by 'double' add 4 bytes padding, and the 
serialization is backward and forward compatible. Older Bullet versions will 
simply store the double precision data into single precision. No duplication 
needed in that case.

Just try out commit 2649:
https://code.google.com/p/bullet/source/detail?r=2649

Original comment by erwin.coumans on 3 Sep 2013 at 4:46

GoogleCodeExporter commented 9 years ago
Thank you! We will try it out.

Would a similar solution also work for the btTypedConstraintData member of 
btHingeConstraintDoubleData?
I mean it is also necessary to create a btTypedConstraintDoubleData as 
btTypedConstraintData contains float values and after that I would like to 
replace the btTypedConstraintData member of btHingeConstraintDoubleData to 
btTypedConstraintDoubleData. Does that also work the same way?

Original comment by nsark...@gmail.com on 4 Sep 2013 at 6:38

GoogleCodeExporter commented 9 years ago
I updated various constraint structures for double precision. See
https://code.google.com/p/bullet/source/detail?r=2667 and
https://code.google.com/p/bullet/source/detail?r=2666

You should still be able to load old .bullet files (backwards compatible) but 
older Bullet SDKs cannot recognize the new data structures.

I have not changed 
btGImpactMeshShapeDoubleData,btStridingMeshInterfaceDoubleData and 
btMeshPartDoubleData.

Those structures can deal with double-precision meshes already. There are only 
3 fields that are single precision: m_collisionMargin, m_localScaling and 
m_scaling. It would be pity to break compatibility for those. Is it really 
necessary?

Original comment by erwin.coumans on 14 Sep 2013 at 6:31

GoogleCodeExporter commented 9 years ago
By the way, from the latest commit 
(https://code.google.com/p/bullet/source/detail?r=2668) the backwards 
compatibility issue only happens with double precision files.

So BULLETf (single precision floating point) files are backwards and forwards 
compatible, while BULLETd (double precision) files are only backwards 
compatible between Bullet 2.82 and older versions.

Original comment by erwin.coumans on 14 Sep 2013 at 4:54

GoogleCodeExporter commented 9 years ago
I assume the issue is fixed? Otherwise, please open a new issue.

Original comment by erwin.coumans on 22 Oct 2013 at 6:37