perf(ui): stop writing into the caller's spec to build a node

A table constructor sizes its hash part to exactly the keys given, so every
field the wrapper added afterwards could rehash the spec -- twice for the
small ones. type and interactive are now arguments, which also keeps the
tree ignorant of what names handlers go by, and ui.label no longer writes w
and h: tree.create already measures the same text, font and style to size a
text node, so Lua was measuring the string twice to say what C derives.

Press styles are keyed by exception rather than one entry per node, which on
a full screen was an array part recording that almost nothing opts out.

Measured in the emulator, 12 sensors / 131 nodes, against the previous
commit: build 74ms -> 52ms, Lua transient +12.0kB -> +5.7kB, free heap at
build end 27936 -> 36140. Tree footprint is byte-identical throughout.

Two behaviour changes fall out. ui.label used to overwrite an explicit w
with the measured text width, so optionlist's fixed 10px marker gutter was
silently variable and is now the width it asks for. tree.create measured a
label in styleNormal while the painter draws it in the node's textStyle, so
a styled label was sized too narrow; it now measures in the style it paints.

lua/lib/ui.lua

@@ -51,8 +51,10 @@ local THEMES = {
 
 local enterHandlers, exitHandlers, clickHandlers, painters = {}, {}, {}, {}
 -- A widget that paints its own press feedback opts out, so pressing one key does not
--- repaint the whole node the way a pressed style would.
-local pressStyles = {}
+-- repaint the whole node the way a pressed style would. Only the opt-outs are stored:
+-- keyed the other way this held one entry per node, which on a full screen is a hash
+-- part of several kB recording that almost nothing is an exception.
+local noPressStyle = {}
 local laidOut = false
 local themeName
 -- One tree per state, built by the function ui.mount() was given. Rebuilding is
@@ -150,7 +152,7 @@ loadTheme(screen.getTheme())
 
 local function clearState()
   enterHandlers, exitHandlers, clickHandlers, painters = {}, {}, {}, {}
-  pressStyles = {}
+  noPressStyle = {}
 end
 
 -- The clip is the slice of the node being repainted now, which for a composited repaint is
@@ -171,12 +173,12 @@ local function build(spec, kind)
     error("build nodes from the function ui.mount() was given, then ui.rebuild()", 3)
   end
 
-  spec.type = kind
-  spec.interactive = spec.on_enter ~= nil or spec.on_exit ~= nil or spec.on_click ~= nil
-  -- Children come straight off the spec's array part. tree.create reads only named
-  -- fields, so copying them into a second table cost one extra Lua table per node
-  -- -- roughly half again the spec's own cost, on the build path that peaks the heap.
-  local id = tree.create(nil, spec)
+  -- Nothing is written into the spec: a table constructor sizes its hash part to
+  -- exactly the keys given, so adding type and interactive rehashed most specs --
+  -- twice for the small ones -- on the build path that peaks the heap. Children
+  -- come straight off the array part, which tree.create never reads.
+  local id = tree.create(nil, spec, kind,
+    spec.on_enter ~= nil or spec.on_exit ~= nil or spec.on_click ~= nil)
   for _, child in ipairs(spec) do
     tree.attach(id, child)
   end
@@ -185,7 +187,9 @@ local function build(spec, kind)
   exitHandlers[id] = spec.on_exit
   clickHandlers[id] = spec.on_click
   painters[id] = spec.paint
-  pressStyles[id] = spec.press_style ~= false
+  if spec.press_style == false then
+    noPressStyle[id] = true
+  end
   if spec.scrollX or spec.scrollY then
     scrollNodes[id] = { x = spec.scrollX == true, y = spec.scrollY == true }
     scrollState[id] = { sx = 0, sy = 0, vx = 0, vy = 0 }
@@ -252,16 +256,20 @@ end
 ---@return NodeId
 function ui.label(text, spec)
   spec = spec or {}
-  local font, style = spec.font or screen.FONT_UI, spec.style or screen.STYLE_NORMAL
-  if spec.fit and screen.getTextWidth(font, text, style) > spec.fit then
-    while #text > 1 and screen.getTextWidth(font, text .. "~", style) > spec.fit do
-      text = text:sub(1, -2)
+  -- Sized by the intrinsic width tree.create measures from the same text, font and
+  -- style, so this measures only when the text has to be truncated. Writing w and h
+  -- here measured the string a second time and grew the spec by two keys, which on a
+  -- one-key spec is two rehashes.
+  if spec.fit then
+    local font = spec.font or screen.FONT_UI
+    local style = spec.style or screen.STYLE_NORMAL
+    if screen.getTextWidth(font, text, style) > spec.fit then
+      while #text > 1 and screen.getTextWidth(font, text .. "~", style) > spec.fit do
+        text = text:sub(1, -2)
+      end
+      text = text .. "~"
     end
-    text = text .. "~"
   end
-  spec.w = screen.getTextWidth(font, text, style)
-  spec.h = screen.getFontHeight(font, style)
-  spec.font, spec.fit = font, nil
   return ui.text(text, spec)
 end
 
@@ -275,7 +283,7 @@ function ui.button(spec)
   spec.label = nil
   local id = build(spec, "button")
   if label then
-    tree.create(id, { type = "text", label = label, font = font or screen.FONT_UI })
+    tree.create(id, { label = label, font = font or screen.FONT_UI }, "text")
   end
   return id
 end
@@ -503,7 +511,7 @@ local function inside(id, x, y)
 end
 
 local function enter(id, x, y)
-  if pressStyles[id] then
+  if not noPressStyle[id] then
     tree.setPressed(id, true)
   end
   local handler = enterHandlers[id]
@@ -513,7 +521,7 @@ local function enter(id, x, y)
 end
 
 local function exit(id, x, y)
-  if pressStyles[id] then
+  if not noPressStyle[id] then
     tree.setPressed(id, false)
   end
   local handler = exitHandlers[id]

lua/test/ui.lua

@@ -71,18 +71,18 @@ tree = {
   reset = function()
     nodes, focus, buttonCount = {}, nil, 0
   end,
-  create = function(parent, spec)
+  create = function(parent, spec, kind, interactive)
     local id = #nodes + 1
     local x = 0
-    if spec.type == "button" then
+    if kind == "button" then
       buttonCount = buttonCount + 1
       x = (buttonCount - 1) * 100
     end
     nodes[id] = {
       parent = parent,
-      type = spec.type,
+      type = kind,
       label = spec.label,
-      interactive = spec.interactive,
+      interactive = interactive or false,
       rect = { x, 0, 90, 50 },
       pressed = false,
       style = {},

native/src/bindings/features/screen/tree.cpp

@@ -121,9 +121,12 @@ ui::Align readAlign(lua_State* state, int index, const char* key) {
   return align;
 }
 
+// Taken as arguments rather than spec fields: writing type and interactive into
+// the caller's table rehashed it, because a constructor sizes the hash part to
+// exactly the keys written and two more keys overflow it. Interactive is the
+// caller's to decide, so the tree stays ignorant of what names handlers go by.
 uint8_t readType(lua_State* state, int index) {
-  lua_getfield(state, index, "type");
-  const char* value = luaL_checkstring(state, -1);
+  const char* value = luaL_checkstring(state, index);
   uint8_t type = ui::BOX;
   if (strcmp(value, "text") == 0) {
     type = ui::TEXT;
@@ -132,10 +135,8 @@ uint8_t readType(lua_State* state, int index) {
   } else if (strcmp(value, "custom") == 0) {
     type = ui::CUSTOM;
   } else if (strcmp(value, "box") != 0) {
-    lua_pop(state, 1);
     luaL_error(state, "unknown node type '%s'", value);
   }
-  lua_pop(state, 1);
   return type;
 }
 
@@ -193,7 +194,7 @@ int create(lua_State* state) {
   const uint16_t parent = hasParent ? checkNode(state, 1) : ui::NONE;
   luaL_checktype(state, 2, LUA_TTABLE);
 
-  const uint8_t type = readType(state, 2);
+  const uint8_t type = readType(state, 3);
   ui::Spec spec;
   bool present = false;
   spec.w = readSize(state, 2, "w", present);
@@ -216,7 +217,7 @@ int create(lua_State* state) {
   uint8_t flags = 0;
   if (readFlag(state, 2, "row"))
     flags |= ui::ROW;
-  if (readFlag(state, 2, "interactive"))
+  if (lua_toboolean(state, 4))
     flags |= ui::INTERACTIVE;
   if (readFlag(state, 2, "scrollX"))
     flags |= ui::SCROLL_X;
@@ -234,7 +235,13 @@ int create(lua_State* state) {
       lua_isnoneornil(state, -1) ? nullptr : luaL_checkstring(state, -1);
   if (label && type == ui::TEXT) {
     GuiProvider& gui = Runtime::from(state)->gui();
-    const int32_t style = gui.fonts().styleNormal;
+    // Measured with the style it will be painted in, not the normal one: a bold
+    // label is wider than its own box otherwise.
+    lua_getfield(state, 2, "textStyle");
+    const int32_t style = lua_isnoneornil(state, -1)
+                              ? gui.fonts().styleNormal
+                              : static_cast<int32_t>(luaL_checkinteger(state, -1));
+    lua_pop(state, 1);
     spec.intrinsicW = static_cast<int16_t>(gui.textWidth(font, label, style));
     spec.intrinsicH = static_cast<int16_t>(gui.fontHeight(font, style));
   }

native/test/runtime_test.cpp

@@ -164,11 +164,10 @@ int main() {
   bench.gui.trace.clear();
   run(state,
       "tree.reset()\n"
-      "local root = tree.create(nil, {type = 'box', w = 'fill', h = 'fill', "
-      "pad = 4, gap = 2})\n"
-      "local label = tree.create(root, {type = 'text', label = 'Hello'})\n"
-      "local button = tree.create(root, {type = 'button', h = 40, interactive "
-      "= true})\n"
+      "local root = tree.create(nil, {w = 'fill', h = 'fill', "
+      "pad = 4, gap = 2}, 'box')\n"
+      "local label = tree.create(root, {label = 'Hello'}, 'text')\n"
+      "local button = tree.create(root, {h = 40}, 'button', true)\n"
       "tree.setStyle(root, {background = 0xFFFFFF, fill = 0xFFFFFF, color = 0, "
       "face = 0xEEEEEE,\n"
       "  border = 0x333333, focusColor = 0xFF0000})\n"
@@ -191,12 +190,12 @@ int main() {
   // The bordered box rounds its corners; the button fills without one.
   assert(bench.gui.trace.find("roundRect(fill,border);") != std::string::npos);
   assert(bench.gui.trace.find("roundRect(fill,-);") != std::string::npos);
-  expectError(state, "tree.create(nil, {type = 'nope'})");
+  expectError(state, "tree.create(nil, {}, 'nope')");
 
   run(state,
       "tree.reset()\n"
-      "local root = tree.create(nil, {type = 'custom', w = 'fill', h = "
-      "'fill'})\n"
+      "local root = tree.create(nil, {w = 'fill', h = "
+      "'fill'}, 'custom')\n"
       "painted = 0\n"
       "tree.setPainter(function(id, x, y, w, h) painted = painted + w end)\n"
       "assert(tree.layout(root, 0, 0, 320, 240))\n"
@@ -209,11 +208,11 @@ int main() {
   bench.gui.trace.clear();
   run(state,
       "tree.reset()\n"
-      "local list = tree.create(nil, {type = 'box', w = 'fill', h = 'fill',\n"
-      "  scrollX = true, scrollY = true})\n"
+      "local list = tree.create(nil, {w = 'fill', h = 'fill',\n"
+      "  scrollX = true, scrollY = true}, 'box')\n"
       "local rows = {}\n"
-      "for i = 1, 5 do rows[i] = tree.create(list, {type = 'box', w = 400, h = "
-      "100, interactive = true}) end\n"
+      "for i = 1, 5 do rows[i] = tree.create(list, {w = 400, h = "
+      "100}, 'box', true) end\n"
       "tree.setStyle(list, {background = 0xFFFFFF, fill = 0xFFFFFF, border = "
       "0x333333})\n"
       // The box is the panel; the content is deliberately larger on both axes.
@@ -234,8 +233,8 @@ int main() {
       "assert(tree.hit(list, 10, 10) == rows[2], 'scrolled-out row is not hit')\n"
       // Hit is pure geometry: the deepest node covering the point, else the container. A
       // scroll pan finds its box by bubbling up from here, not by a flag on hit.
-      "local bare = tree.create(nil, {type = 'box', w = 'fill', h = 'fill', scrollY = true})\n"
-      "local child = tree.create(bare, {type = 'box', w = 100, h = 900})\n"
+      "local bare = tree.create(nil, {w = 'fill', h = 'fill', scrollY = true}, 'box')\n"
+      "local child = tree.create(bare, {w = 100, h = 900}, 'box')\n"
       "assert(tree.layout(bare, 0, 0, 320, 240))\n"
       "assert(tree.hit(bare, 10, 10) == child, 'deepest node by geometry wins')\n"
       "assert(tree.hit(bare, 200, 10) == bare, 'the container answers where no child covers it')\n"
@@ -256,9 +255,9 @@ int main() {
 
   // A scrolled axis has no size to hand down, so the box needs one of its own.
   expectError(state, "tree.reset()\n"
-                     "local l = tree.create(nil, {type = 'box', scrollY = "
-                     "true})\n"
-                     "tree.create(l, {type = 'box', w = 10, h = 10})\n"
+                     "local l = tree.create(nil, {scrollY = "
+                     "true}, 'box')\n"
+                     "tree.create(l, {w = 10, h = 10}, 'box')\n"
                      "assert(tree.layout(l, 0, 0, 320, 240))");
 
   // A timer firing inside draw is still one batch.