Static B3D writing support

[christianclavet]

Hi,

Nice work JLouisB, you expanded the original patch from Hendu to add support for animations!

Just added the files and made the patch for Trunk. There is no issues in compiling, but there are 2 things to note:
- You need to add the 2 files to the project (CB3DMeshWriter.cpp ad CB3DMeshWriter.h) to your Irrlicht source project, then in IrrCompileConfig.h you need to add near the object loader enabler this:
#define _IRR_COMPILE_WITH_B3D_WRITER_

The makefile is patched, so perhaps by rebuilding the projects the files will be there... But that's currently too complicated for little me!

I've recompiled the library and did not see any compilation errors. When I have more time, I will check it with IRB to see if the saved meshes can be loaded back.
I've submitted the patch to the patch tracker for SVN Trunk (Patched on SVN 5029)
https://sourceforge.net/p/irrlicht/patches/301/

[CuteAlien]

Thanks. Yeah, adding new files to all the project files is always a pain without CMake.

[JLouisB]

Thanks for the advice CuteAlien, I will use the trunk, at least if I want post new patches in the futur.

christianclavet, thanks for the update to SVN and the patch !

[CuteAlien]

Found some time to look at it. Had to make a few minor changes (see patch-tracker). But my main problem is that it doesn't work yet. At least not with the default ninja coming with Irrlicht. My test is here: https://code.google.com/p/irr-playgroun ... dwrite.cpp

Wish I had time to debug, but have to get some sleep now.

[JLouisB]

Ok, I will check your test, but I think that I have tested ninja.b3d with my code and it should work.

If you have tested the generated model by load it in Irrlicht, I think that I had a crash when I have tried to do this, because the b3d loader crash if it can't find the textures or something like this.
I have used Open3Dmod and Fragmotion for my tests.

[CuteAlien]

Yeah - I tried writing in Irrlicht and then loading again. Maybe I have later on also some time to debug ... not sure yet.

edit: *sigh* not today. Wasted whole evening because of a bug in the the patch utility in Debian. So many hours lost all the time just because 50 years ago some guys couldn't decide on a single line-ending format...

[CuteAlien]

Found it. It was writing a wrong brushId and that somehow confused things somewhere. It's checked in now in svn trunk r5095.
Still needs a few minor changes, but works basically. Thanks for the code!

[christianclavet]

Thanks CuteAlien That's great!

[JLouisB]

Thanks.
I have tested the previous patch today, strangely only Irrlicht crash to load the model and the problem looks like random.
Good new if it's fixed :)

[JLouisB]

Hello,

I have a new patch for the B3D exporter for some improvements :
- There is an issue when you try to export an animated model with a low FPS animation value with a float precision for the keys (typically with some formats the keys are in seconds with an FPS value of 1)
No problem in Irrlicht because the keyframes are floats, but in B3D files they are int values.
My fix for this is to add a MINIMUM_ANIMATION_FPS define in the exporter (at 60 for example), and if the FPS of the animation is too low all the keyframes and FPS values are multiplied to reach this minimum and avoid these problems

- Optimization of the exporter, by reducing the number of file->write calls (1 call per vector instead of 3 for example), it makes a big difference especially on the keyframe writing and it reduces the export time by about 60% with my test models.

- Code quality :
-> No more complex calculations on the size of chunks, the size is simply written at the end of the chunk by taking the number of bytes written (simpler way of doing things and less risk of errors)
-> No more calls to the B3DWriter::write() function and Size variables : it was basically used to count the number of bytes written in the file but it's enough to use file->getPos where needed.
-> Minor fixes like typo in variable name, non-Irrlicht types used...

I have tested the updated exporter with different files (Irrlicht models and others) and tested the generated files with Irrlicht and Assimp viewer, no issue found.

Is it possible to integrate these corrections into Irrlicht ?

Here are the new files and the patches :
https://www.dropbox.com/s/74pqg563hyenb ... h.zip?dl=0

[CuteAlien]

Thanks, I'll take a look soon.

[CuteAlien]

The MINIMUM_ANIMATION_FPS is good, looks better than the current solution. The define itself shouldn't be in the header, better to make it a const in that function or put it in the .cpp (to avoid accidental namespace pollution when someone includes that header).

The spelling fixes for numMeshBuffers certainly also OK :-)

The optimizations are tricky. It makes the code simpler, but in my tests it wasn't faster (no difference when writing 1000 meshes in release). The problem is that they have the potential to be a lot slower depending on hardware as it uses file-functions like fseek and ftell instead of a just an in-memory counter. And those functions can be slow. Having writeVector3 etc... is OK again, it's removing the manual counting part which may cause trouble. Given that it's only called 6 times + number of meshbuffers it's usually not too bad (but I did write same file 1000 times which likely caches things, so I can't tell real cost... too lazy to write a better speed-test right now).

So... not sure if I should apply it. Do you want to rewrite it once more to get the size counter back? Should I just apply patch for spell-fix and MINIMUM_ANIMATION_FPS? Or would you prefer if I apply it like that and bad luck to people with slow hard-discs (it will be usually fine)? I'll leave the decision up to you - in the end the patch will have your name ;-)
edit: Btw... for more speed you could try writing everything to a memory-file and then copying that memory-file to disc. But don't do that in the mesh-writer, just something you can try in your app if you need things to be faster.

On a side-node - how do you create the patch files? With svn? Just wondering because svn failed to work with the patch files. And the patches don't have context lines which makes applying them manually also a bit hard. I used your .cpp./.h files in the end for testing which willl work here, but might not always be OK for future patches.

[JLouisB]

Hi,

Thank you for your feedback.
I hadn't considered the cost of seek and tell so I did more testing comparing the 3 versions (unpatched, patched with the chunk size calculation and the original patch) to see if it could have a significant impact on performance when I export on my HDD, I've noted my results here:
https://www.dropbox.com/s/d0ook8t6p39q4 ... s.ods?dl=0

Indeed the performance gain isn't huge when exporting a lot of small meshes but becomes more visible on meshes with more vertices and keyframes (Irrlicht's example meshes are a bit limited in this respect).
Calculating sizes "by hand" seems indeed noticeably more optimal (even if it's not obvious in all the cases and difficult to measure precisely), but the added cost seems negligible compared to the other optimizations of the patch, so in my opinion calculating chunk sizes adds a lot of complexity to the code for a limited benefit.
Personally I think that the patch in its current state is a good compromise between performance and code simplicity.
So if you agree I suggest to also apply this change. (but if you have a concrete example that this change can adds a really significant cost with a standard hdd I'm ok to have the counter back).

I moved MINIMUM_ANIMATION_FPS as a const in the function as you suggest.

yes sorry for the patch, I couldn't retrieve the SVN so I made a patch between 2 zip versions I had !

Here's a new patch (this time really made from the svn) :
https://www.dropbox.com/s/nwnwclqglxsxk ... patch?dl=0

[CuteAlien]

OK, thanks for doing some more speed test and the new patch! Applied to svn trunk r6108.

[JLouisB]

Thank you !