Surface ReflectionTypeLoadExceptions in HDUtils.GetRenderPipelineMaterialList() #5474
Conversation
…rialList(). Without surfacing these exceptions, developers cannot act on any underlying reflection errors in the HDRP assembly.
|
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. HDRP Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
|
Adding Remi to get his though. Note that we can have Material define outside of HDRP package (it is the case for automotive), thus why we rely on the reflection mechanism. |
I agree. We cannot do it with hard coded list. But perhaps we can do it more efficiently using the TypeCache instead of C# reflection. Other than that, Yes we want to throw error as explicite as possible for user creating its own material know what is the issue. |
RSlysz
left a comment
There was a problem hiding this comment.
Proposed version is better than what we currently have.
But we should plan some improvement on it later:
- Is there a similar mechanisme in URP? If yes, we should handle this in CoreSRP directly.
- Using TypeCache will only gather type that are already loaded. Thus it already handled the load exception. It would also be more efficient this way than using reflection.
Nope |
…rialList(). Without surfacing these exceptions, developers cannot act on any underlying reflection errors in the HDRP assembly. (#5474)
Without surfacing these exceptions, developers cannot act on any underlying reflection errors in the HDRP assembly.
https://unity.slack.com/archives/CE7DZN2H1/p1630106304084900
An example error message that this will now generate (from the case I hit while developing, which previously silently failed):
Encountered an exception while attempting to reflect the HDRP assembly to extract all RenderPipelineMaterial types. This exception must be fixed in order to fully initialize HDRP correctly. Generic class cannot have explicit layout. UnityEngine.Rendering.HighDefinition.AtlasAllocatorDynamic1/AtlasNode`To be clear - the example error will not fire on master - that is just an example from code changes I was making on a branch.
Outside of the scope of this PR: I wonder if it would be better to not rely on reflection here, and instead just manually populate the list as mentioned in the comment with all RenderPipelineMaterial types - the list is fairly short, and I would expect this reflection the be the cause of maybe > 1 second during initialization time.