Merge branch 'update_setting_pages' into 'master'

Optimize setting group rendering

See merge request OpenMW/openmw!3929

apps/openmw/mwlua/uibindings.cpp

@@ -134,7 +134,10 @@ namespace MWLua
         };
 
         api["updateAll"] = [luaManager = context.mLuaManager, menu]() {
-            LuaUi::Element::forEach(menu, [](LuaUi::Element* e) { e->mUpdate = true; });
+            LuaUi::Element::forEach(menu, [](LuaUi::Element* e) {
+                if (e->mState == LuaUi::Element::Created)
+                    e->mState = LuaUi::Element::Update;
+            });
             luaManager->addAction([menu]() { LuaUi::Element::forEach(menu, [](LuaUi::Element* e) { e->update(); }); },
                 "Update all menu UI elements");
         };
@@ -305,15 +308,15 @@ namespace MWLua
         element["layout"] = sol::property([](const LuaUi::Element& element) { return element.mLayout; },
             [](LuaUi::Element& element, const sol::table& layout) { element.mLayout = layout; });
         element["update"] = [luaManager = context.mLuaManager](const std::shared_ptr<LuaUi::Element>& element) {
-            if (element->mDestroy || element->mUpdate)
+            if (element->mState != LuaUi::Element::Created)
                 return;
-            element->mUpdate = true;
+            element->mState = LuaUi::Element::Update;
             luaManager->addAction([element] { wrapAction(element, [&] { element->update(); }); }, "Update UI");
         };
         element["destroy"] = [luaManager = context.mLuaManager](const std::shared_ptr<LuaUi::Element>& element) {
-            if (element->mDestroy)
+            if (element->mState == LuaUi::Element::Destroyed)
                 return;
-            element->mDestroy = true;
+            element->mState = LuaUi::Element::Destroy;
             luaManager->addAction(
                 [element] { wrapAction(element, [&] { LuaUi::Element::erase(element.get()); }); }, "Destroy UI");
         };

components/lua_ui/element.cpp

@@ -89,12 +89,16 @@ namespace LuaUi
             root->updateCoord();
         }
 
-        WidgetExtension* pluckElementRoot(const sol::object& child)
+        WidgetExtension* pluckElementRoot(const sol::object& child, uint64_t depth)
         {
             std::shared_ptr<Element> element = child.as<std::shared_ptr<Element>>();
-            WidgetExtension* root = element->mRoot;
+            if (element->mState == Element::Destroyed || element->mState == Element::Destroy)
-            if (!root)
                 throw std::logic_error("Using a destroyed element as a layout child");
+            // child Element was created in the same frame and its action hasn't been processed yet
+            if (element->mState == Element::New)
+                element->create(depth + 1);
+            WidgetExtension* root = element->mRoot;
+            assert(root);
             WidgetExtension* parent = root->getParent();
             if (parent)
             {
@@ -107,7 +111,7 @@ namespace LuaUi
             return root;
         }
 
-        WidgetExtension* createWidget(const sol::table& layout, uint64_t depth);
+        WidgetExtension* createWidget(const sol::table& layout, bool isRoot, uint64_t depth);
         void updateWidget(WidgetExtension* ext, const sol::table& layout, uint64_t depth);
 
         std::vector<WidgetExtension*> updateContent(
@@ -130,7 +134,7 @@ namespace LuaUi
                 sol::object child = content.at(i);
                 if (child.is<Element>())
                 {
-                    WidgetExtension* root = pluckElementRoot(child);
+                    WidgetExtension* root = pluckElementRoot(child, depth);
                     if (ext != root)
                         destroyChild(ext);
                     result[i] = root;
@@ -145,7 +149,7 @@ namespace LuaUi
                     else
                     {
                         destroyChild(ext);
-                        ext = createWidget(newLayout, depth);
+                        ext = createWidget(newLayout, false, depth);
                     }
                     result[i] = ext;
                 }
@@ -156,9 +160,9 @@ namespace LuaUi
             {
                 sol::object child = content.at(i);
                 if (child.is<Element>())
-                    result[i] = pluckElementRoot(child);
+                    result[i] = pluckElementRoot(child, depth);
                 else
-                    result[i] = createWidget(child.as<sol::table>(), depth);
+                    result[i] = createWidget(child.as<sol::table>(), false, depth);
             }
             return result;
         }
@@ -191,7 +195,7 @@ namespace LuaUi
             });
         }
 
-        WidgetExtension* createWidget(const sol::table& layout, uint64_t depth)
+        WidgetExtension* createWidget(const sol::table& layout, bool isRoot, uint64_t depth)
         {
             static auto widgetTypeMap = widgetTypeToName();
             std::string type = widgetType(layout);
@@ -205,7 +209,7 @@ namespace LuaUi
             WidgetExtension* ext = dynamic_cast<WidgetExtension*>(widget);
             if (!ext)
                 throw std::runtime_error("Invalid widget!");
-            ext->initialize(layout.lua_state(), widget, depth == 0);
+            ext->initialize(layout.lua_state(), widget, isRoot);
 
             updateWidget(ext, layout, depth);
             return ext;
@@ -247,8 +251,7 @@ namespace LuaUi
         : mRoot(nullptr)
         , mLayout(std::move(layout))
         , mLayer()
-        , mUpdate(false)
+        , mState(Element::New)
-        , mDestroy(false)
     {
     }
 
@@ -267,12 +270,12 @@ namespace LuaUi
         sGameElements.erase(element);
     }
 
-    void Element::create()
+    void Element::create(uint64_t depth)
     {
         assert(!mRoot);
-        if (!mRoot)
+        if (mState == New)
         {
-            mRoot = createWidget(layout(), 0);
+            mRoot = createWidget(layout(), true, depth);
             mLayer = setLayer(mRoot, layout());
             updateRootCoord(mRoot);
         }
@@ -280,15 +283,16 @@ namespace LuaUi
 
     void Element::update()
     {
-        if (mRoot && mUpdate)
+        if (mState == Update)
         {
+            assert(mRoot);
             if (mRoot->widget()->getTypeName() != widgetType(layout()))
             {
                 destroyRoot(mRoot);
                 WidgetExtension* parent = mRoot->getParent();
                 auto children = parent->children();
                 auto it = std::find(children.begin(), children.end(), mRoot);
-                mRoot = createWidget(layout(), 0);
+                mRoot = createWidget(layout(), true, 0);
                 assert(it != children.end());
                 *it = mRoot;
                 parent->setChildren(children);
@@ -301,16 +305,18 @@ namespace LuaUi
             mLayer = setLayer(mRoot, layout());
             updateRootCoord(mRoot);
         }
-        mUpdate = false;
+        mState = Created;
     }
 
     void Element::destroy()
     {
-        if (mRoot)
+        if (mState != Destroyed)
         {
             destroyRoot(mRoot);
             mRoot = nullptr;
+            if (mState != New)
                 mLayout = sol::make_object(mLayout.lua_state(), sol::nil);
+            mState = Destroyed;
         }
     }
 }

components/lua_ui/element.hpp

@@ -21,10 +21,18 @@ namespace LuaUi
         WidgetExtension* mRoot;
         sol::object mLayout;
         std::string mLayer;
-        bool mUpdate;
-        bool mDestroy;
 
-        void create();
+        enum State
+        {
+            New,
+            Created,
+            Update,
+            Destroy,
+            Destroyed,
+        };
+        State mState;
+
+        void create(uint64_t dept = 0);
 
         void update();
 

files/data/scripts/omw/settings/menu.lua

@@ -6,8 +6,11 @@ local core = require('openmw.core')
 local storage = require('openmw.storage')
 local I = require('openmw.interfaces')
 
+local auxUi = require('openmw_aux.ui')
+
 local common = require('scripts.omw.settings.common')
--- :reset on startup instead of :removeOnExit
+common.getSection(false, common.groupSectionKey):setLifeTime(storage.LIFE_TIME.GameSession)
+-- need to :reset() on reloadlua as well as game session end
 common.getSection(false, common.groupSectionKey):reset()
 
 local renderers = {}
@@ -21,6 +24,7 @@ local interfaceL10n = core.l10n('Interface')
 local pages = {}
 local groups = {}
 local pageOptions = {}
+local groupElements = {}
 
 local interval = { template = I.MWUI.templates.interval }
 local growingIntreval = {
@@ -116,6 +120,11 @@ local function renderSetting(group, setting, value, global)
         }
     end
     local argument = common.getArgumentSection(global, group.key):get(setting.key)
+    local ok, rendererResult = pcall(renderFunction, value, set, argument)
+    if not ok then
+        print(string.format('Setting %s renderer "%s" error: %s', setting.key, setting.renderer, rendererResult))
+    end
+
     return {
         name = setting.key,
         type = ui.TYPE.Flex,
@@ -129,7 +138,7 @@ local function renderSetting(group, setting, value, global)
         content = ui.content {
             titleLayout,
             growingIntreval,
-            renderFunction(value, set, argument),
+            ok and rendererResult or {}, -- TODO: display error?
         },
     }
 end
@@ -245,11 +254,13 @@ end
 
 local function generateSearchHints(page)
     local hints = {}
+    do
         local l10n = core.l10n(page.l10n)
         table.insert(hints, l10n(page.name))
         if page.description then
             table.insert(hints, l10n(page.description))
         end
+    end
     local pageGroups = groups[page.key]
     for _, pageGroup in pairs(pageGroups) do
         local group = common.getSection(pageGroup.global, common.groupSectionKey):get(pageGroup.key)
@@ -281,7 +292,15 @@ local function renderPage(page, options)
         if not group then
             error(string.format('%s group "%s" was not found', pageGroup.global and 'Global' or 'Player', pageGroup.key))
         end
-        table.insert(groupLayouts, renderGroup(group, pageGroup.global))
+        local groupElement = groupElements[page.key][group.key]
+        if not groupElement or not groupElement.layout then
+            groupElement = ui.create(renderGroup(group, pageGroup.global))
+        end
+        if groupElement.layout == nil then
+            error(string.format('Destroyed group element for %s %s', page.key, group.key))
+        end
+        groupElements[page.key][group.key] = groupElement
+        table.insert(groupLayouts, groupElement)
     end
     local groupsLayout = {
         name = 'groups',
@@ -329,10 +348,14 @@ local function renderPage(page, options)
             bigSpacer,
         },
     }
-    if options.element then options.element:destroy() end
     options.name = l10n(page.name)
-    options.element = ui.create(layout)
     options.searchHints = generateSearchHints(page)
+    if options.element then
+        options.element.layout = layout
+        options.element:update()
+    else
+        options.element = ui.create(layout)
+    end
 end
 
 local function onSettingChanged(global)
@@ -340,18 +363,23 @@ local function onSettingChanged(global)
         local group = common.getSection(global, common.groupSectionKey):get(groupKey)
         if not group or not pageOptions[group.page] then return end
 
+        local groupElement = groupElements[group.page][group.key]
+
         if not settingKey then
+            if groupElement then
+                groupElement.layout = renderGroup(group)
+                groupElement:update()
+            else
                 renderPage(pages[group.page], pageOptions[group.page])
+            end
             return
         end
 
         local value = common.getSection(global, group.key):get(settingKey)
-        local element = pageOptions[group.page].element
+        local settingsContent = groupElement.layout.content.settings.content
-        local groupsLayout = element.layout.content.groups
+        auxUi.deepDestroy(settingsContent[settingKey]) -- support setting renderers which return UI elements
-        local groupLayout = groupsLayout.content[groupLayoutName(group.key, global)]
+        settingsContent[settingKey] = renderSetting(group, group.settings[settingKey], value)
-        local settingsContent = groupLayout.content.settings.content
+        groupElement:update()
-        settingsContent[settingKey] = renderSetting(group, group.settings[settingKey], value, global)
-        element:update()
     end)
 end
 
@@ -360,6 +388,8 @@ local function onGroupRegistered(global, key)
     if not group then return end
 
     groups[group.page] = groups[group.page] or {}
+    groupElements[group.page] = groupElements[group.page] or {}
+
     local pageGroup = {
         key = group.key,
         global = global,
@@ -376,11 +406,9 @@ local function onGroupRegistered(global, key)
 
             local value = common.getSection(global, group.key):get(settingKey)
 
-            local element = pageOptions[group.page].element
+            local element = groupElements[group.page][group.key]
-            local groupsLayout = element.layout.content.groups
+            local settingsContent = element.layout.content.settings.content
-            local groupLayout = groupsLayout.content[groupLayoutName(group.key, global)]
+            settingsContent[settingKey] = renderSetting(group, group.settings[settingKey], value)
-            local settingsContent = groupLayout.content.settings.content
-            settingsContent[settingKey] = renderSetting(group, group.settings[settingKey], value, global)
             element:update()
         end))
     end
@@ -418,6 +446,11 @@ local function resetPlayerGroups()
     for pageKey, page in pairs(groups) do
         for groupKey in pairs(page) do
             if not menuGroups[groupKey] then
+                if groupElements[pageKey][groupKey] then
+                    groupElements[pageKey][groupKey]:destroy()
+                    print(string.format('destroyed group element %s %s', pageKey, groupKey))
+                    groupElements[pageKey][groupKey] = nil
+                end
                 page[groupKey] = nil
                 playerGroupsSection:set(groupKey, nil)
             end
@@ -426,7 +459,8 @@ local function resetPlayerGroups()
         if options then
             if not menuPages[pageKey] then
                 if options.element then
-                    options.element:destroy()
+                    auxUi.deepDestroy(options.element)
+                    options.element = nil
                 end
                 ui.removeSettingsPage(options)
                 pageOptions[pageKey] = nil
@@ -461,9 +495,6 @@ local function registerPage(options)
     }
     pages[page.key] = page
     groups[page.key] = groups[page.key] or {}
-    if pageOptions[page.key] then
-        pageOptions[page.key].element:destroy()
-    end
     pageOptions[page.key] = pageOptions[page.key] or {}
     renderPage(page, pageOptions[page.key])
     ui.registerSettingsPage(pageOptions[page.key])