From: Alexander G. <ale...@gm...> - 2010-12-02 14:49:47
|
I would agree that there are some inconsistencies (and even obvious flaws, such as potential memory leaks) in our MD5 loading code (I am the original author - I think I put it together from older code of mine). Any patches are appreciated, of course. I am too willing to spend some time on overhauling it, but I can't say when I'll find time for it. > - How are models with ambiguous file extension (e.g. .mdl, which is used be several file > formats) supposed to be handled? Loads need to know of each other and their CanRead() method is asked to reject files with the correct file extension, but wrong contents. > - Are there any coding guidelines for Assimp? I've found the code to be *really* inconsistent There are no official guidelines. > This would remove (large parts of) the resource allocation issues. The equivalent for > mNumFaces is obviously mFaces.size(), the equivalent of HasFaces() is !mFaces.empty(), the raw > pointer equivalent (for C API access) is&mFaces[0]. Agreed, it would be beneficial. The inner core of Assimp would work with a C++ish aiScene and for C users it would be 'wrapped' by the current, C-compatible data structure. Sadly, it would mean a full rewrite of 50k LOC. I agree that we'd end up having 2/3 of the original code with fewer potential issues, but I just don't see a way to do this. > In other > words, there is no point in submitting patches that you possibly don't like anyways, right? What we're especially interested in are bugfixes. Data structure changes have been proposed multiple times. Additions and minor fixes are fine, but we have always been very skeptical towards intrusive changes. For good reasons, I think. Bye, Alex On 12/2/2010 2:44 PM, Carsten Fuchs wrote: > Hi all, > > I've been reading a bit through the code (especially the MD5 loader, but also elsewhere), and now > have a few more questions, in random order: > > > - Can animations be imported separately from loading the base mesh? I.e., how do you load > multiple animations into a single md5 model? > > - How are models with ambiguous file extension (e.g. .mdl, which is used be several file > formats) supposed to be handled? > > - Are there any coding guidelines for Assimp? I've found the code to be *really* inconsistent > in this regard... > > - What is the preferred way to submit patches? (Do you want any to start with?) Is there a > ticket tracker? > > - In the MD5 loader, I've found use of boost besides code like > > // sort all bone weights - per bone > unsigned int* piCount = new unsigned int[meshParser.mJoints.size()]; > ::memset(piCount,0,sizeof(unsigned int)*meshParser.mJoints.size()); > > where for example > > // For each joint, determine how many weights there are. > std::vector<unsigned int> WeightsPerJoint(meshParser.mJoints.size(), 0); > > had been more correct and appropriate. > There are plenty of similar examples - are you interested in related patches? > > - I see that many class members are of the pattern > > struct aiMesh > { > unsigned int mNumFaces; > aiFace* mFaces; > bool HasFaces() const { return mNumFaces>0; } > }; > > Well, what about > > struct aiMesh > { > std::vector<aiFace> mFaces; > }; > > instead? > > This would remove (large parts of) the resource allocation issues. The equivalent for > mNumFaces is obviously mFaces.size(), the equivalent of HasFaces() is !mFaces.empty(), the raw > pointer equivalent (for C API access) is&mFaces[0]. > > > Sorry for all these questions, but I'm still trying to evaluate if Assimp is the right library for > the Cafu engine, and if and how much time I might spend with helping improving it. :-) (In other > words, there is no point in submitting patches that you possibly don't like anyways, right? ;-) ) > > Many thanks for your help, and kind regards, > Carsten > > > |