From dfa097e98857f3153a003010f5a7e15f9545796b Mon Sep 17 00:00:00 2001 From: Kinneyzhang Date: Sun, 6 Sep 2026 18:23:11 +0800 Subject: [PATCH] fix: deliver viewport updates after window size hooks --- ebox-state-contract.el | 12 ++ ebox-viewport.el | 76 ++++++-- tests/ebox-m0a-inventory-fixture.el | 12 ++ tests/ebox-m0a-inventory-tests.el | 2 + tests/ebox-state-contract-tests.el | 3 +- tests/ebox-surface-tests.el | 267 +++++++++++++++++++++++++--- 6 files changed, 329 insertions(+), 43 deletions(-) diff --git a/ebox-state-contract.el b/ebox-state-contract.el index bfa2367..b1a58d1 100644 --- a/ebox-state-contract.el +++ b/ebox-state-contract.el @@ -117,6 +117,18 @@ :rollback cancel-candidate-work :rebuild-proof reschedule-from-committed-generation :cleanup cancel-timers-jobs-and-gc-lease) + (:id viewport-event-authority + :storage (ebox--window-size-change-pending-frames + ebox--window-size-change-timer) + :current-contract frame-intent-queue-and-single-event-loop-task + :target-contract frame-intent-queue-and-single-event-loop-task + :category generation-bound-mutable-authority + :owner ebox-viewport + :mutation-api window-notify-schedule-and-drain + :generation-binding current-mounted-generation-sampled-at-delivery + :rollback publication-owned-rollback-and-requeue-unstarted-frames + :rebuild-proof subsequent-window-notification + :cleanup consume-frame-intents-and-release-dispatched-timer) (:id incremental-batch-authority :storage (ebox-incremental--batch-table) :current-contract process-buffer-keyed-open-batch-state diff --git a/ebox-viewport.el b/ebox-viewport.el index 2d04eaf..61f2aab 100644 --- a/ebox-viewport.el +++ b/ebox-viewport.el @@ -1,13 +1,13 @@ -;;; ebox-viewport.el --- Immediate Emacs viewport delivery -*- lexical-binding: t; -*- +;;; ebox-viewport.el --- Event-loop viewport delivery -*- lexical-binding: t; -*- ;;; Commentary: ;; Emacs owns window events; Ebox owns the mounted viewport. This module is -;; the single bridge between them. Size hooks only sample the latest visible -;; dimensions and synchronously publish one exact incremental viewport update. -;; Emacs serializes the hook; Ebox's retained renderer bounds each transaction. +;; the single bridge between them. Size hooks record frame intents; one task +;; samples current visible dimensions and publishes after the redisplay hook. ;;; Code: +(require 'cl-lib) (require 'ebox-surface) (declare-function ebox--buffer-render-state "ebox-incremental" (buffer)) @@ -15,23 +15,24 @@ "ebox" (buffer viewport-width &optional viewport-height)) (defvar ebox--window-size-change-in-progress nil - "Non-nil while Ebox is synchronizing visible window viewports. -External GUI events cannot interleave in Emacs's single thread. A nested call -can only originate from the current publication and therefore has no newer -window geometry to publish.") + "Non-nil while Ebox is draining one batch of viewport notifications.") + +(defvar ebox--window-size-change-pending-frames nil + "Frames awaiting viewport synchronization, deduplicated by EQ. +The queue holds no buffers, runtime roots or sampled dimensions.") + +(defvar ebox--window-size-change-timer nil + "The single owned task scheduled to drain viewport notifications.") (defun ebox-viewport-window-width (window) "Return Ebox's display-safe viewport width for live WINDOW." (and (window-live-p window) (ebox-surface--window-content-width window))) -(defun ebox--window-size-change (frame) - "Synchronize mounted Ebox buffers visible in FRAME immediately." - (when (and (frame-live-p frame) - (not noninteractive) - (not ebox--window-size-change-in-progress)) - (let ((ebox--window-size-change-in-progress t) - (seen (make-hash-table :test #'eq))) +(defun ebox--window-size-change-sync-frame (frame) + "Synchronize current mounted Ebox viewports whose canonical window is in FRAME." + (when (frame-live-p frame) + (let ((seen (make-hash-table :test #'eq))) (dolist (window (window-list frame 'no-minibuf)) (when (window-live-p window) (let ((buffer (window-buffer window))) @@ -58,6 +59,51 @@ window geometry to publish.") (ebox-rerender-buffer-with-context buffer width height)))))))))))) +(defun ebox--window-size-change-schedule () + "Schedule one task for live pending frames outside the current batch." + (setq ebox--window-size-change-pending-frames + (cl-delete-if-not #'frame-live-p + ebox--window-size-change-pending-frames)) + (when (and ebox--window-size-change-pending-frames + (not ebox--window-size-change-in-progress) + (not ebox--window-size-change-timer)) + (setq ebox--window-size-change-timer + (run-at-time 0 nil #'ebox--window-size-change-drain)))) + +(defun ebox--window-size-change-drain () + "Consume one frame batch, leaving nested notifications for a later task." + (unless ebox--window-size-change-in-progress + (let ((ebox--window-size-change-in-progress t) + (frames (nreverse ebox--window-size-change-pending-frames)) + first-error) + (unwind-protect + (progn + (setq ebox--window-size-change-pending-frames nil + ebox--window-size-change-timer nil) + ;; Timer dispatch inhibits quit by default. Enable C-g only after + ;; cleanup is installed; TP retains its own final-accept protection. + (let ((inhibit-quit nil)) + (while frames + (let ((frame (pop frames))) + (condition-case failure + (ebox--window-size-change-sync-frame frame) + (error (unless first-error (setq first-error failure)))))))) + ;; A quit may leave unstarted frames. Keep those intents, but do not + ;; retry the frame whose publication already started and failed. + (dolist (frame frames) + (unless (memq frame ebox--window-size-change-pending-frames) + (push frame ebox--window-size-change-pending-frames))) + (let ((ebox--window-size-change-in-progress nil)) + (ebox--window-size-change-schedule))) + (when first-error (signal (car first-error) (cdr first-error)))))) + +(defun ebox--window-size-change (frame) + "Record FRAME's latest viewport intent without rendering inside the hook." + (when (and (frame-live-p frame) (not noninteractive)) + (unless (memq frame ebox--window-size-change-pending-frames) + (push frame ebox--window-size-change-pending-frames)) + (ebox--window-size-change-schedule))) + (when (boundp 'window-size-change-functions) (add-hook 'window-size-change-functions #'ebox--window-size-change)) diff --git a/tests/ebox-m0a-inventory-fixture.el b/tests/ebox-m0a-inventory-fixture.el index 8533983..123d4f8 100644 --- a/tests/ebox-m0a-inventory-fixture.el +++ b/tests/ebox-m0a-inventory-fixture.el @@ -280,6 +280,18 @@ :authority ebox :lifetime process-by-buffer :rollback tp-client-state-or-snapshot :rebuild render-or-commit :cleanup buffer-kill-and-unmount) + (:id viewport/pending-frames + :storage (:global ebox--window-size-change-pending-frames) + :owner ebox-viewport :authority ebox :lifetime until-event-loop-drain + :rollback unstarted-frames-requeued-on-quit + :rebuild subsequent-window-size-notification + :cleanup drain-consumption-and-dead-frame-pruning) + (:id viewport/drain-timer + :storage (:global ebox--window-size-change-timer) + :owner ebox-viewport :authority ebox :lifetime until-event-loop-drain + :rollback not-publication-authority + :rebuild schedule-remaining-pending-frames + :cleanup timer-entry) (:id buffer-runtime/node-table :storage (:state-key :node-table) :proposed-category generation-fact :owner ebox-tree :authority ebox diff --git a/tests/ebox-m0a-inventory-tests.el b/tests/ebox-m0a-inventory-tests.el index 10caa85..b8a5821 100644 --- a/tests/ebox-m0a-inventory-tests.el +++ b/tests/ebox-m0a-inventory-tests.el @@ -231,6 +231,8 @@ scratch tables, which are intentionally outside the retained-store inventory." (:global ebox--scroll-idle-prefetch-timers) (:global ebox--runtime-prewarm-jobs) (:global ebox--runtime-prewarm-timers) + (:global ebox--window-size-change-pending-frames) + (:global ebox--window-size-change-timer) (:global ebox--window-line-renderer-table) (:global ebox--window-line-prewarmer-table) (:global ebox-native-reflow--compile-property-template-ids) diff --git a/tests/ebox-state-contract-tests.el b/tests/ebox-state-contract-tests.el index 1f73910..3350e88 100644 --- a/tests/ebox-state-contract-tests.el +++ b/tests/ebox-state-contract-tests.el @@ -407,7 +407,8 @@ exclusion freshness, but only retained/container evidence is coverage-gated." (ert-deftest ebox-state-contract-never-classifies-live-authority-as-cache () "Cleanup-sensitive handles retain generation-bound mutable authority." (dolist (id '(scroll-runtime-authority native-runtime-authority - runtime-prewarm-authority incremental-batch-authority + runtime-prewarm-authority viewport-event-authority + incremental-batch-authority layout-context-port-authority identity-allocation-authority buffer-surface-runtime-authority)) (let ((record (ebox-state-contract-record id))) diff --git a/tests/ebox-surface-tests.el b/tests/ebox-surface-tests.el index c3a1975..d232599 100644 --- a/tests/ebox-surface-tests.el +++ b/tests/ebox-surface-tests.el @@ -440,8 +440,71 @@ (when (get-buffer " *other-window*") (kill-buffer " *other-window*"))))) +(defvar ebox--window-size-change-pending-frames) +(defvar ebox--window-size-change-timer) + +(defmacro ebox-surface-test--with-viewport-task (&rest body) + "Run BODY with an isolated viewport task queue and DISPATCH callback." + (declare (indent 0)) + `(let ((ebox--window-size-change-pending-frames nil) + (ebox--window-size-change-timer nil) + (ebox--window-size-change-in-progress nil) + (native-comp-enable-subr-trampolines nil) + scheduled) + (cl-letf (((symbol-function 'run-at-time) + (lambda (delay repeat function &rest arguments) + (should (eql delay 0)) + (should-not repeat) + (let ((task (list function arguments))) + (setq scheduled (append scheduled (list task))) + task))) + (noninteractive nil)) + (cl-labels ((dispatch () + (should (= (length scheduled) 1)) + (let ((task (pop scheduled)) + ;; Match the event loop's timer-handler default. + (inhibit-quit t)) + (apply (car task) (cadr task))))) + ,@body)))) + +(ert-deftest ebox-window-size-change-defers-and-resamples-latest-geometry () + "The hook records one frame intent and leaves geometry/render work to dispatch." + (let* ((buffer (generate-new-buffer " *ebox-viewport-event*")) + (window (selected-window)) + (old-buffer (window-buffer window)) + (width 240) (height 20) (samples 0) calls) + (unwind-protect + (progn + (set-window-buffer window buffer) + (ebox-surface-test--with-viewport-task + (cl-letf (((symbol-function 'ebox--buffer-render-state) + (lambda (_) '(:viewport-width 100 :viewport-height 10))) + ((symbol-function 'ebox-surface-buffer-mounted-p) + (lambda (_) t)) + ((symbol-function 'ebox-surface--window-content-width) + (lambda (_) (cl-incf samples) width)) + ((symbol-function 'window-body-height) + (lambda (&rest _) height)) + ((symbol-function 'ebox-rerender-buffer-with-context) + (lambda (&rest args) (push args calls)))) + (dotimes (_ 20) (ebox--window-size-change (selected-frame))) + (should-not calls) + (should (= samples 0)) + (should (= (length scheduled) 1)) + (should (equal ebox--window-size-change-pending-frames + (list (selected-frame)))) + (setq width 430 height 31) + (dispatch) + (should (equal calls (list (list buffer 430 31)))) + (should (= samples 1)) + (should-not scheduled) + (should-not ebox--window-size-change-pending-frames) + (should-not ebox--window-size-change-timer)))) + (set-window-buffer window old-buffer) + (kill-buffer buffer)))) + (ert-deftest ebox-window-size-change-publishes-every-visible-viewport () - "Continuous frame changes publish each sampled viewport immediately." + "Separate event-loop batches publish each current visible viewport." (let* ((buffer (generate-new-buffer " *ebox-window-size-change*")) (window (selected-window)) (old-buffer (window-buffer window)) @@ -451,7 +514,8 @@ (unwind-protect (progn (set-window-buffer window buffer) - (cl-letf (((symbol-function 'ebox--buffer-render-state) + (ebox-surface-test--with-viewport-task + (cl-letf (((symbol-function 'ebox--buffer-render-state) (lambda (_buffer) '(:viewport-width 100 :viewport-height 10))) ((symbol-function 'ebox-surface-buffer-mounted-p) @@ -460,13 +524,10 @@ (lambda (_window) (pop sampled-widths))) ((symbol-function 'ebox-rerender-buffer-with-context) (lambda (target width height) - (push (list target width height) calls))) - ((symbol-function 'run-at-time) - (lambda (&rest _) - (ert-fail "viewport delivery must not schedule a timer"))) - (noninteractive nil)) + (push (list target width height) calls)))) (dotimes (_index 20) - (ebox--window-size-change (selected-frame))) + (ebox--window-size-change (selected-frame)) + (dispatch)) (should (= (length calls) 20)) (should (equal (mapcar #'cadr (nreverse (copy-sequence calls))) @@ -474,12 +535,12 @@ (should (cl-every (lambda (call) (and (eq (car call) buffer) (= (nth 2 call) sampled-height))) - calls)))) + calls))))) (set-window-buffer window old-buffer) (kill-buffer buffer)))) -(ert-deftest ebox-window-size-change-rejects-reentrant-publication () - "A viewport commit cannot recursively enter the global size hook." +(ert-deftest ebox-window-size-change-defers-reentrant-notifications () + "Nested notifications become a later batch without recursive publication." (let* ((buffer (generate-new-buffer " *ebox-reentrant-window-size*")) (window (selected-window)) (old-buffer (window-buffer window)) @@ -488,7 +549,8 @@ (unwind-protect (progn (set-window-buffer window buffer) - (cl-letf (((symbol-function 'ebox--buffer-render-state) + (ebox-surface-test--with-viewport-task + (cl-letf (((symbol-function 'ebox--buffer-render-state) (lambda (_buffer) '(:viewport-width 100 :viewport-height 10))) ((symbol-function 'ebox-surface-buffer-mounted-p) @@ -500,11 +562,18 @@ ((symbol-function 'ebox-rerender-buffer-with-context) (lambda (&rest arguments) (push arguments calls) - (ebox--window-size-change (selected-frame)))) - (noninteractive nil)) - (ebox--window-size-change (selected-frame))) - (should (= (length calls) 1)) - (should (equal sampled-widths '(440)))) + (when (= (length calls) 1) + (ebox--window-size-change (selected-frame)))))) + (ebox--window-size-change (selected-frame)) + (dispatch) + (should (= (length calls) 1)) + (should (equal sampled-widths '(440))) + (should-not ebox--window-size-change-in-progress) + (dispatch) + (should (= (length calls) 2)) + (should-not sampled-widths) + (should-not scheduled) + (should-not ebox--window-size-change-timer)))) (set-window-buffer window old-buffer) (kill-buffer buffer)))) @@ -518,7 +587,8 @@ (unwind-protect (progn (set-window-buffer window buffer) - (cl-letf (((symbol-function 'ebox--buffer-render-state) + (ebox-surface-test--with-viewport-task + (cl-letf (((symbol-function 'ebox--buffer-render-state) (lambda (_buffer) '(:viewport-width 100 :viewport-height 10))) ((symbol-function 'ebox-surface-buffer-mounted-p) @@ -530,12 +600,15 @@ ((symbol-function 'ebox-rerender-buffer-with-context) (lambda (&rest arguments) (push arguments calls) - (when fail (error "viewport update failed")))) - (noninteractive nil)) - (should-error (ebox--window-size-change (selected-frame))) + (when fail (error "viewport update failed"))))) + (ebox--window-size-change (selected-frame)) + (should-error (dispatch)) (should-not ebox--window-size-change-in-progress) + (should-not ebox--window-size-change-timer) + (should-not scheduled) (setq fail nil) - (ebox--window-size-change (selected-frame))) + (ebox--window-size-change (selected-frame)) + (dispatch))) (should (= (length calls) 2))) (set-window-buffer window old-buffer) (kill-buffer buffer)))) @@ -543,15 +616,17 @@ (ert-deftest ebox-window-size-change-follows-canonical-display-frame () "A stale frame cannot overwrite a surface owned by another live frame." (let ((buffer (generate-new-buffer " *ebox-canonical-frame*")) + (canonical-frame 'stale-frame) calls) (unwind-protect - (cl-letf (((symbol-function 'frame-live-p) (lambda (_frame) t)) + (ebox-surface-test--with-viewport-task + (cl-letf (((symbol-function 'frame-live-p) (lambda (_frame) t)) ((symbol-function 'window-list) (lambda (&rest _) '(event-window))) ((symbol-function 'window-live-p) (lambda (_window) t)) ((symbol-function 'window-buffer) (lambda (_window) buffer)) ((symbol-function 'window-frame) - (lambda (_window) 'canonical-frame)) + (lambda (_window) canonical-frame)) ((symbol-function 'ebox-surface--buffer-display-window) (lambda (_buffer) 'canonical-window)) ((symbol-function 'ebox-surface-buffer-mounted-p) @@ -564,14 +639,152 @@ ((symbol-function 'window-body-height) (lambda (&rest _) 20)) ((symbol-function 'ebox-rerender-buffer-with-context) - (lambda (&rest arguments) (push arguments calls))) - (noninteractive nil)) + (lambda (&rest arguments) (push arguments calls)))) (ebox--window-size-change 'stale-frame) + (setq canonical-frame 'canonical-frame) + (dispatch) (should-not calls) (ebox--window-size-change 'canonical-frame) - (should (equal calls (list (list buffer 420 20))))) + (dispatch) + (should (equal calls (list (list buffer 420 20)))))) (kill-buffer buffer)))) +(ert-deftest ebox-window-size-change-discards-obsolete-intents () + "Dead frames, killed buffers and unmounted surfaces do no deferred work." + (dolist (retired '(frame buffer surface)) + (let ((buffer (generate-new-buffer " *ebox-retired-viewport*")) + (live t) (mounted t) calls) + (unwind-protect + (ebox-surface-test--with-viewport-task + (cl-letf (((symbol-function 'frame-live-p) (lambda (_) live))) + (ebox--window-size-change 'frame)) + ;; Real buffer disposal must see Emacs's real window functions. + ;; Install the synthetic display topology only for dispatch. + (pcase retired + ('frame (setq live nil)) + ('buffer (kill-buffer buffer)) + ('surface (setq mounted nil))) + (cl-letf (((symbol-function 'frame-live-p) (lambda (_) live)) + ((symbol-function 'window-list) (lambda (&rest _) '(window))) + ((symbol-function 'window-live-p) (lambda (window) (eq window 'window))) + ((symbol-function 'window-buffer) (lambda (_) buffer)) + ((symbol-function 'window-frame) (lambda (_) 'frame)) + ((symbol-function 'ebox-surface--buffer-display-window) + (lambda (_) 'window)) + ((symbol-function 'ebox-surface-buffer-mounted-p) + (lambda (_) mounted)) + ((symbol-function 'ebox--buffer-render-state) + (lambda (_) '(:viewport-width 100 :viewport-height 10))) + ((symbol-function 'ebox-surface--window-content-width) + (lambda (_) 420)) + ((symbol-function 'window-body-height) (lambda (&rest _) 20)) + ((symbol-function 'ebox-rerender-buffer-with-context) + (lambda (&rest args) (push args calls)))) + (dispatch) + (should-not calls) + (should-not scheduled) + (should-not ebox--window-size-change-timer) + (should-not ebox--window-size-change-pending-frames))) + (when (buffer-live-p buffer) (kill-buffer buffer)))))) + +(ert-deftest ebox-window-size-change-error-preserves-other-frame-work () + "One frame failure cannot drop other frames or retry itself automatically." + (ebox-surface-test--with-viewport-task + (let (calls) + (cl-letf (((symbol-function 'frame-live-p) (lambda (_) t)) + ((symbol-function 'ebox--window-size-change-sync-frame) + (lambda (frame) + (should-not inhibit-quit) + (push frame calls) + (when (eq frame 'first) + (ebox--window-size-change 'nested) + (should-not scheduled) + (error "First frame failed"))))) + (ebox--window-size-change 'first) + (ebox--window-size-change 'second) + (should-error (dispatch)) + (should (equal (reverse calls) '(first second))) + (should-not ebox--window-size-change-in-progress) + (should (= (length scheduled) 1)) + (dispatch) + (should (equal (reverse calls) '(first second nested))) + (should-not scheduled) + (should-not ebox--window-size-change-timer) + (should-not ebox--window-size-change-pending-frames))))) + +(ert-deftest ebox-window-size-change-quit-restores-unstarted-work () + "Quit escapes the task while cleanup keeps only unstarted or new intents." + (ebox-surface-test--with-viewport-task + (let (calls aborted) + (cl-letf (((symbol-function 'frame-live-p) (lambda (_) t)) + ((symbol-function 'ebox--window-size-change-sync-frame) + (lambda (frame) + (should-not inhibit-quit) + (push frame calls) + (when (eq frame 'first) + (ebox--window-size-change 'nested) + (signal 'quit nil))))) + (ebox--window-size-change 'first) + (ebox--window-size-change 'second) + (condition-case nil (dispatch) (quit (setq aborted t))) + (should aborted) + (should (equal calls '(first))) + (should-not ebox--window-size-change-in-progress) + (should (= (length scheduled) 1)) + (dispatch) + (should (= (length calls) 3)) + (should (memq 'second calls)) + (should (memq 'nested calls)) + (should-not scheduled) + (should-not ebox--window-size-change-timer) + (should-not ebox--window-size-change-pending-frames))))) + +(ert-deftest ebox-window-size-change-preserves-synchronous-public-update () + "A direct publication completes synchronously and makes a queued intent a no-op." + (let* ((buffer (generate-new-buffer " *ebox-direct-viewport*")) + (window (selected-window)) + (old-buffer (window-buffer window)) + (ebox-viewport-width 180) (ebox-viewport-height 6) + (ebox-runtime-idle-prewarm nil) + (ebox-runtime-idle-reflow-cache-prewarm nil) + (map (make-sparse-keymap)) + (text (propertize "Open" 'keymap map 'help-echo "Open item")) + (input (ebox-build `(box :width (viewport) :height (viewport-height) + (text ,text))))) + (define-key map [mouse-1] #'ignore) + (unwind-protect + (cl-letf (((symbol-function 'ebox-native-reflow-layout-ready-p) + (lambda () nil))) + (set-window-buffer window buffer) + (ebox-render-to-buffer buffer input) + (ebox-surface-test--with-viewport-task + (cl-letf (((symbol-function 'ebox-surface--window-content-width) + (lambda (_) 420)) + ((symbol-function 'window-body-height) (lambda (&rest _) 20))) + (let* ((surface (plist-get (ebox--buffer-render-state buffer) :surface)) + (revision (tp-surface-revision surface))) + (ebox--window-size-change (selected-frame)) + ;; Only notification requires the simulated interactive hook. + ;; Run the public API in this test process's batch context. + (let ((noninteractive t)) + (ebox-rerender-buffer-with-context buffer 420 20)) + (should (= (tp-surface-revision surface) (1+ revision))) + (should (= (plist-get (ebox--buffer-render-state buffer) + :viewport-width) 420)) + (should (= (plist-get (ebox--buffer-render-state buffer) + :viewport-height) 20)) + (let ((output (with-current-buffer buffer (buffer-string)))) + (dispatch) + (should (= (tp-surface-revision surface) (1+ revision))) + (should (equal-including-properties + output (with-current-buffer buffer (buffer-string)))) + (should (eq (lookup-key (get-text-property 0 'keymap output) + [mouse-1]) #'ignore)) + (should (equal (get-text-property 0 'help-echo output) "Open item"))) + (should-not scheduled))))) + (set-window-buffer window old-buffer) + (when (buffer-live-p buffer) (kill-buffer buffer))))) + (defun ebox-surface-test--hash-fingerprint (table) "Return a stable content fingerprint for hash TABLE. The fingerprint checks entries rather than only table identity, so a failed