Settle the layout before painting it, not after
Every change the client made painted one frame of the old layout under the new contents, and one screen never stopped moving at all. Three separate reasons, all here rather than in the client. A container is as big as what its children asked for LAST frame, so the first walk after the reconciler patches the tree knows the new children and the old sizes. `frame!` would have caught that — it settles before it draws — except that it skips the settling passes while nothing is moving, which is almost every frame, and that test looks at what the last walk did. A patch arrives between two walks and is invisible to it until the walk that paints. So the backend says what it did: `unsettle!`, from the `:before` hook, which is handed the context now for that purpose. The frame that would have shown the old layout spends itself settling instead. Three settling passes were not enough either. A size travels up the tree in one pass and back down in the next, so a change deep inside a screen — the reader's list inside a split inside a page — was still moving on the third and settled on the fourth, one pass after it was painted. Eight, and only the frames that are actually moving pay for them. And `:max-width` on a page was a floor rather than a ceiling: the column reported whatever its children asked for, which made its width a function of its own width. One thing too wide to wrap — a URL, a row of buttons — dragged the column past the cap and took every paragraph on the page out with it, and where the sums did not come to rest the column swung between two widths, one per frame, for as long as the screen was up. The settings screen did exactly that. `:fixed` there, and the width is the cap's and the window's; the wrapping under it has something that holds still to wrap against. Measured by walking frq's own tree headlessly and asking, per frame, whether what was painted was still moving. Before: one to three such frames on every screen change, message, banner and panel, and settings never settling. After: none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
51145df parent: b44d525 modified
glimmer-backends/glimmer-jvui/src/glimmer_jvui/core.clj +24 -4 | @@ -456,10 +456,30 @@ | ||
| 456 | 456 | (swap! timers dissoc id) |
| 457 | 457 | (println "glimmer-jvui: timer failed, cancelled:" (ex-message e))))))) |
| 458 | 458 | |
| 459 | -(defn- drain-pending! [] | |
| 459 | +(defn- drain-pending! | |
| 460 | + "Run this frame's timers and the reconciler's queued patches. | |
| 461 | + | |
| 462 | + Answers whether any patch ran — which the caller turns into | |
| 463 | + `core/unsettle!`, and which is the whole of this backend's part in keeping | |
| 464 | + a changed tree from being painted at the sizes of the old one." | |
| 465 | + [] | |
| 460 | 466 | (run-timers!) |
| 461 | 467 | (let [[ws] (reset-vals! pending [])] |
| 462 | - (doseq [w ws] (w)))) | |
| 468 | + (doseq [w ws] (w)) | |
| 469 | + (boolean (seq ws)))) | |
| 470 | + | |
| 471 | +(defn- before! | |
| 472 | + "jvui's per-frame hook: patch the tree, then say that we did. | |
| 473 | + | |
| 474 | + A patch lands between two walks, where nothing jvui measures has moved yet | |
| 475 | + — every container still holds the size its old children asked for. Without | |
| 476 | + the `unsettle!` the next walk is the one that paints, and it paints the new | |
| 477 | + tree at those old sizes: the frame where a card is still the height of the | |
| 478 | + message it no longer holds and everything under it sits wherever that put | |
| 479 | + it. With it, that walk is a settling pass and the frame that reaches the | |
| 480 | + screen is the settled one." | |
| 481 | + [cx] | |
| 482 | + (when (drain-pending!) (c/unsettle! cx))) | |
| 463 | 483 | |
| 464 | 484 | (defn- run! |
| 465 | 485 | "glimmer.backend's :run. Creates the root page, mounts into it, then hands the |
| @@ -478,7 +498,7 @@ | ||
| 478 | 498 | (app/run! (fn [] (emit! root)) |
| 479 | 499 | {:title title :width width :height height |
| 480 | 500 | :theme (or theme theme/dark) |
| 481 | - :before drain-pending! | |
| 501 | + :before before! | |
| 482 | 502 | :frames frames :auto-quit-ms auto-quit-ms :shot shot}) |
| 483 | 503 | (finally (reset! b/loop-running? false))))) |
| 484 | 504 | |
| @@ -507,7 +527,7 @@ | ||
| 507 | 527 | for a test to assert about a layout and to click on it." |
| 508 | 528 | ([root cx] (render-once root cx [])) |
| 509 | 529 | ([root cx evs] |
| 510 | - (drain-pending!) | |
| 530 | + (before! cx) | |
| 511 | 531 | (swap! cx assoc :events evs) |
| 512 | 532 | (swap! cx c/apply-input evs) |
| 513 | 533 | (binding [*record-rects?* true] |
| @@ -456,10 +456,30 @@ | |||
| 456 | (swap! timers dissoc id) | 456 | (swap! timers dissoc id) |
| 457 | (println "glimmer-jvui: timer failed, cancelled:" (ex-message e))))))) | 457 | (println "glimmer-jvui: timer failed, cancelled:" (ex-message e))))))) |
| 458 | 458 | ||
| 459 | -(defn- drain-pending! [] | 459 | +(defn- drain-pending! |
| 460 | + "Run this frame's timers and the reconciler's queued patches. | ||
| 461 | + | ||
| 462 | + Answers whether any patch ran — which the caller turns into | ||
| 463 | + `core/unsettle!`, and which is the whole of this backend's part in keeping | ||
| 464 | + a changed tree from being painted at the sizes of the old one." | ||
| 465 | + [] | ||
| 460 | (run-timers!) | 466 | (run-timers!) |
| 461 | (let [[ws] (reset-vals! pending [])] | 467 | (let [[ws] (reset-vals! pending [])] |
| 462 | - (doseq [w ws] (w)))) | 468 | + (doseq [w ws] (w)) |
| 469 | + (boolean (seq ws)))) | ||
| 470 | + | ||
| 471 | +(defn- before! | ||
| 472 | + "jvui's per-frame hook: patch the tree, then say that we did. | ||
| 473 | + | ||
| 474 | + A patch lands between two walks, where nothing jvui measures has moved yet | ||
| 475 | + — every container still holds the size its old children asked for. Without | ||
| 476 | + the `unsettle!` the next walk is the one that paints, and it paints the new | ||
| 477 | + tree at those old sizes: the frame where a card is still the height of the | ||
| 478 | + message it no longer holds and everything under it sits wherever that put | ||
| 479 | + it. With it, that walk is a settling pass and the frame that reaches the | ||
| 480 | + screen is the settled one." | ||
| 481 | + [cx] | ||
| 482 | + (when (drain-pending!) (c/unsettle! cx))) | ||
| 463 | 483 | ||
| 464 | (defn- run! | 484 | (defn- run! |
| 465 | "glimmer.backend's :run. Creates the root page, mounts into it, then hands the | 485 | "glimmer.backend's :run. Creates the root page, mounts into it, then hands the |
| @@ -478,7 +498,7 @@ | |||
| 478 | (app/run! (fn [] (emit! root)) | 498 | (app/run! (fn [] (emit! root)) |
| 479 | {:title title :width width :height height | 499 | {:title title :width width :height height |
| 480 | :theme (or theme theme/dark) | 500 | :theme (or theme theme/dark) |
| 481 | - :before drain-pending! | 501 | + :before before! |
| 482 | :frames frames :auto-quit-ms auto-quit-ms :shot shot}) | 502 | :frames frames :auto-quit-ms auto-quit-ms :shot shot}) |
| 483 | (finally (reset! b/loop-running? false))))) | 503 | (finally (reset! b/loop-running? false))))) |
| 484 | 504 | ||
| @@ -507,7 +527,7 @@ | |||
| 507 | for a test to assert about a layout and to click on it." | 527 | for a test to assert about a layout and to click on it." |
| 508 | ([root cx] (render-once root cx [])) | 528 | ([root cx] (render-once root cx [])) |
| 509 | ([root cx evs] | 529 | ([root cx evs] |
| 510 | - (drain-pending!) | 530 | + (before! cx) |
| 511 | (swap! cx assoc :events evs) | 531 | (swap! cx assoc :events evs) |
| 512 | (swap! cx c/apply-input evs) | 532 | (swap! cx c/apply-input evs) |
| 513 | (binding [*record-rects?* true] | 533 | (binding [*record-rects?* true] |
modified
glimmer-backends/glimmer-jvui/test/glimmer_jvui/tests.clj +48 -1 | @@ -5,6 +5,7 @@ | ||
| 5 | 5 | reconciler, this backend's walk, jvui's layout and event routing — can be |
| 6 | 6 | driven with a stub that says eight pixels a character. `jolt test`." |
| 7 | 7 | (:require [glimmer.ratom :as ra] |
| 8 | + [glimmer.backend :as b] | |
| 8 | 9 | [jvui.widgets :as w] |
| 9 | 10 | [glimmer.core :as gui] |
| 10 | 11 | [glimmer-jvui.core :as jv] |
| @@ -251,6 +252,51 @@ | ||
| 251 | 252 | |
| 252 | 253 | ;; --- runner ------------------------------------------------------------------ |
| 253 | 254 | |
| 255 | +(def ^:private rows (ra/atom 1)) | |
| 256 | + | |
| 257 | +(defn- growing [] | |
| 258 | + [:vbox {} | |
| 259 | + [:card {} | |
| 260 | + (for [i (range (ra/deref rows))] | |
| 261 | + ^{:key i} [:label {:label (str "line " i)}])] | |
| 262 | + [:button {:label "under"}]]) | |
| 263 | + | |
| 264 | +(defn- check-a-patch-settles-before-it-paints! [] | |
| 265 | + ;; What the eye actually catches. A container is as big as what its children | |
| 266 | + ;; asked for LAST frame, so the first walk after the reconciler patches the | |
| 267 | + ;; tree knows the new children and the old sizes: the card already holds four | |
| 268 | + ;; lines and the button under it still sits where one line put it. That frame | |
| 269 | + ;; is painted, and the frame after it is right — a flash of the old layout | |
| 270 | + ;; under the new contents, on every change the client makes. | |
| 271 | + ;; | |
| 272 | + ;; The fix is for the backend to say that it patched something, so jvui | |
| 273 | + ;; spends that walk settling instead of painting. Checked here rather than in | |
| 274 | + ;; jvui's own tests because it takes a reconciler to reproduce: the patch has | |
| 275 | + ;; to arrive BETWEEN two frames, which is the one thing a widget called | |
| 276 | + ;; directly cannot do. | |
| 277 | + ;; | |
| 278 | + ;; `loop-running?` is what makes glimmer defer a re-render onto the frame | |
| 279 | + ;; loop rather than run it inline, which is the arrangement a window is in | |
| 280 | + ;; and the only one where any of this happens. | |
| 281 | + (let [root (jv/root-node) cx (ctx)] | |
| 282 | + (ra/reset! rows 1) | |
| 283 | + (gui/mount root :page [growing]) | |
| 284 | + (dotimes [_ 3] (jv/render-once root cx)) | |
| 285 | + (reset! b/loop-running? true) | |
| 286 | + (try | |
| 287 | + (let [under #(second (:rect @(tagged root :button))) | |
| 288 | + was (under)] | |
| 289 | + (ra/reset! rows 4) | |
| 290 | + (jv/render-once root cx) | |
| 291 | + (let [painted (under)] | |
| 292 | + (jv/render-once root cx) | |
| 293 | + (check! (not= painted was) | |
| 294 | + "the frame after a patch has moved what the patch moved") | |
| 295 | + (check! (= painted (under)) | |
| 296 | + (str "and it is already where it comes to rest: painted at " | |
| 297 | + painted ", settles at " (under))))) | |
| 298 | + (finally (reset! b/loop-running? false))))) | |
| 299 | + | |
| 254 | 300 | (def ^:private checks |
| 255 | 301 | [["the reconciler builds a tree" check-tree!] |
| 256 | 302 | ["the walk places widgets" check-walk-places-widgets!] |
| @@ -262,7 +308,8 @@ | ||
| 262 | 308 | ["a list sticks to the end" check-scroll-sticks!] |
| 263 | 309 | ["Enter sends" check-enter-sends!] |
| 264 | 310 | ["an entry round-trips" check-entry-round-trips!] |
| 265 | - ["a checkbox round-trips" check-checkbox-round-trips!]]) | |
| 311 | + ["a checkbox round-trips" check-checkbox-round-trips!] | |
| 312 | + ["a patch settles before it paints" check-a-patch-settles-before-it-paints!]]) | |
| 266 | 313 | |
| 267 | 314 | (defn -main [& _] |
| 268 | 315 | (doseq [[name f] checks] |
| @@ -5,6 +5,7 @@ | |||
| 5 | reconciler, this backend's walk, jvui's layout and event routing — can be | 5 | reconciler, this backend's walk, jvui's layout and event routing — can be |
| 6 | driven with a stub that says eight pixels a character. `jolt test`." | 6 | driven with a stub that says eight pixels a character. `jolt test`." |
| 7 | (:require [glimmer.ratom :as ra] | 7 | (:require [glimmer.ratom :as ra] |
| 8 | + [glimmer.backend :as b] | ||
| 8 | [jvui.widgets :as w] | 9 | [jvui.widgets :as w] |
| 9 | [glimmer.core :as gui] | 10 | [glimmer.core :as gui] |
| 10 | [glimmer-jvui.core :as jv] | 11 | [glimmer-jvui.core :as jv] |
| @@ -251,6 +252,51 @@ | |||
| 251 | 252 | ||
| 252 | ;; --- runner ------------------------------------------------------------------ | 253 | ;; --- runner ------------------------------------------------------------------ |
| 253 | 254 | ||
| 255 | +(def ^:private rows (ra/atom 1)) | ||
| 256 | + | ||
| 257 | +(defn- growing [] | ||
| 258 | + [:vbox {} | ||
| 259 | + [:card {} | ||
| 260 | + (for [i (range (ra/deref rows))] | ||
| 261 | + ^{:key i} [:label {:label (str "line " i)}])] | ||
| 262 | + [:button {:label "under"}]]) | ||
| 263 | + | ||
| 264 | +(defn- check-a-patch-settles-before-it-paints! [] | ||
| 265 | + ;; What the eye actually catches. A container is as big as what its children | ||
| 266 | + ;; asked for LAST frame, so the first walk after the reconciler patches the | ||
| 267 | + ;; tree knows the new children and the old sizes: the card already holds four | ||
| 268 | + ;; lines and the button under it still sits where one line put it. That frame | ||
| 269 | + ;; is painted, and the frame after it is right — a flash of the old layout | ||
| 270 | + ;; under the new contents, on every change the client makes. | ||
| 271 | + ;; | ||
| 272 | + ;; The fix is for the backend to say that it patched something, so jvui | ||
| 273 | + ;; spends that walk settling instead of painting. Checked here rather than in | ||
| 274 | + ;; jvui's own tests because it takes a reconciler to reproduce: the patch has | ||
| 275 | + ;; to arrive BETWEEN two frames, which is the one thing a widget called | ||
| 276 | + ;; directly cannot do. | ||
| 277 | + ;; | ||
| 278 | + ;; `loop-running?` is what makes glimmer defer a re-render onto the frame | ||
| 279 | + ;; loop rather than run it inline, which is the arrangement a window is in | ||
| 280 | + ;; and the only one where any of this happens. | ||
| 281 | + (let [root (jv/root-node) cx (ctx)] | ||
| 282 | + (ra/reset! rows 1) | ||
| 283 | + (gui/mount root :page [growing]) | ||
| 284 | + (dotimes [_ 3] (jv/render-once root cx)) | ||
| 285 | + (reset! b/loop-running? true) | ||
| 286 | + (try | ||
| 287 | + (let [under #(second (:rect @(tagged root :button))) | ||
| 288 | + was (under)] | ||
| 289 | + (ra/reset! rows 4) | ||
| 290 | + (jv/render-once root cx) | ||
| 291 | + (let [painted (under)] | ||
| 292 | + (jv/render-once root cx) | ||
| 293 | + (check! (not= painted was) | ||
| 294 | + "the frame after a patch has moved what the patch moved") | ||
| 295 | + (check! (= painted (under)) | ||
| 296 | + (str "and it is already where it comes to rest: painted at " | ||
| 297 | + painted ", settles at " (under))))) | ||
| 298 | + (finally (reset! b/loop-running? false))))) | ||
| 299 | + | ||
| 254 | (def ^:private checks | 300 | (def ^:private checks |
| 255 | [["the reconciler builds a tree" check-tree!] | 301 | [["the reconciler builds a tree" check-tree!] |
| 256 | ["the walk places widgets" check-walk-places-widgets!] | 302 | ["the walk places widgets" check-walk-places-widgets!] |
| @@ -262,7 +308,8 @@ | |||
| 262 | ["a list sticks to the end" check-scroll-sticks!] | 308 | ["a list sticks to the end" check-scroll-sticks!] |
| 263 | ["Enter sends" check-enter-sends!] | 309 | ["Enter sends" check-enter-sends!] |
| 264 | ["an entry round-trips" check-entry-round-trips!] | 310 | ["an entry round-trips" check-entry-round-trips!] |
| 265 | - ["a checkbox round-trips" check-checkbox-round-trips!]]) | 311 | + ["a checkbox round-trips" check-checkbox-round-trips!] |
| 312 | + ["a patch settles before it paints" check-a-patch-settles-before-it-paints!]]) | ||
| 266 | 313 | ||
| 267 | (defn -main [& _] | 314 | (defn -main [& _] |
| 268 | (doseq [[name f] checks] | 315 | (doseq [[name f] checks] |
modified
jvui/src/jvui/app.clj +5 -4 | @@ -49,9 +49,10 @@ | ||
| 49 | 49 | `render` is a thunk: it calls widgets, and what they answer is what the |
| 50 | 50 | person did. Options: :title :width :height :theme :frames :auto-quit-ms. |
| 51 | 51 | |
| 52 | - :before is a thunk run once per frame, before the walk. A caller with a tree | |
| 53 | - to patch — glimmer's reconciler is the one — needs somewhere to do it that is | |
| 54 | - not *during* a walk of that tree, and this is it. | |
| 52 | + :before is run once per frame, before the walk, and is handed the context. A | |
| 53 | + caller with a tree to patch — glimmer's reconciler is the one — needs | |
| 54 | + somewhere to do it that is not *during* a walk of that tree, and this is it; | |
| 55 | + the context is there so it can say what it did, with `core/unsettle!`. | |
| 55 | 56 | |
| 56 | 57 | :frames and :auto-quit-ms exist for the same reason glimmer-gfx's do — so a |
| 57 | 58 | smoke test can paint a real window and then leave, without a person in front |
| @@ -86,7 +87,7 @@ | ||
| 86 | 87 | (swap! ctx (fn [s] (-> s (assoc :events evs) |
| 87 | 88 | (assoc :size [ow oh])))) |
| 88 | 89 | (swap! ctx c/apply-input evs) |
| 89 | - (when before (before)) | |
| 90 | + (when before (before ctx)) | |
| 90 | 91 | (let [[br bg bb ba] (get (:theme @ctx) :bg)] |
| 91 | 92 | (sdl/draw-color! renderer br bg bb ba) |
| 92 | 93 | (sdl/clear! renderer)) |
| @@ -49,9 +49,10 @@ | |||
| 49 | `render` is a thunk: it calls widgets, and what they answer is what the | 49 | `render` is a thunk: it calls widgets, and what they answer is what the |
| 50 | person did. Options: :title :width :height :theme :frames :auto-quit-ms. | 50 | person did. Options: :title :width :height :theme :frames :auto-quit-ms. |
| 51 | 51 | ||
| 52 | - :before is a thunk run once per frame, before the walk. A caller with a tree | 52 | + :before is run once per frame, before the walk, and is handed the context. A |
| 53 | - to patch — glimmer's reconciler is the one — needs somewhere to do it that is | 53 | + caller with a tree to patch — glimmer's reconciler is the one — needs |
| 54 | - not *during* a walk of that tree, and this is it. | 54 | + somewhere to do it that is not *during* a walk of that tree, and this is it; |
| 55 | + the context is there so it can say what it did, with `core/unsettle!`. | ||
| 55 | 56 | ||
| 56 | :frames and :auto-quit-ms exist for the same reason glimmer-gfx's do — so a | 57 | :frames and :auto-quit-ms exist for the same reason glimmer-gfx's do — so a |
| 57 | smoke test can paint a real window and then leave, without a person in front | 58 | smoke test can paint a real window and then leave, without a person in front |
| @@ -86,7 +87,7 @@ | |||
| 86 | (swap! ctx (fn [s] (-> s (assoc :events evs) | 87 | (swap! ctx (fn [s] (-> s (assoc :events evs) |
| 87 | (assoc :size [ow oh])))) | 88 | (assoc :size [ow oh])))) |
| 88 | (swap! ctx c/apply-input evs) | 89 | (swap! ctx c/apply-input evs) |
| 89 | - (when before (before)) | 90 | + (when before (before ctx)) |
| 90 | (let [[br bg bb ba] (get (:theme @ctx) :bg)] | 91 | (let [[br bg bb ba] (get (:theme @ctx) :bg)] |
| 91 | (sdl/draw-color! renderer br bg bb ba) | 92 | (sdl/draw-color! renderer br bg bb ba) |
| 92 | (sdl/clear! renderer)) | 93 | (sdl/clear! renderer)) |
modified
jvui/src/jvui/core.clj +36 -1 | @@ -86,6 +86,24 @@ | ||
| 86 | 86 | "Ask for another layout pass before anything is drawn." |
| 87 | 87 | [] (upd! assoc :refresh? true)) |
| 88 | 88 | |
| 89 | +(defn unsettle! | |
| 90 | + "Say that the tree changed under us, so the next frame settles before it | |
| 91 | + paints. Takes the context rather than reading `*ui*`: it is called between | |
| 92 | + frames, which is the only time it means anything. | |
| 93 | + | |
| 94 | + `frame!` skips its settling passes while nothing is moving, which is almost | |
| 95 | + every frame. That test looks at what the LAST walk did, and a change | |
| 96 | + arriving from outside the walk — a reconciler patching the tree between two | |
| 97 | + frames — is invisible to it until the damage is done: a box takes its size | |
| 98 | + from what its children asked for last frame, so the first frame after a | |
| 99 | + patch draws the new tree at the old tree's sizes. A card the wrong height | |
| 100 | + with everything below it in the wrong place, for one frame, on every change | |
| 101 | + the client makes. Saying so here is what turns that frame back into a | |
| 102 | + settling pass, which is thrown away rather than shown. | |
| 103 | + | |
| 104 | + It costs those passes only on the frames that actually changed." | |
| 105 | + [ctx-atom] (swap! ctx-atom assoc :settled? false)) | |
| 106 | + | |
| 89 | 107 | ;; -------------------------------------------------------------------- theme |
| 90 | 108 | |
| 91 | 109 | (defn th [k] (get (:theme (ui)) k)) |
| @@ -667,6 +685,23 @@ | ||
| 667 | 685 | {:n (ctr f N) :expanders (ctr f EXPANDERS) :along (ctr f ALONG)})) |
| 668 | 686 | (:refresh? @ctx-atom))) |
| 669 | 687 | |
| 688 | +(def ^:private settle-passes | |
| 689 | + "How many walks a frame may spend settling before it paints anyway. | |
| 690 | + | |
| 691 | + Three was the first number and it was not enough: a size travels up the | |
| 692 | + tree in one pass and back down in the next, so a change deep inside a | |
| 693 | + screen — the reader's list inside a split inside a page — is still moving | |
| 694 | + on the third, and what settles on the fourth was painted on the third. The | |
| 695 | + cost is paid only by the frames that are actually moving, since the loop | |
| 696 | + stops the moment nothing reports a new size, and those are the frames that | |
| 697 | + would otherwise be the ones with something wrong on them. | |
| 698 | + | |
| 699 | + It is still a bound and not a promise: a layout that oscillates rather than | |
| 700 | + converges — a wrap whose width depends on its own height — would spin here | |
| 701 | + forever, and a bounded loop turns that into something visible instead of | |
| 702 | + something hung." | |
| 703 | + 8) | |
| 704 | + | |
| 670 | 705 | (defn frame! |
| 671 | 706 | "Walk `render` until the layout settles, then walk it once more and draw. |
| 672 | 707 | |
| @@ -684,7 +719,7 @@ | ||
| 684 | 719 | (loop [pass 0] |
| 685 | 720 | (swap! ctx-atom assoc :events []) |
| 686 | 721 | (let [again? (one-pass! ctx-atom render false)] |
| 687 | - (when (and again? (< pass 2)) (recur (inc pass))))) | |
| 722 | + (when (and again? (< pass (dec settle-passes))) (recur (inc pass))))) | |
| 688 | 723 | (swap! ctx-atom assoc :events real)) |
| 689 | 724 | (let [moved? (one-pass! ctx-atom render true)] |
| 690 | 725 | (swap! ctx-atom assoc :settled? (not moved?)) |
| @@ -86,6 +86,24 @@ | |||
| 86 | "Ask for another layout pass before anything is drawn." | 86 | "Ask for another layout pass before anything is drawn." |
| 87 | [] (upd! assoc :refresh? true)) | 87 | [] (upd! assoc :refresh? true)) |
| 88 | 88 | ||
| 89 | +(defn unsettle! | ||
| 90 | + "Say that the tree changed under us, so the next frame settles before it | ||
| 91 | + paints. Takes the context rather than reading `*ui*`: it is called between | ||
| 92 | + frames, which is the only time it means anything. | ||
| 93 | + | ||
| 94 | + `frame!` skips its settling passes while nothing is moving, which is almost | ||
| 95 | + every frame. That test looks at what the LAST walk did, and a change | ||
| 96 | + arriving from outside the walk — a reconciler patching the tree between two | ||
| 97 | + frames — is invisible to it until the damage is done: a box takes its size | ||
| 98 | + from what its children asked for last frame, so the first frame after a | ||
| 99 | + patch draws the new tree at the old tree's sizes. A card the wrong height | ||
| 100 | + with everything below it in the wrong place, for one frame, on every change | ||
| 101 | + the client makes. Saying so here is what turns that frame back into a | ||
| 102 | + settling pass, which is thrown away rather than shown. | ||
| 103 | + | ||
| 104 | + It costs those passes only on the frames that actually changed." | ||
| 105 | + [ctx-atom] (swap! ctx-atom assoc :settled? false)) | ||
| 106 | + | ||
| 89 | ;; -------------------------------------------------------------------- theme | 107 | ;; -------------------------------------------------------------------- theme |
| 90 | 108 | ||
| 91 | (defn th [k] (get (:theme (ui)) k)) | 109 | (defn th [k] (get (:theme (ui)) k)) |
| @@ -667,6 +685,23 @@ | |||
| 667 | {:n (ctr f N) :expanders (ctr f EXPANDERS) :along (ctr f ALONG)})) | 685 | {:n (ctr f N) :expanders (ctr f EXPANDERS) :along (ctr f ALONG)})) |
| 668 | (:refresh? @ctx-atom))) | 686 | (:refresh? @ctx-atom))) |
| 669 | 687 | ||
| 688 | +(def ^:private settle-passes | ||
| 689 | + "How many walks a frame may spend settling before it paints anyway. | ||
| 690 | + | ||
| 691 | + Three was the first number and it was not enough: a size travels up the | ||
| 692 | + tree in one pass and back down in the next, so a change deep inside a | ||
| 693 | + screen — the reader's list inside a split inside a page — is still moving | ||
| 694 | + on the third, and what settles on the fourth was painted on the third. The | ||
| 695 | + cost is paid only by the frames that are actually moving, since the loop | ||
| 696 | + stops the moment nothing reports a new size, and those are the frames that | ||
| 697 | + would otherwise be the ones with something wrong on them. | ||
| 698 | + | ||
| 699 | + It is still a bound and not a promise: a layout that oscillates rather than | ||
| 700 | + converges — a wrap whose width depends on its own height — would spin here | ||
| 701 | + forever, and a bounded loop turns that into something visible instead of | ||
| 702 | + something hung." | ||
| 703 | + 8) | ||
| 704 | + | ||
| 670 | (defn frame! | 705 | (defn frame! |
| 671 | "Walk `render` until the layout settles, then walk it once more and draw. | 706 | "Walk `render` until the layout settles, then walk it once more and draw. |
| 672 | 707 | ||
| @@ -684,7 +719,7 @@ | |||
| 684 | (loop [pass 0] | 719 | (loop [pass 0] |
| 685 | (swap! ctx-atom assoc :events []) | 720 | (swap! ctx-atom assoc :events []) |
| 686 | (let [again? (one-pass! ctx-atom render false)] | 721 | (let [again? (one-pass! ctx-atom render false)] |
| 687 | - (when (and again? (< pass 2)) (recur (inc pass))))) | 722 | + (when (and again? (< pass (dec settle-passes))) (recur (inc pass))))) |
| 688 | (swap! ctx-atom assoc :events real)) | 723 | (swap! ctx-atom assoc :events real)) |
| 689 | (let [moved? (one-pass! ctx-atom render true)] | 724 | (let [moved? (one-pass! ctx-atom render true)] |
| 690 | (swap! ctx-atom assoc :settled? (not moved?)) | 725 | (swap! ctx-atom assoc :settled? (not moved?)) |
modified
jvui/src/jvui/widgets.clj +11 -0 | @@ -56,6 +56,17 @@ | ||
| 56 | 56 | :spacing (c/th :spacing) |
| 57 | 57 | :expand :vertical |
| 58 | 58 | :gravity [0.5 0.0] |
| 59 | + ;; EXACTLY this wide, rather than at least: without | |
| 60 | + ;; `:fixed` the column reports what its children asked | |
| 61 | + ;; for, and its own width is then whatever they asked | |
| 62 | + ;; for last time — a page whose width is a function of | |
| 63 | + ;; its own width. Where that does not come to rest it | |
| 64 | + ;; swings between two widths forever, one per frame, | |
| 65 | + ;; and every card on the page swings with it. Fixed | |
| 66 | + ;; here, the width is the window's and the cap's, and | |
| 67 | + ;; the wrapping below it has something that holds | |
| 68 | + ;; still to wrap against. | |
| 69 | + :fixed true | |
| 59 | 70 | :min-size [(min (double (or (:max-width opts) 1.0e9)) |
| 60 | 71 | (max 0.0 (- (double (first (:size (c/ui)))) |
| 61 | 72 | (* 2 (double (c/th :padding)))))) |
| @@ -56,6 +56,17 @@ | |||
| 56 | :spacing (c/th :spacing) | 56 | :spacing (c/th :spacing) |
| 57 | :expand :vertical | 57 | :expand :vertical |
| 58 | :gravity [0.5 0.0] | 58 | :gravity [0.5 0.0] |
| 59 | + ;; EXACTLY this wide, rather than at least: without | ||
| 60 | + ;; `:fixed` the column reports what its children asked | ||
| 61 | + ;; for, and its own width is then whatever they asked | ||
| 62 | + ;; for last time — a page whose width is a function of | ||
| 63 | + ;; its own width. Where that does not come to rest it | ||
| 64 | + ;; swings between two widths forever, one per frame, | ||
| 65 | + ;; and every card on the page swings with it. Fixed | ||
| 66 | + ;; here, the width is the window's and the cap's, and | ||
| 67 | + ;; the wrapping below it has something that holds | ||
| 68 | + ;; still to wrap against. | ||
| 69 | + :fixed true | ||
| 59 | :min-size [(min (double (or (:max-width opts) 1.0e9)) | 70 | :min-size [(min (double (or (:max-width opts) 1.0e9)) |
| 60 | (max 0.0 (- (double (first (:size (c/ui)))) | 71 | (max 0.0 (- (double (first (:size (c/ui)))) |
| 61 | (* 2 (double (c/th :padding)))))) | 72 | (* 2 (double (c/th :padding)))))) |
modified
jvui/test/jvui/tests.clj +30 -0 | @@ -361,6 +361,35 @@ | ||
| 361 | 361 | (check! (<= w 400.5) "a long label wraps inside its box") |
| 362 | 362 | (check! (> h 16.5) "and takes more than one line to do it")))) |
| 363 | 363 | |
| 364 | +(defn- check-page-holds-its-width! [] | |
| 365 | + ;; A page is `:max-width` wide and stays there, whatever is inside it. | |
| 366 | + ;; | |
| 367 | + ;; It used to report whatever its children asked for, which made its width a | |
| 368 | + ;; function of its own width: a child asks for the room it was given, the | |
| 369 | + ;; page grows to that, the child asks for the new room. One thing too wide to | |
| 370 | + ;; wrap — a URL, a row of buttons — dragged the whole column out past the cap | |
| 371 | + ;; and took every wrapping paragraph on the page with it, and where the sums | |
| 372 | + ;; did not come to rest the column swung between two widths, one per frame, | |
| 373 | + ;; for as long as the screen was up. The settings screen did exactly that. | |
| 374 | + ;; | |
| 375 | + ;; Eight pixels a character from the stub, so 40 of them is 320 — wider than | |
| 376 | + ;; the 200 the page is capped at, and unbreakable because it has no spaces. | |
| 377 | + (let [cx (ctx) seen (atom []) | |
| 378 | + wide (apply str (repeat 40 "x")) | |
| 379 | + text (apply str (repeat 12 "wordy ")) | |
| 380 | + render (fn [] | |
| 381 | + (w/page {:max-width 200} | |
| 382 | + (w/card {} (w/label wide {:wrap false :expand :none})) | |
| 383 | + (w/card {} (swap! seen conj (w/label text)))))] | |
| 384 | + (dotimes [_ 6] (frame! cx render)) | |
| 385 | + (let [widths (distinct (map #(nth % 2) (take-last 3 @seen)))] | |
| 386 | + (check! (= 1 (count widths)) | |
| 387 | + (str "a page settles on one width: " (pr-str widths))) | |
| 388 | + (check! (<= (first widths) 200.5) | |
| 389 | + (str "and it is the one it was capped at, not the one the widest " | |
| 390 | + "thing on it wanted: " (pr-str widths)))))) | |
| 391 | + | |
| 392 | + | |
| 364 | 393 | ;; --- runner ------------------------------------------------------------------ |
| 365 | 394 | |
| 366 | 395 | (def ^:private checks |
| @@ -368,6 +397,7 @@ | ||
| 368 | 397 | ["a column stacks" check-vbox-stacks!] |
| 369 | 398 | ["a row runs" check-hbox-runs!] |
| 370 | 399 | ["a long label wraps" check-label-wraps!] |
| 400 | + ["a page holds its width" check-page-holds-its-width!] | |
| 371 | 401 | ["an unbreakable word breaks" check-long-word-breaks!] |
| 372 | 402 | ["a row wraps" check-row-wraps!] |
| 373 | 403 | ["expand shares the slack" check-expand-shares!] |
| @@ -361,6 +361,35 @@ | |||
| 361 | (check! (<= w 400.5) "a long label wraps inside its box") | 361 | (check! (<= w 400.5) "a long label wraps inside its box") |
| 362 | (check! (> h 16.5) "and takes more than one line to do it")))) | 362 | (check! (> h 16.5) "and takes more than one line to do it")))) |
| 363 | 363 | ||
| 364 | +(defn- check-page-holds-its-width! [] | ||
| 365 | + ;; A page is `:max-width` wide and stays there, whatever is inside it. | ||
| 366 | + ;; | ||
| 367 | + ;; It used to report whatever its children asked for, which made its width a | ||
| 368 | + ;; function of its own width: a child asks for the room it was given, the | ||
| 369 | + ;; page grows to that, the child asks for the new room. One thing too wide to | ||
| 370 | + ;; wrap — a URL, a row of buttons — dragged the whole column out past the cap | ||
| 371 | + ;; and took every wrapping paragraph on the page with it, and where the sums | ||
| 372 | + ;; did not come to rest the column swung between two widths, one per frame, | ||
| 373 | + ;; for as long as the screen was up. The settings screen did exactly that. | ||
| 374 | + ;; | ||
| 375 | + ;; Eight pixels a character from the stub, so 40 of them is 320 — wider than | ||
| 376 | + ;; the 200 the page is capped at, and unbreakable because it has no spaces. | ||
| 377 | + (let [cx (ctx) seen (atom []) | ||
| 378 | + wide (apply str (repeat 40 "x")) | ||
| 379 | + text (apply str (repeat 12 "wordy ")) | ||
| 380 | + render (fn [] | ||
| 381 | + (w/page {:max-width 200} | ||
| 382 | + (w/card {} (w/label wide {:wrap false :expand :none})) | ||
| 383 | + (w/card {} (swap! seen conj (w/label text)))))] | ||
| 384 | + (dotimes [_ 6] (frame! cx render)) | ||
| 385 | + (let [widths (distinct (map #(nth % 2) (take-last 3 @seen)))] | ||
| 386 | + (check! (= 1 (count widths)) | ||
| 387 | + (str "a page settles on one width: " (pr-str widths))) | ||
| 388 | + (check! (<= (first widths) 200.5) | ||
| 389 | + (str "and it is the one it was capped at, not the one the widest " | ||
| 390 | + "thing on it wanted: " (pr-str widths)))))) | ||
| 391 | + | ||
| 392 | + | ||
| 364 | ;; --- runner ------------------------------------------------------------------ | 393 | ;; --- runner ------------------------------------------------------------------ |
| 365 | 394 | ||
| 366 | (def ^:private checks | 395 | (def ^:private checks |
| @@ -368,6 +397,7 @@ | |||
| 368 | ["a column stacks" check-vbox-stacks!] | 397 | ["a column stacks" check-vbox-stacks!] |
| 369 | ["a row runs" check-hbox-runs!] | 398 | ["a row runs" check-hbox-runs!] |
| 370 | ["a long label wraps" check-label-wraps!] | 399 | ["a long label wraps" check-label-wraps!] |
| 400 | + ["a page holds its width" check-page-holds-its-width!] | ||
| 371 | ["an unbreakable word breaks" check-long-word-breaks!] | 401 | ["an unbreakable word breaks" check-long-word-breaks!] |
| 372 | ["a row wraps" check-row-wraps!] | 402 | ["a row wraps" check-row-wraps!] |
| 373 | ["expand shares the slack" check-expand-shares!] | 403 | ["expand shares the slack" check-expand-shares!] |