Resizing Performance Buff Reminder

Open
  • Is something wrong?
#30 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
50/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
lua
Domain
performance

Research direction

Start by reading Resize:_Move and the Window:SetSize path to see where each pointer move triggers a geometry cascade, then follow the Logic subscription that currently calls _Refresh. Run resize_test to see the 22 timing assertions the issue mentions and confirm the proposed next-tick timing by adding a tick after pointer moves. Done means resizing applies once per Logic tick, the 1-move-per-frame case does not regress, the tests reflect the new timing, and the double overlay sync noted in scroll.lua is checked.

Written by the indexing model from the issue text.

Description

enhancement

Resize:_Move applies setRect on every ONHOLD, and Window:SetSize does a full geometry cascade each time. I counted it on your own test harness with a window that has 150 content rows and its 188 geometry resolutions per pointer move, so if the mouse outruns the frame your'e doing more cascades than there are frames to draw them. idle is completely free though, 50 ticks and zero work, so its only during a drag

this stashes the pending rect and lets the Logic tick apply it once. the cost goes flat instead of scaling with how fast you drag, at 3 moves a frame its 11280 resolutions down to 3760 and at 6 its 22560 down to 3760. at 1 move a frame its identical so nothing regresses

fair warning it does change the timing, the resize lands on the next tick instead of straight away, and resize_test asserts the old timing in 22 places. I ran your suite with a tick() after each pointer move and it all passes, so I think the logic is right and only the timing moved, but that's your call not mine

the one thing I couldnt measure is how many ONHOLD events the client actually gives you per logic tick. if its exactly one this buys nothing

@@ -156,21 +156,34 @@
     local dx, dy = mouse.x - drag.mouseX, mouse.y - drag.mouseY
     drag.mouseX, drag.mouseY = mouse.x, mouse.y
     local root, parent, edge = self.root, self.parent, edges[drag.edge]
-    local nextX, width = resizeAxis(root.x, root.width, dx, edge.west, edge.east,
+    -- accumulate here, the Logic tick applies it once
+    local from = drag.pending or root
+    local nextX, width = resizeAxis(from.x, from.width, dx, edge.west, edge.east,
         parent.width, self.minWidth, self.maxWidth)
-    local nextY, height = resizeAxis(root.y, root.height, dy, edge.north, edge.south,
+    local nextY, height = resizeAxis(from.y, from.height, dy, edge.north, edge.south,
         parent.height, self.minHeight, self.maxHeight)
-    if nextX ~= root.x or nextY ~= root.y or width ~= root.width or height ~= root.height then
+    drag.pending = { x = nextX, y = nextY, width = width, height = height }
+    return false
+end
+
+function Resize:_Flush()
+    local drag = self.dragState
+    local rect = drag and drag.pending
+    if not rect then return end
+    drag.pending = nil
+    if not available(self) then return end
+    local root = self.root
+    if rect.x ~= root.x or rect.y ~= root.y or
+        rect.width ~= root.width or rect.height ~= root.height then
         -- Owners may synchronously reflow their content and call Refresh again.
         self.applying = true
-        self.setRect(nextX, nextY, width, height)
+        self.setRect(rect.x, rect.y, rect.width, rect.height)
         self.applying = false
     end
-    self:Refresh()
-    return false
 end
 
 function Resize:Cancel()
+    self:_Flush()
     self:_Clear()
     self:Refresh()
     return false
@@ -234,7 +247,10 @@
     controllers[self] = true
     if wasEmpty then
         Event.Logic.Subscribe(EVENT_ID, function()
-            for controller in pairs(controllers) do controller:_Refresh(false) end
+            for controller in pairs(controllers) do
+                controller:_Flush()
+                controller:_Refresh(false)
+            end
         end)
     end
     self:Refresh()

also noticed SetSize syncs overlays after scroll:Refresh() already did one through SetScrollPosition, so thats two syncs per resize. theres a comment in scroll.lua mentioning it so you probably know already

Dominant language
Lua
Stars
6
Forks
4
Avg merge
2d 7h
Merged PRs (30d)
18

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from PrettyRS/prettyui

All issues in PrettyRS/prettyui

Similar issues

More Lua issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.