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

Gltf instance improvements. #377

Open
All8Up opened this issue Mar 10, 2022 · 10 comments
Open

Gltf instance improvements. #377

All8Up opened this issue Mar 10, 2022 · 10 comments

Comments

@All8Up
Copy link

All8Up commented Mar 10, 2022

When loading a Gltf, it is desirable to have an optional "root" transform inserted which all of the gltf nodes are parented to. The basic reason is that loading a Gltf may end up providing multiple root nodes for different portions of the model but using the model as an entity you need (prefer at least) a single 0,0,0 located transform you can manipulate to move the entire model. If this addition is made, it would be handy to have a small API which provides access to it directly without inspecting the node tree.

My current use case is to occasionally visualize the results of a heavily threaded simulation engine. I had made a change to a pre-anim fork, example:
WhenPigsFly
It works, but the 20,000 flying pigs and 100 cherries they are chasing drops the FPS to 5ish though the sim is running at ~30ish FPS on my laptop.. I would love to update to latest but changes made it difficult and rendering is not my focus right now.

@setzer22
Copy link
Contributor

setzer22 commented Mar 10, 2022

I think I'm missing something here 🤔 Why does having an single root transform in the Gltf save you from having to inspect the node tree?

The transforms you upload to rend3 are global, so you still need some kind of transform propagation system to calculate the positions of sub-nodes given their chain of parent transforms. So presumably you still need to calculate the transforms for sub-meshes no matter what.

20,000 flying pigs and 100 cherries they are chasing drops the FPS to 5ish though the sim is running at ~30ish FPS on my laptop

Do you mean this is related to the fact that there's not a single root node?

@All8Up
Copy link
Author

All8Up commented Mar 10, 2022

My apologies for not being clear. The loading of the Gltf remains the same, but when you create an 'instance' of the scene and add it to the renderer, that is where I inserted the root transform for the above pigs. The latest code has the separation of loading and instancing, which I had hacked into an older version with the option of adding the root transform.

As to the perf, no that's just because I'm abusing Rend3 for the most part. A proper instancing solution would solve much of the perf problem but I figure that's out of scope at the moment. I'm not really concerned about the performance too much right now as I only enable this once in a while to see that the sims is functional. (And usually with only 5-10k pigs, the above was extreme.)

@setzer22
Copy link
Contributor

setzer22 commented Mar 10, 2022

My apologies for not being clear.

Oh, no, don't worry at all! 😄 I just wanted to understand your issue better.

The latest code has the separation of loading and instancing, which I had hacked into an older version with the option of adding the root transform.

I see, so you'd like an option for the new version to add this "virtual" root transform node, is that right? That doesn't look like a very hard change. But I must admit I'm still not sure why do you need it, because you still need to traverse the hierarchy either way.

Note that one major change in the node hierarchy is that it is now flattened. Instead of having nested Vecs, you get a single flat Vec of nodes, where each node contains an index to their parent (or None, for the root nodes). So one thing that has changed substantially with the new version is the way you iterate the hierarchy of nodes.

@All8Up
Copy link
Author

All8Up commented Mar 10, 2022

The root in the scene I'm using is rotated and positioned relative to origin (i.e. not identity). In order to maintain the proper orientation etc, the inserted root is identity with the model space root(s) as a child to maintain the relative information. I'm assuming like most renderers that Rend3 is following the hierarchy as it issues the drawcalls but from my perspective I only change the root, the pig and jetpack (the two root nodes in the source) follow along correctly given the inserted parent.

As to the changes in the current repo. I very much like the modifications, that's one of the reasons I was hoping to update. I was also hoping to stand up EGui or Imgui but those were more significantly changed (the render graph side) so I'd have to rework everything anyway, I just prefer to not end up hacking this bit again if possible.

Of course, thinking about it further, I'm wondering if the lack of an identity root when loaded is possibly just the exporter side being dumb, unfortunately I'm crap with the DCC tools, so I could have been using those incorrectly when I exported the model... I'll go investigate that first thing today.

Perhaps a small modification to the request before I dig into the exporter side. An example of the instancing in the new code could be useful. Having looked at it briefly I was not entirely certain what to pass to the instancing function after having read in the Gltf, it wanted the loaded GltfScene and some other items that were not immediately clear.

@setzer22
Copy link
Contributor

setzer22 commented Mar 10, 2022

I'm assuming like most renderers that Rend3 is following the hierarchy as it issues the drawcalls but from my perspective I only change the root

Oh, this is what I meant on my first message. That's not how it currently works. The core of rend3 only understands about Objects: That is, a specific combination of a mesh, a material and a transform. These transforms are global, there's nothing linking them in a hierarchy from rend3's point of view.

If you have a gltf model consisting of a hierarchy of objects, that's just disconnected meshes to rend3. There's nothing that takes the parent transformation and applies them to the children. This currently needs to happen in user code.

But that doesn't mean rend3-gltf couldn't provide a helper function to do this operation. I think that would also be quite helpful in your case, something like:

impl GltfSceneInstance
    pub fn propagate_transforms(&self, renderer: &mut Renderer, root_transform: Transform) { ... }
}

Do you think that solves your issue? The propagate_transforms function would take a root_transform node that applies to all the root nodes in the hierarchy, and you'd just need to call that function every frame with the desired transform for each scene instance.

@All8Up
Copy link
Author

All8Up commented Mar 10, 2022

Oh!!! Well how the hell does my hackery work then! Doh.. :) I did that real quick a month or so ago just to get things on screen for debugging. I'm just coming back to it now to try and undo the hackery since I have the sim side fixed up for the most part and wanted to get visualization going again. I wonder if I have a bug that was effectively making it work just for that model and I simply didn't notice/care given it worked for my quick needs.

Keep in mind, that is a new function. There was no split between the scene and instance when I initially pulled in Rend3, so I've not really poked at the new stuff, just peeked so far.

@setzer22
Copy link
Contributor

Oh, I see! Yeah, I'm not sure why that worked on your end but was probably due to a coincidence 😄

Keep in mind, that is a new function.

Oh, no. What I mean is that this function doesn't even exist yet. I was asking if you think a function like that could be a solution to your problem.

@All8Up
Copy link
Author

All8Up commented Mar 10, 2022

Oh, I see! Yeah, I'm not sure why that worked on your end but was probably due to a coincidence 😄

Actually, I just checked. Seems I actually did the propagation myself and just forgot about that. I remember I did significant hackery, turns out it was more than I remembered.

Keep in mind, that is a new function.

Oh, no. What I mean is that this function doesn't even exist yet. I was asking if you think a function like that could be a solution to your problem.

The only problem with that proposed function is fairly specific to what I'm doing, the "&mut Renderer". I use a custom threading solution which basically extends Rust borrow rules into the threading domain. I used terminology from GLSL such that I have "uniform" and "varying" data where uniforms are shared among all entities and varying data is per entity. In this case the Renderer is a uniform and when iterating entities it can't be borrowed mutably since entity iteration is parallel. This is not the most horrible thing in the world and I can just switch it to being a serial executed code path, but... The benefit, in this case, is getting the quat+translation+scale->Mat4 + the per instance propagation done in parallel before the access to the renderer takes place. I would loose that of course and as the old call to "renderer.set_object_transform(handle, transform)" showed in VTune as being the primary bottleneck when rendering was enabled, it would probably make that worse, but it would work.

So, depending if you want to plan for parallel issuance of updates, that would work fine but end up slow for my use case, or perhaps we could approach it slightly differently? Up to you of course, I just prefer a faster solution if viable. Go figure. :) There are several possible approaches, I'm just not sure what would be best for Rend3 because my needs are likely just a "touch" out of your intended scope right now...

And crap, hit the close & comment button on accident.. :(

@All8Up All8Up closed this as completed Mar 10, 2022
@cwfitzgerald cwfitzgerald reopened this Mar 10, 2022
@setzer22
Copy link
Contributor

it can't be borrowed mutably since entity iteration is parallel

In fact, I just remembered rend3 uses interior mutability in many ways. So you really don't need a &mut Renderer to set the transform, just a &Renderer.

What happens internally when calling set_transform is that the call is stored as an Instruction. All the instructions are stored and get applied in batch when you call renderer.ready().

So yeah, the signature of the function I was suggesting would definitely take a &Renderer.

@All8Up
Copy link
Author

All8Up commented Mar 11, 2022

That makes sense and will likely work no better/worse than the current solution. I figure when Rend3 starts worrying about instancing and such I can revisit things and maybe get the ridiculous entity counts to render relatively decent. :) On the other hand, the root node problem seems to have been due to not having flattened the history in the DCC tool prior to export. Uh, doh... So, initial request seems moot. :)

The only other question is an API item. When calling "instance_loaded_scene" it seems to want the "LoadedGltfScene" and also some data from the original source file. Is there a reason the loaded data doesn't pull that out itself? It just seems odd to have a "LoadedGltfScene" but not be able to instance it without the original file also. I can pipe it through the system, it just strikes me as rather redundant and actually pretty expensive even for the silly pig model I have.

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

3 participants