Merge pull request #2019 from LarryThiessen/fix-2018-pool-reset-styling

#2018 Reset styling, subframe art and frame flags on widget reuse

Author: TimothyLuke

GSE_GUI/Editor.lua

@@ -607,6 +607,20 @@ local MACRO_BOX_CHROME_SLACK = 6
 local MACRO_BOX_MIN_INNER = 40
 -- Rows -> pixels in the box's OWN font: a row is the measured line height, and
 -- every row after the first also carries the editbox's line spacing.
+-- Height meters, keyed by the box's frame rather than stored ON it.
+--
+-- A WoW FontString cannot be destroyed, so the meter has to be created once per
+-- frame and found again on reuse. It lived on the widget table first -- #2014's
+-- pool strips post-construction keys, so that leaked one FontString per reuse --
+-- and then on the frame, which this branch's reset now sweeps too, which would
+-- leak it again exactly the same way. Neither table is a safe home, so it lives
+-- here. Weak keys: the frame is the only thing that should keep an entry alive.
+--
+-- Left as a warning for the next caller that caches a region on a widget or its
+-- frame: the pool is now entitled to remove it, and the failure is silent and
+-- cumulative -- an orphan FontString per reuse, each one another entry in the
+-- GetRegions() walk that the reset itself runs on every reuse.
+local macroBoxHeightMeters = setmetatable({}, {__mode = "k"})
 local function MacroBoxRowsHeight(rows, oneRow, spacing)
     return rows * oneRow + math.max(rows - 1, 0) * spacing
 end
@@ -696,18 +710,11 @@ local function FitMacroEditBoxToContent(macroEditBox, text)
     -- MEASURE the rendered text height with a hidden FontString in the box's
     -- own font (wraps included) instead of estimating rows x font size --
     -- estimates drifted by about a row and showed a spare empty line.
-    -- On the FRAME, not the widget table. Since #2014 MultiLineEditBox is a
-    -- pooled type, and resetForReuse strips every key added after construction
-    -- -- including this one. A FontString cannot be destroyed, so caching it on
-    -- the widget meant a fresh one on every reuse, piling up hidden regions on a
-    -- frame that is reused forever. That also slows the pool down: its reset
-    -- walks {frame:GetRegions()} each time. The frame object survives reuse
-    -- unchanged, so the meter parked on it is found again.
-    local meter = macroEditBox.frame.gseHeightMeter
+    local meter = macroBoxHeightMeters[macroEditBox.frame]
     if not meter then
         meter = macroEditBox.frame:CreateFontString(nil, "ARTWORK")
         meter:Hide()
-        macroEditBox.frame.gseHeightMeter = meter
+        macroBoxHeightMeters[macroEditBox.frame] = meter
     end
     local fontPath, fontSize, fontFlags = eb:GetFont()
     if fontPath then meter:SetFont(fontPath, fontSize or 14, fontFlags or "") end

GSE_GUI/NativeUI.lua

@@ -5891,23 +5891,54 @@ local function snapshotPristine(widget)
             scripts[#scripts + 1] = { v, rec }
         end
     end
-    -- Callers also hang REGIONS (FontStrings, Textures) and child FRAMES
-    -- directly off the widget's frame (frame:CreateFontString, CreateFrame
-    -- with the frame as parent). Those are invisible to the key sweep -- they
-    -- live on the frame, not in the widget table -- and a reused widget would
-    -- keep rendering them (e.g. a dependency header bleeding into a macro
-    -- block). Record what the frame owned at construction; anything else gets
-    -- hidden on reuse.
-    local owned = {}
-    for _, region in ipairs({ widget.frame:GetRegions() }) do owned[region] = true end
-    for _, child in ipairs({ widget.frame:GetChildren() }) do owned[child] = true end
-    widget.__gsePristineOwned = owned
+    -- Callers also hang REGIONS (FontStrings, Textures), child FRAMES and
+    -- plain FLAGS directly off the widget's frames -- the TOP frame and its
+    -- exposed subframes alike (frame:CreateFontString, CreateFrame with the
+    -- frame as parent, editBox.GSEMacroEditorColoring = true...). All of that
+    -- is invisible to the widget-table key sweep and survives into the next
+    -- life: dependency headers bled into macro blocks, block rail art bled
+    -- into the Config panel, and the macro-colouring flag turned the NOTES
+    -- box into a macro editor. Record, for every frame the widget exposes,
+    -- its construction-time key set, regions and children; on reuse, sweep
+    -- added keys and hide stranger regions/children.
+    local frames = {}
+    for _, v in pairs(widget) do
+        if type(v) == "table" and v.GetScript and v.SetScript and v.CreateFontString and not frames[v] then
+            local fkeys = {}
+            for k in pairs(v) do fkeys[k] = true end
+            local owned = {}
+            for _, region in ipairs({ v:GetRegions() }) do owned[region] = true end
+            for _, child in ipairs({ v:GetChildren() }) do owned[child] = true end
+            frames[v] = { keys = fkeys, owned = owned }
+        end
+    end
+    widget.__gsePristineFrames = frames
+    -- Callers also restyle the widget's FontStrings (class colours, heading
+    -- fonts, justification). Record each construction-time text region's font,
+    -- colour and justify so a reused label does not wear its previous life's
+    -- styling.
+    local texts = {}
+    for _, v in pairs(widget) do
+        -- FontStrings only: they carry SetFont/SetText but -- unlike Frames and
+        -- EditBoxes, which also have font APIs -- cannot CreateFontString.
+        -- (Filtering on "no SetScript" was wrong: FontStrings are ScriptRegions
+        -- and DO have SetScript, so nothing was ever captured.)
+        if type(v) == "table" and v.GetFont and v.SetText and not v.CreateFontString and not texts[v] then
+            texts[v] = {
+                font = { v:GetFont() },
+                color = { v:GetTextColor() },
+                justifyH = v.GetJustifyH and v:GetJustifyH() or nil,
+                justifyV = v.GetJustifyV and v:GetJustifyV() or nil,
+            }
+        end
+    end
+    widget.__gsePristineTexts = texts
     widget.__gsePristineKeys = keys
     widget.__gsePristineScripts = scripts
     widget.__gsePristineW, widget.__gsePristineH = widget.frame:GetSize()
     keys.__gsePristineKeys, keys.__gsePristineScripts = true, true
     keys.__gsePristineW, keys.__gsePristineH = true, true
-    keys.__gsePristineOwned = true
+    keys.__gsePristineFrames, keys.__gsePristineTexts = true, true
 end
 
 local function resetForReuse(widget)
@@ -5924,13 +5955,17 @@ local function resetForReuse(widget)
         end
     end
     local frame = widget.frame
-    local owned = widget.__gsePristineOwned
-    if owned then
-        for _, region in ipairs({ frame:GetRegions() }) do
-            if not owned[region] then region:Hide() end
+    for subframe, rec in pairs(widget.__gsePristineFrames or {}) do
+        -- Sweep caller-added fields off the frame itself ([0] is the engine
+        -- userdata; construction-time keys stay).
+        for k in pairs(subframe) do
+            if not rec.keys[k] and k ~= 0 then subframe[k] = nil end
         end
-        for _, child in ipairs({ frame:GetChildren() }) do
-            if not owned[child] then child:Hide() end
+        for _, region in ipairs({ subframe:GetRegions() }) do
+            if not rec.owned[region] then region:Hide() end
+        end
+        for _, child in ipairs({ subframe:GetChildren() }) do
+            if not rec.owned[child] then child:Hide() end
         end
     end
     frame:Hide()
@@ -5947,6 +5982,12 @@ local function resetForReuse(widget)
     if widget.text and widget.text ~= widget.label and widget.text.SetText then
         pcall(widget.text.SetText, widget.text, "")
     end
+    for fontString, style in pairs(widget.__gsePristineTexts or {}) do
+        if style.font[1] then pcall(fontString.SetFont, fontString, unpack(style.font)) end
+        pcall(fontString.SetTextColor, fontString, unpack(style.color))
+        if style.justifyH then pcall(fontString.SetJustifyH, fontString, style.justifyH) end
+        if style.justifyV then pcall(fontString.SetJustifyV, fontString, style.justifyV) end
+    end
 end
 
 function UI:Create(typeName)