microsoft / MixedRealityToolkit-Unity

This repository is for the legacy Mixed Reality Toolkit (MRTK) v2. For the latest version of the MRTK please visit https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity
https://aka.ms/mrtkdocs
MIT License
6k stars 2.12k forks source link

MixedRealityCanvasInspector crash #8143

Closed cellarmation closed 3 years ago

cellarmation commented 4 years ago

Describe the bug

MRTK includes a custom inspector for the inbuilt canvas component. This can crash in some situations stopping the rendering of the canvas editor UI before it is complete making it unuseable. This seems to occur when there are orphened TMP_SubMeshUI components under the canvas. This exception is also shown in the console whenever you try to look at the canvas component:

ArgumentNullException: Value cannot be null. Parameter name: source UnityEngine.Material..ctor (UnityEngine.Material source) (at C:/buildslave/unity/build/Runtime/Export/Shader.bindings.cs:109) TMPro.TMP_SubMeshUI.CreateMaterialInstance (UnityEngine.Material source) (at Library/PackageCache/com.unity.textmeshpro@1.4.1/Scripts/Runtime/TMP_SubMeshUI.cs:758) TMPro.TMP_SubMeshUI.GetMaterial (UnityEngine.Material mat) (at Library/PackageCache/com.unity.textmeshpro@1.4.1/Scripts/Runtime/TMP_SubMeshUI.cs:737) TMPro.TMP_SubMeshUI.get_material () (at Library/PackageCache/com.unity.textmeshpro@1.4.1/Scripts/Runtime/TMP_SubMeshUI.cs:59) Microsoft.MixedReality.Toolkit.Input.MixedRealityCanvasInspector.GetGraphicsWhichRequireScaleMeshEffect (UnityEngine.Object[] targets) (at Assets/MRTK/Services/InputSystem/Editor/MixedRealityCanvasInspector.cs:171) Microsoft.MixedReality.Toolkit.Input.MixedRealityCanvasInspector.OnInspectorGUI () (at Assets/MRTK/Services/InputSystem/Editor/MixedRealityCanvasInspector.cs:63) UnityEditor.InspectorWindow.DoOnInspectorGUI (System.Boolean rebuildOptimizedGUIBlock, UnityEditor.Editor editor, System.Boolean wasVisible, UnityEngine.Rect& contentRect) (at C:/buildslave/unity/build/Editor/Mono/Inspector/InspectorWindow.cs:1647) UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr) (at C:/buildslave/unity/build/Modules/IMGUI/GUIUtility.cs:179)

To reproduce

Steps to reproduce the behavior:

1) Create a new blank Unity project. 2) Import Microsoft.MixedReality.Toolkit.Unity.Foundation.2.4.0.unitypackage (don't bother doing any of the settings stuff). 3) Import TextMeshPro using Window -> TextMeshPro -> Import TMP Pro Essential Resources. 4) Right click in the scene and select UI > Text - TextMeshPro, this creates a TextMeshProUGUI component on an object as a child of an object with a canvas. 5) Set the font asset to the MRTK provided font "segoeui". 6) Set the text of the TextMeshProUGUI component to be something that uses a character not in the font asset e.g. "©". 7) Delete the TextMeshProUGUI component. 8) Click onto the "Canvas" GameObject that has the Canvas component on it. 9) The MRTK provided custom editor for the Canvas component is now broken and an exception of "ArgumentNullException: Value cannot be null." is generated.

Expected behavior

The canvas editor should be useable in this situation, just like it would be if MRTK wasn't included in the project.

Screenshots

A canvas component without a TMP_SubMeshUI under it: MixedRealityCanvasInspectorCrash

A canvas component with a TMP_SubMeshUI under it: MixedRealityCanvasInspectorCrash2

Your setup (please complete the following information)

Target platform (please complete the following information)

Additional context

This appears to be due to MRTK using a custom inspector for Canvas components. This is done by MixedRealityCanvasInspector.cs, which uses the method GetGraphicsWhichRequireScaleMeshEffect() to get all the components in the children looking for Graphic components and accesses the material property of the graphics. It seems that TextMeshPro generated TMP_SubMeshUI component that are orphened fire this exception if you try to use its material property. This causes the custom inspector to crash half way through drawing the UI making it unusable. If there was no custom inspector the canvas component would still be usable in this situation. I guess the root of this bug is in TextMeshPro but I am reporting it here as the custom inspector probably needs to be more defensive, this is a relatively easy state for someone to get into and would not normally fail so badly.

cellarmation commented 4 years ago

I have created a thread on the Unity forums about the underlying TextMeshPro bug.

keveleigh commented 4 years ago

I wonder if using .sharedMaterial here will fix this, since it shouldn't try to create a new material, which is what appears to be the underlying issue.

keveleigh commented 4 years ago

My previous suggestion of .sharedMaterial won't work, because Graphics don't actually have that!

Otherwise, I haven't been able to repro this anymore, after the latest updates to the TMPro package. Are you still seeing it @cellarmation?

polar-kev commented 3 years ago

Hey @cellarmation, we haven't been able to reproduce this so I'm going to close this issue.