Mixedrealitytoolkit-unity: Proposal: performance optimization and refactor of InteractiveThemeWidget

[ivan2007]

Overview

Themes are a good thing for unify the UI aspect of the application.
If you create many UI component at runtime there is a performance issue related to the function FindObjectsOfType that can be very slow.
It would be nice if we can implement a cache for the application themes.

Expected Behavior

No performance issue with complex scene and UI runtime generation

I have implemented this in my app and doing so, I saw that we can do also a refactor of the class.
This is my implementation:
`

    /// <summary>
    /// Find a ColorInteractiveTheme by tag
    /// </summary>
    /// <param name="tag"></param>
    /// <returns></returns>
    public ColorInteractiveTheme GetColorTheme(string tag)
    {
        return GetCachedTheme<ColorInteractiveTheme, Color>(tag);
    }

    /// <summary>
    /// Find a Vector3InteractiveTheme by tag
    /// </summary>
    /// <param name="tag"></param>
    /// <returns></returns>
    public Vector3InteractiveTheme GetVector3Theme(string tag)
    {
        return GetCachedTheme<Vector3InteractiveTheme, Vector3>(tag);
    }

    /// <summary>
    /// Find a TextureInteractiveTheme by tag
    /// </summary>
    /// <param name="tag"></param>
    /// <returns></returns>
    public TextureInteractiveTheme GetTextureTheme(string tag)
    {
        return GetCachedTheme<TextureInteractiveTheme, Texture>(tag);
    }

    private static ConcurrentDictionary<string, MonoBehaviour> _themeCache = new ConcurrentDictionary<string, MonoBehaviour>();

    private Theme GetCachedTheme<Theme, ThemeType>(string themeTag) where Theme : InteractiveTheme<ThemeType>
    {
        Theme result = default(Theme);
        MonoBehaviour cachedTheme;
        var themeKey = typeof(Theme).Name + "_" + themeTag;

        if (_themeCache.TryGetValue(themeKey, out cachedTheme))
        {
            result = (Theme)cachedTheme;
        }
        else
        {
            result = GetTheme<Theme, ThemeType>(themeTag);

            if (result != null)
            {
                _themeCache.TryAdd(themeKey, result);
            }
        }

        if (!mThemeUpdated) mThemeUpdated = result != null;

        return result;
    }

    public Theme GetTheme<Theme, ThemeType>(string themeTag) where Theme : InteractiveTheme<ThemeType>
    {
        // Search locally
        Theme[] textureThemes = InteractiveHost.GetComponentsInChildren<Theme>();
        Theme theme = FindTheme<Theme, ThemeType>(textureThemes, themeTag);

        // Search globally
        if (theme == null)
        {
            textureThemes = FindObjectsOfType<Theme>();
            theme = FindTheme<Theme, ThemeType>(textureThemes, themeTag);
        }

        return theme;
    }

    public Theme FindTheme<Theme, ThemeType>(Theme[] textureThemes, string themeTag) where Theme : InteractiveTheme<ThemeType>
    {
        for (int i = 0; i < textureThemes.Length; ++i)
        {
            if (textureThemes[i].Tag == themeTag)
            {
                return textureThemes[i];
            }
        }

        return null;
    }
}

`
InteractiveThemeWidget.zip

Unity Editor Version

2017.4.6f1

Mixed Reality Toolkit Release Version

2017.4.0.0

[StephenHodgson]

We're currently not taking large refactoring against the master branch while vNEXT is in development.

But if you'd like to propose this as a vNEXT thing then cool. 😀

For the most part we strictly avoid using FindObjectsOfType and use a very good pattern that the uGUI system implements with interfaces, which might be a great time into how to implement something similar with a theme profile using a scriptable object.

[cre8ivepark]

@killerantz for visibility

[killerantz]

That's a good option. I have a newer version of themes and widgets that uses static dictionaries to cache the themes and themes add themselves to the dictionaries on enable. The widgets work a little differently too, if you are familiar with how they work now, the newer widgets look for the themes and their Interactive hosts on Update so things are more modular. Widgets look at the dictionaries for their themes, so no more searching the scene during runtime.

We are working on some themes for vNEXT that are more integrated so we avoid the searching in the future as well.

[Yoyozilla]

We have done some perf work on MRTK v2, could you please take a look and reopen issues against v2?