Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Importer duplicates accessors in memory for every mesh that references it, causing extreme memory usage #29

Closed
JoeyVrowl opened this issue Jul 4, 2018 · 3 comments

Comments

@JoeyVrowl
Copy link

JoeyVrowl commented Jul 4, 2018

There is a problem regarding shared accessors. If you have a single accessor containing all vertices for the .gltf file, but there are multiple meshes, then all vertices in that accessor will get loaded into every mesh. This is a big problem when you have a big .gltf with hundreds of meshes. For instance, we had a .gtlf with 150k tris, but when loading it, the importer would load 260 million verts because of the duplication. This caused Blender to sit at a whopping 10GB memory usage, in an empty scene with just that model.

Here is a simple example of 2 boxes. The caps are separate meshes, and the body is a single mesh. So 3 submeshes per box for body and caps, 48 vertices total in the accessor. This means 2348=288 vertices loaded in Blender, where we would only expect 48 vertices loaded.
incorrect.zip

This problem came to light when exporting using the Sketchup GLTF exporter by CentaurMare. This plugin puts all vertices in a single accessor to save a bit of file size. The author of that plugin was very helpful, and we had some back and forth to come to this conclusion. The author says he discussed the problem on https://gitter.im/KhronosGroup/glTF to check if shared accessors like that are part of the .gltf spec. Consensus was that it is, and so it should be a bug in the importer in Blender rather than the exporter in Sketchup. Maybe worth noting is that the official Unity GLTF importer exhibits the exact same issue as the Blender importer, I have made a report there as well.

@scurest
Copy link
Collaborator

scurest commented Jul 17, 2018

Ah, I see what you mean. If there are four vertices, but the indices array is [0,1,2], we should only "draw" the first three vertices, but we draw them all.

Can you try my iss29 branch and see if it's fixed there?

@JoeyVrowl
Copy link
Author

I am extremely swamped at work right now, I can hopefully check it out next week. Thanks for looking into it.

@JoeyVrowl
Copy link
Author

Hi,

I tried the branch, and it appears to be working fine now. No more crazy high vertex counts or memory usage, and I don't see any obvious problems with the models themselves. Thanks!

ksons pushed a commit that referenced this issue Aug 15, 2018
* Use empties instead of an armature for glTF nodes

This removes support for skinning.

* configure the camera from the gltf data

* camera/create_camera: intentionally return an uninitialized camera for an unexpected type

* camera/create_camera: handle a missing zfar in the perspective data

* camera/create_camera: fallback behavior to handle a missing apesctRatio

* create_camera: the glTF yfov must clearly go to the property `angle_y`
of the Blender camera, while the `aspectRatio`, if present, affects
the resolution instead of the `angle_x` (since the latter is computed
from the `angle_y` and the resolution in Blender)

*  removed auto smoothing

* Tidy up __init__ and unify the caches...

...using just one cache-lookup function instead of one for every
different type of object.

* Tidy up the mesh creator and enable RGBA colors

RGBA vertex colors are supported (required) by Blender 2.79 it
appears.

* Begin implementing a new node creator

Based on creating first a virtual forest that mirrors the scene
graph we're about to create in Blender. Details to be documented
later... if it works out :)

* Creates skins and attach them to armatures

Skinning is now working (again (again))...

* WIP animation loading

Messy and VERY slow for the moment, but functional so I'm commiting
it for my own reference.

* Put animation handling in order

Comments explained the main reasoning behind the bone function.

Also switching from keyframe_points.insert to the add/co loop
gave a **huge** performance boost.

* Use memoryviews for buffers and buffer views.

This is for faster slicing.

This doesn't make any apparent impact on import times but hopefully
it saves some memory ^^;

* Cleanup and explain how the vforest works

* Note that Blender 2.79 is needed

...because 4-component vertex colors are assumed.

Should we use some shims to papers over the Blender version
differences and get greater support? I don't have access to !=2.79
vresion atm though so it doesn't matter for now.

* Minor tweaks

* Put cameras in the scene

They don't point the right way or anything but they're there :)

* Handle creating scenes again

But have an option to just put everything into the current scene
too (I like that better).

* Add more import options

* Cache images used for texturing

This prevents loading the same one multiple times.

* Autogenerate node group data

Using the groups from the exporter, thus relieving us form writing
all our own node groups.

The group for Specular Glossiness is also part of the serialized
data, so supporting that is getting closer.

* Small work-around to enable vertex color influence

We add superfluous COLOR_0 layers to any mesh that uses a material
that is ever applied to a primitive that has a COLOR_0 attribute.
This let's us add an Attribute node to those materials without the
problem of solid red colors coming in.

COLOR_1, etc. are still not supported (well, they're imported of
course, they just don't influence the materials).

* Tighten up scene graph creation

- Avoid creating dummy vnodes for meshes when possible.
- Rotate cameras to their (correct?) position
- Some slightly tighter docs.
- Some more robustness in realizing vnodes; it would probably be
  possible to use an armature for the whole forest again with minimal
  code changes

* Fix some typos in animation.py

Animating non-bone locations should work now. Curve clean-up should
be working too.

* Avoid bone->nonbone->bone child relations

The vtree beneath an armature should now be all bones except
possibly at the leaves, and we should realize them correctly(?)
now.

Also fix a typo with mesh names.

* Rename node.py -> scene.py

Avoids confusion with the similarly named node_groups stuff.

* Large push towards full material implementation

Support for pbrSpecularGlossiness was added here too.

* Fill in sampler properties

Such as they are, anyway.

* Support KHR_materials_unlit extension

* Gate animation importing behind a flag...

...since it's still weird. Make the options prettier and ignore
scenes by default too.

* Allow for >4 vertex weights

...by using multiple attributes sets. We anticipate here, since
though there is a PR for this, it hasn't been merged into the spec
yet.

* Bugfix: do not modify vertex color accessor arrays

If the accessor was used again later in the glTF, we would have
seen a modified array since we hard-modified the accessor elements.
Avoid doing this.

* Bugfix: don't set vertex weights of 0.0

If the joints were eg. [0,0,0,0] and the weights were [1,0,0,0],
previously we would overwrite the weight for 0 three times so it
ended up as 0. Now it will correctly be 1.

* Support MSFT_texture_dds extension.

We hardly have to do anything since Blender will already handle DDS
textures so we might as well. Untested though.

* Implement KHR_texture_transform

Untested.

* Greatly simplify animation importing

I initially thought the pose TRS was computed from (final TRS) *
(rest TRS)^{-1} (ie. the opposite order of multiplication from what
I now believe is correct.) Under this scheme, the pose components
could depend on multiple final components, which required bringing
the curves under a common domain. But under the correct order of
multiplication each pose components depends only on its corresponding
final component (and the rest TRS) so that turns out to have been
over-complicated and, thank God, we can finally get rid of it!

Though, alas! it does not fix Monster.

* Add COLOR_X, etc. layers in order of X

This ensures that COLOR_0 is always the first (and ensures Blender
thinks its the default, for eg. TEXCOORD_0 and UV maps?) and avoids
any gaps in the sequence of X.

* Slightly better bones

- Pick better bone lengths (when the bones are rotated the armature
  actually looks plausible now)
- Report a warning when bones had non-unit scalings in the glTF

* Only add vertices referenced by the indices (#29)

* Work-around re vertex colors for Blender 2.79.0

* Set animation curve interpolation

Also fix a typo that mysteriously did not prevent Blender from
figuring out what the fcurves were targeting.

* Allow the user to rotate bones

For example, try rotating the bones in CesiumMan or Monster by 90d
in the X direction to get the skeleton to look correct.

I am somewhat uncertain of the math for this.

* Rotate the short way for linear interpolation

See KhronosGroup/glTF/issues/1395. Observe the difference in
AnimatedTriangle or the low-poly fox.

* Be noiser when we put a node in the wrong place

Just so the user knows. Very minor, theoreteical fix for bone
lengths too.

* Polish armature/bone creation pass

The restriction about overlapping armatures is lifted and we no
longer require a skeleton property.

* Add groups to animations

This makes it more like what you'd by adding keyframes by hitting I

* Fix action creation

Fix a typo that created superfluous actions.
Always make the first animation the one that plays by default.
Fix setting of the default action on armatures.
Don't let Blender delete unused actions.

* Fix regression of #16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants