Fixed library to use less memory when loading and processing models.

[samsinsane]

This is a pretty big change, but it removes all of the mini-allocations that occurred for indices. These mini-allocations would result in excessive memory usage, at least on Windows (potentially not an issue on Windows 10 though). Resolves #944

[jdduke]

This will break every single Assimp client in existence, and require non-trivial changes when upgrading. Ideally this kind of optimization would preserve the public-facing struct API in some fashion, otherwise it'll probably have to wait until the next major version bump.

I took a stab at something similar a while ago in an attempt to minimize heap usage (master...jdduke:noheap_face). It preserves the outward-facing aiFace struct data members, but eliminates heap allocations for faces with index count <= 4 (albeit in a somewhat crude fashion, index storage on the mesh is a better long term solution, but unless we make mIndices a pointer into the mesh index array, which has its own set of issues, this is a breaking change).

[samsinsane]

If I change it to be a pointer that references the indices array in the mesh, will this PR be accepted? This solution works for us, and I'm happy to make the change if that's the only thing stopping it from being accepted, but I've already spent way too much time working on this.

[kimkulling]

I guess we need to release this changes ( when they are tested and reviewed ) in a release 4.0. As jdduke mentioned before this will break the API and every client need to change his code.

[samsinsane]

I guess we need to release this changes ( when they are tested and reviewed ) in a release 4.0.

What's the time frame on the 4.0 release?

As jdduke mentioned before this will break the API and every client need to change his code.

If the breaking the API is the only thing blocking this from being merged, I can change the mIndices back to a pointer that references the indices array in the mesh? It's up to you, if you want this to wait to 4.0 anyway, then I'll leave it as is.

[kimkulling]

I think we can release 4.0 next .....

Samuel Surtees notifications@github.com schrieb am Fr., 29. Juli 2016
03:54:

I guess we need to release this changes ( when they are tested and
reviewed ) in a release 4.0.

What's the time frame on the 4.0 release?

As jdduke mentioned before this will break the API and every client need
to change his code.

If the breaking the API is the only thing blocking this from being merged,
I can change the mIndices back to a pointer that references the indices
array in the mesh? It's up to you, if you want this to wait to 4.0 anyway,
then I'll leave it as is.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#958 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACNy1CtUpWu8qWtKYfYjLHhAZhemOALXks5qaV1VgaJpZM4JV12q
.

[acgessler]

I think I'd be in favor of not breaking the API and have mIndices refer to the indices array.

Also it should be backwards-compatible -- importers should still be able to allocate their own data arrays. I am not sure how it would work with deleting, perhaps we should add a boolean to aiFace so it knows whether to free the data.

[samsinsane]

Also it should be backwards-compatible -- importers should still be able to allocate their own data arrays. I am not sure how it would work with deleting, perhaps we should add a boolean to aiFace so it knows whether to free the data.

This kind of defeats the purpose of this PR. The large memory consumption I'm experiencing is due to the faces having their own allocations for indices. Additionally, adding more data per face doesn't help with the data consumption problems. This is why I favour the API changing, it prevents this issue from reoccurring by not allowing you to make mini-allocations for each face.

[kimkulling]

One question: will this pull request in the current state break the ABI? If not: we should merge it!

[kimkulling]

I thought quiet a long time about this patch. We should try a different approach before merging this one.
My idea in short:
Using pool allocators for faces. So a big bundle of faces will be allocated in the beginning and we just using this to store data inside of the faces. So we do not have any API-changes ( we have a lot at the moment and from my point of view the patch will create a lot of anger ) and we can get you idea in.

I will prepare a special branch for testing this.

[OsiJr]

This line wipes out the existing mIndices (withing freeing) and results in the index buffer being unallocated.