From 65c799dc3f57c2cd327e0b402ab6e61b88392318 Mon Sep 17 00:00:00 2001 From: Daneel Date: Fri, 27 Feb 2026 16:56:05 +0100 Subject: [PATCH] =?UTF-8?q?patches:=20review=20fixes=20=E2=80=94=20specpdl?= =?UTF-8?q?=20protection,=20overlay=5Fmodiff=20tracking,=20binary=20search?= =?UTF-8?q?,=20enum=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M1: accessibilityRangeForPosition uses specpdl unwind protection for block_input/unblock_input (consistent with all other methods). M2: Track BUF_OVERLAY_MODIFF in ensureTextCache — overlay-only changes (timer-based completion highlight) now invalidate the text cache. M3: Detect narrowing/widening by comparing cachedTextStart vs BUF_BEGV. m1: Binary search (O(log n)) for visible runs in both accessibilityIndexForCharpos and charposForAccessibilityIndex. m3: Add EmacsAXSpanTypeNone = -1 to enum instead of (EmacsAXSpanType)-1 cast. m5: Add TODO comment in ns_ax_mode_line_text about non-CHAR_GLYPH limitation. README: Remove resolved overlay_modiff limitation, document binary search and narrowing detection, update architecture section. --- ...oundsForRange-for-macOS-Zoom-cursor-.patch | 101 +++++++++++------- patches/README.txt | 23 ++-- 2 files changed, 75 insertions(+), 49 deletions(-) diff --git a/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch b/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch index 3f5c70c..0b81d9f 100644 --- a/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch +++ b/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch @@ -88,7 +88,7 @@ Lisp calls, read only immutable NSString and scalar cache). etc/NEWS | 11 + src/nsterm.h | 108 ++ src/nsterm.m | 2870 +++++++++++++++++++++++++++++++++++++++++++++++--- - 3 files changed, 2960 insertions(+), 149 deletions(-) + 3 files changed, 2987 insertions(+), 149 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index 7367e3cc..0e4480ad 100644 @@ -160,6 +160,7 @@ index 7c1ee4cf..542e7d59 100644 +} +@property (nonatomic, retain) NSString *cachedText; +@property (nonatomic, assign) ptrdiff_t cachedTextModiff; ++@property (nonatomic, assign) ptrdiff_t cachedOverlayModiff; +@property (nonatomic, assign) ptrdiff_t cachedTextStart; +@property (nonatomic, assign) ptrdiff_t cachedModiff; +@property (nonatomic, assign) ptrdiff_t cachedPoint; @@ -181,6 +182,7 @@ index 7c1ee4cf..542e7d59 100644 +/* Span types for interactive AX child elements. */ +typedef NS_ENUM (NSInteger, EmacsAXSpanType) +{ ++ EmacsAXSpanTypeNone = -1, + EmacsAXSpanTypeButton = 0, + EmacsAXSpanTypeLink = 1, + EmacsAXSpanTypeCompletionItem = 2, @@ -496,6 +498,10 @@ index 932d209f..ea2de6f2 100644 + +/* ---- Helper: extract mode line text from glyph rows ---- */ + ++/* TODO: Only CHAR_GLYPH characters (>= 32) are extracted. Image ++ glyphs, stretch glyphs, and composed glyphs are silently skipped. ++ Mode lines using icon fonts (e.g. doom-modeline with nerd-font) ++ will produce incomplete accessibility text. */ +static NSString * +ns_ax_mode_line_text (struct window *w) { @@ -1018,7 +1024,7 @@ index 932d209f..ea2de6f2 100644 + while (pos < vis_end) + { + Lisp_Object plist = Ftext_properties_at (make_fixnum (pos), buf_obj); -+ EmacsAXSpanType span_type = (EmacsAXSpanType) -1; ++ EmacsAXSpanType span_type = EmacsAXSpanTypeNone; + Lisp_Object limit_prop = Qnil; + + if (!NILP (Fplist_get (plist, Qns_ax_widget, Qnil))) @@ -1070,7 +1076,7 @@ index 932d209f..ea2de6f2 100644 + } + } + -+ if ((NSInteger) span_type == -1) ++ if (span_type == EmacsAXSpanTypeNone) + { + /* Skip to the next position where any interactive property + changes. Try each scannable property in turn and take @@ -1379,6 +1385,7 @@ index 932d209f..ea2de6f2 100644 +@implementation EmacsAccessibilityBuffer +@synthesize cachedText; +@synthesize cachedTextModiff; ++@synthesize cachedOverlayModiff; +@synthesize cachedTextStart; +@synthesize cachedModiff; +@synthesize cachedPoint; @@ -1433,13 +1440,17 @@ index 932d209f..ea2de6f2 100644 + return; + + ptrdiff_t modiff = BUF_MODIFF (b); ++ ptrdiff_t overlay_modiff = BUF_OVERLAY_MODIFF (b); + ptrdiff_t pt = BUF_PT (b); + NSUInteger textLen = cachedText ? [cachedText length] : 0; -+ /* TODO: Also track BUF_OVERLAY_MODIFF to catch overlay-only ++ /* Track both BUF_MODIFF and BUF_OVERLAY_MODIFF. Overlay-only + changes (e.g., timer-based completion highlight move without -+ point change). Currently, overlay changes without text edits -+ are detected only when point also moves. */ ++ text edit) bump overlay_modiff but not modiff. Also detect ++ narrowing/widening which changes BUF_BEGV without bumping ++ either modiff counter. */ + if (cachedText && cachedTextModiff == modiff ++ && cachedOverlayModiff == overlay_modiff ++ && cachedTextStart == BUF_BEGV (b) + && pt >= cachedTextStart + && (textLen == 0 + || [self accessibilityIndexForCharpos:pt] <= textLen)) @@ -1455,6 +1466,7 @@ index 932d209f..ea2de6f2 100644 + [cachedText release]; + cachedText = [text retain]; + cachedTextModiff = modiff; ++ cachedOverlayModiff = overlay_modiff; + cachedTextStart = start; + + if (visibleRuns) @@ -1474,22 +1486,31 @@ index 932d209f..ea2de6f2 100644 + thread invalidates the text cache concurrently. */ + @synchronized (self) + { -+ for (NSUInteger i = 0; i < visibleRunCount; i++) ++ if (visibleRunCount == 0) ++ return 0; ++ ++ /* Binary search: runs are sorted by charpos (ascending). Find the ++ run whose [charpos, charpos+length) range contains the target, ++ or the nearest run after an invisible gap. O(log n) instead of ++ O(n) — matters for org-mode with many folded sections. */ ++ NSUInteger lo = 0, hi = visibleRunCount; ++ while (lo < hi) + { -+ ns_ax_visible_run *r = &visibleRuns[i]; -+ if (charpos >= r->charpos && charpos < r->charpos + r->length) ++ NSUInteger mid = lo + (hi - lo) / 2; ++ ns_ax_visible_run *r = &visibleRuns[mid]; ++ if (charpos < r->charpos) ++ hi = mid; ++ else if (charpos >= r->charpos + r->length) ++ lo = mid + 1; ++ else + { -+ /* Compute UTF-16 delta inside this run directly from cachedText -+ (an NSString built on the main thread) — no Lisp calls needed. */ ++ /* Found: charpos is inside this run. Compute UTF-16 delta ++ directly from cachedText — no Lisp calls needed. */ + NSUInteger chars_in = (NSUInteger)(charpos - r->charpos); + if (chars_in == 0 || !cachedText) + return r->ax_start; -+ /* ax_start + UTF-16 units for the first chars_in chars of the run. */ + NSUInteger run_end_ax = r->ax_start + r->ax_length; + NSUInteger scan = r->ax_start; -+ /* Each visible Emacs char maps to 1 or 2 UTF-16 units. -+ Walk the NSString using rangeOfComposedCharacterSequenceAtIndex -+ which handles surrogates correctly. */ + for (NSUInteger c = 0; c < chars_in && scan < run_end_ax; c++) + { + NSRange seq = [cachedText @@ -1498,18 +1519,12 @@ index 932d209f..ea2de6f2 100644 + } + return (scan <= run_end_ax) ? scan : run_end_ax; + } -+ /* If charpos falls in an invisible gap before the next run, -+ map it to the start of the next visible run. */ -+ if (charpos < r->charpos) -+ return r->ax_start; + } -+ /* Past end — return total length. */ -+ if (visibleRunCount > 0) -+ { -+ ns_ax_visible_run *last = &visibleRuns[visibleRunCount - 1]; -+ return last->ax_start + last->ax_length; -+ } -+ return 0; ++ /* charpos falls in an invisible gap or past the end. */ ++ if (lo < visibleRunCount) ++ return visibleRuns[lo].ax_start; ++ ns_ax_visible_run *last = &visibleRuns[visibleRunCount - 1]; ++ return last->ax_start + last->ax_length; + } /* @synchronized */ +} + @@ -1521,17 +1536,25 @@ index 932d209f..ea2de6f2 100644 + /* May be called from AX server thread — synchronize. */ + @synchronized (self) + { -+ for (NSUInteger i = 0; i < visibleRunCount; i++) ++ if (visibleRunCount == 0) ++ return cachedTextStart; ++ ++ /* Binary search: runs are sorted by ax_start (ascending). */ ++ NSUInteger lo = 0, hi = visibleRunCount; ++ while (lo < hi) + { -+ ns_ax_visible_run *r = &visibleRuns[i]; -+ if (ax_idx >= r->ax_start -+ && ax_idx < r->ax_start + r->ax_length) ++ NSUInteger mid = lo + (hi - lo) / 2; ++ ns_ax_visible_run *r = &visibleRuns[mid]; ++ if (ax_idx < r->ax_start) ++ hi = mid; ++ else if (ax_idx >= r->ax_start + r->ax_length) ++ lo = mid + 1; ++ else + { ++ /* Found: ax_idx is inside this run. Walk composed character ++ sequences to count Emacs characters up to ax_idx. */ + if (!cachedText) + return r->charpos; -+ -+ /* Walk forward through NSString composed character sequences to -+ count Emacs characters (= composed sequences) up to ax_idx. */ + NSUInteger scan = r->ax_start; + ptrdiff_t cp = r->charpos; + while (scan < ax_idx) @@ -1545,7 +1568,7 @@ index 932d209f..ea2de6f2 100644 + } + } + /* Past end — return last charpos. */ -+ if (visibleRunCount > 0) ++ if (lo > 0) + { + ns_ax_visible_run *last = &visibleRuns[visibleRunCount - 1]; + return last->charpos + last->length; @@ -2078,7 +2101,11 @@ index 932d209f..ea2de6f2 100644 + return NSMakeRange (0, 0); + + /* Block input to prevent concurrent redisplay from modifying the -+ glyph matrix while we traverse it. */ ++ glyph matrix while we traverse it. Use specpdl unwind protection ++ so block_input is always matched by unblock_input, even if ++ ensureTextCache triggers a Lisp signal (longjmp). */ ++ specpdl_ref count = SPECPDL_INDEX (); ++ record_unwind_protect_void (unblock_input); + block_input (); + + /* Find the glyph row at this y coordinate. */ @@ -2101,7 +2128,7 @@ index 932d209f..ea2de6f2 100644 + + if (!hit_row) + { -+ unblock_input (); ++ unbind_to (count, Qnil); + return NSMakeRange (0, 0); + } + @@ -2131,7 +2158,7 @@ index 932d209f..ea2de6f2 100644 + if (cachedText && ax_idx > [cachedText length]) + ax_idx = [cachedText length]; + -+ unblock_input (); ++ unbind_to (count, Qnil); + return NSMakeRange (ax_idx, 1); +} + diff --git a/patches/README.txt b/patches/README.txt index cd8914f..f5f8319 100644 --- a/patches/README.txt +++ b/patches/README.txt @@ -77,7 +77,8 @@ ARCHITECTURE text range, line/index/range conversions, frame-for-range, range-for-position, and insertion-point-line-number. - Maintains a text cache (cachedText / visibleRuns) keyed on - BUF_MODIFF. The cache is the single source of truth for all + BUF_MODIFF, BUF_OVERLAY_MODIFF, and BUF_BEGV (narrowing). + The cache is the single source of truth for all index-to-charpos and charpos-to-index mappings. - Detects buffer edits (modiff change), cursor movement (point change), and mark changes, and posts the appropriate @@ -282,14 +283,19 @@ TEXT CACHE AND VISIBLE RUNS non-contiguous visible segments. The mapping array is stored in the EmacsAccessibilityBuffer ivar `visibleRuns' (C array, xmalloc'd). - Index mapping (charpos <-> ax_index) does a linear scan of the run - array. Within a run, UTF-16 unit counting uses + Index mapping (charpos <-> ax_index) uses binary search over the + sorted run array — O(log n) per lookup. Within a run, UTF-16 unit + counting uses rangeOfComposedCharacterSequenceAtIndex: to handle surrogate pairs (emoji, rare CJK) correctly -- one Emacs character may occupy 2 UTF-16 units. - Cache invalidation is triggered whenever BUF_MODIFF changes - (ensureTextCache compares cachedTextModiff). The cache is also + Cache invalidation is triggered whenever BUF_MODIFF or + BUF_OVERLAY_MODIFF changes (ensureTextCache compares both + cachedTextModiff and cachedOverlayModiff). Additionally, + narrowing/widening is detected by comparing cachedTextStart + against BUF_BEGV — these operations change the visible region + without bumping either modiff counter. The cache is also invalidated when the window tree is rebuilt. NS_AX_TEXT_CAP = 100,000 UTF-16 units (~200 KB) caps total exposure; buffers larger than ~50,000 lines are truncated for accessibility purposes. VoiceOver @@ -499,13 +505,6 @@ KEY DESIGN DECISIONS KNOWN LIMITATIONS ----------------- - - BUF_OVERLAY_MODIFF is not tracked. Overlay changes (e.g. moving - the completions-highlight overlay via Tab without changing buffer - text) do not bump BUF_MODIFF, so the text cache is not invalidated. - The notification logic detects point changes (cachedPoint) which - covers the common case, but overlay-only changes with a stationary - point would be missed. A future fix would compare overlay_modiff. - - Interactive span scan uses Fnext_single_property_change across multiple properties to skip non-interactive regions in bulk, but still visits every property-change boundary. For buffers with