My team is still having an issue similar to what Christian reported here:
[Content removed]
We’re on 5.6 and seeing this behavior where ~750ms cycles of FMaterialUpdateContext and FlushRenderingCommands are repeatedly being called as seen here. There’s definitely some work to do on our end to avoid the LoadSynchronous call higher in the stack but I am wondering if there are changes we can pull to improve this?
Steps to Reproduce
Presumably a synchronous load of an object with a number of Niagara assets in a large level would repro this but I have not tried to set up a repro.
I just confirmed that we do have some static bool bindings on some renderers though I haven’t dug deep enough to understand if that’s directly causing this behavior. If you let me know which changes you had in mind I can grab them and see if they fix this.
We just hit this as well (Hello Stu!). We are seeing 10s+ stalls during PIE sessions when we first use a Niagara effect,. For us, compiling the effect results in 140K primitive component render states being recreated. I trapped one of the cases and yes, it is because a Static Parameter is being changed. Callstack below. I added a stat marker to
It is worth noting that both of our teams are using an internal plugin that introduces new types that derive from UNiagaraRendererProperties. These new properties have behavior in their PostLoad that can potentially make modifications to the MaterialParameters object. After learning that Chris’ team was hitting this as well I did a bit more digging and noticed that all of the materials that are hitting this FMaterialUpdateContext within UMaterialInstance::UpdateStaticPermutation are of the type used by this internal plugin. I’ve reached out to the original author of this original plugin to take a look but if you have any advice about where it might be safer to make changes like this other than within the PostLoad I’d love to hear your recommendation.
As an example of what this is doing currently …
void UNiagaraBuiltInMaterialRibbonRendererProperties::PostLoad()
{
#if WITH_EDITOR
// Make changes to the MaterialParameters to simplify setup for artists. Ex: Ensure various modules are present and ensure various texture samplers are using the right settings.
...
#endif //WITH_EDITOR
Super::PostLoad();
}
So I think you could test with making a FMaterialUpdateContext which has no flags and pass that into MIC->UpdateStaticPermutation. Niagara will automatically reregister the Niagara components post cache so we don’t need EOptions::RecreateRenderStates, and looking inside UpdateStaticPermutation it generates a context with only that flag (unless I’ve missed something).
The other option would be to pass in a FMaterialUpdateContext from the cache to the renderers so at least there’s a single sync vs one per renderer.
Let me know if that works out as I don’t have time to test it locally right now, but I’ll add it to my things to look at list
Thanks Stu, yeah providing a MaterialUpdateContext without the RecreateRenderStates flag does fix it, so we’ll go with that assuming it won’t have any bad/worse side effects.
I still see about 370ms for the worst case which I’m going to dig into - at first glance I think this might be legit because we have so many objects that use the materials. Either way, this isn’t a deal breaker for PIE.
So I found one that still took almost 800ms which is getting a bit high. I think moving the MaterialUpdateContext into CacheFromCompiledData outside the emitter loop will be worthwhile because we typically have 4-5 emitters. That will touch a lot of files to pass the context around, so I want to make sure there are no issues before I proceed. Stu - do you foresee any problems deferring the MaterialUpdates to the end of UNiagaraSystem::CacheFromCompiledData?
There should be no problem with deferring the updates to the end of the cache. However I think there’s a bit of logic inside FMaterialUpdateContext we could be skipping here, I’m assming it’s going inside bUpdateStaticDrawLists and that is redundant in the case of Niagara. We are pretty much always dynamic draws and our components should already be invalidated as part of this. I’m not sure there’s a good way to avoid it currently however beyond adding a flag, could be worthwhile doing a quick test if you have the time? It will be less effort than having to pass that context around the cache chain
There is a bigger question of why the assets haven’t been saved in a state with the MICs are up to date, this could mean a resave would ‘fix’ it or something is noodling with the flags in a way that the saved MICs are not the correct state.
If you find adding an extra flag works out I can get that into the engine after some negotation with the materials team
I just tried this and it seems to work. Specifically I added a new flag SkipUpdateStaticDrawList to FMaterialUpdateContext::EOptions, and skipped everything in the destructor except ReloadNaniteFixedFunctionBins(). Can we skip that as well? Wondering if I should just make it a general “Do Nothing” flag. Doesn’t matter much - we’re down to worst case 40ms, and typically less than 1ms, now from the original 13s so I’ll take it
Awesome, thanks for testing Chris. Pretty sure we can skip the Nanite part also, but I’ll confirm that with some other folks, when I have a CL I’ll pass it on also.