swordlegend / recastnavigation

Automatically exported from code.google.com/p/recastnavigation
zlib License
0 stars 0 forks source link

Custom memory management #78

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Allow users to provide custom memory manager via interface.

Original issue reported on code.google.com by memono...@gmail.com on 7 May 2010 at 11:33

GoogleCodeExporter commented 9 years ago
I've added custom allocators to Detour, since I'm only using Detour in my game 
code, not Recast. I've used the same method as Bullet, but without the option 
to align allocations (fow now). The same method could be applied to Recast. 
Attached patch in case it is of use.

As an aside, for things like this it might be good to have common headers 
between Detour and Recast.

Original comment by cameron....@gmail.com on 28 Jun 2010 at 10:04

Attachments:

GoogleCodeExporter commented 9 years ago
Added allocation hints to original patch.

Original comment by cameron....@gmail.com on 28 Jun 2010 at 10:42

Attachments:

GoogleCodeExporter commented 9 years ago
Looks good! I'm glad there are so few changes :)

There seems to be some people who dislike those global vars. I think the 
allocations done with Detour (and Recast too) are so simple that it would be 
possible to pass allocator interface (which has alloc/dealloc func pointers or 
virtual funcs) to the required functions instead. Or in case of Detour navmesh, 
it would be possible to pass the allocator via constructor.

What do think about that?

Original comment by memono...@gmail.com on 28 Jun 2010 at 12:46

GoogleCodeExporter commented 9 years ago
Yeah that would be OK. I originally did had a struct containing function 
pointers that I passed around, but decided there would be less code changes the 
globals way (and it was good enough for Bullet). The main reason I went for 
less code changes means less svn conflicts when merging from you, but if you 
adopt the changes it's not really an issue :)

The only thing is you need to keep a copy of that allocator in dtNavMesh, 
dtNodePool and dtNodeQueue for the destructors to use.

Original comment by cameron....@gmail.com on 28 Jun 2010 at 7:58

GoogleCodeExporter commented 9 years ago
Good. I think I will then go for that struct solution then. It will also allow 
people to decide if the navmesh builder allocations should go into Recast or 
Detour allocator, as it is sort of there in the middle.

My idea is to also have some other stuff in the interface (or maybe separate 
interface, and one context struct, we'll see), such as timers and log for 
example, so that both recast and detour will need to rely less on OS and that 
you can disable some of the verbose functionality.

Original comment by memono...@gmail.com on 28 Jun 2010 at 8:33

GoogleCodeExporter commented 9 years ago
While you are thinking about this stuff, an assert macro would be nice :)

Original comment by cameron....@gmail.com on 28 Jun 2010 at 9:37

GoogleCodeExporter commented 9 years ago
Yes it would :) Any recommendations which works on win32/osx/linux?

Original comment by memono...@gmail.com on 28 Jun 2010 at 9:44

GoogleCodeExporter commented 9 years ago
Nothing fancy, just a macro that a user can easily override to plug in their 
own assert handler.

e.g.

#ifdef NDEBUG
#define dtAssert(x)
#else
#include <assert.h> 
#define dtAssert assert
#endif

Assert is standard, so will work on all OSs. This makes it easy for someone to 
redefine what dtAssert does and have all asserts go through their own handler.

This is also how Bullet handles it 
http://code.google.com/p/bullet/source/browse/trunk/src/LinearMath/btScalar.h#12
5

Original comment by cameron....@gmail.com on 28 Jun 2010 at 9:56

GoogleCodeExporter commented 9 years ago
I did first rough implementation of the custom allocator based on your patch. 
Some things get ugly when the allocator is passed around, but I think I'll let 
it be that way.

I wish C++ was better at this. Initially I wanted to use something like: 
new(customAllocatorInstace) dtNavmesh; But the destructor for that is so ugly 
that it scares me away and so complicated that I cannot remember how to write 
it. Added C-style alloc/free helper functions for dtNavMesh instead.

I guess I need to let the implementation simmer a little to see how bad it 
feels, and few more tests and I'm ready to submit.

Original comment by memono...@gmail.com on 8 Jul 2010 at 2:05

GoogleCodeExporter commented 9 years ago
I went back to the Bullet way. I just cannot live with how ugly the interface 
version looks like and how much confusion it adds to the people who does not 
care about it.

Detour is custom-allocatorized in R176 (not yet in, googlecode keeps failing). 
I will handle Recast soon.

Original comment by memono...@gmail.com on 9 Jul 2010 at 10:20

GoogleCodeExporter commented 9 years ago
Added support for Recast in R177.

Original comment by memono...@gmail.com on 9 Jul 2010 at 12:58