From 8e76d2341c00e63d2920eab6fd94ae5ff45124d3 Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Wed, 14 Oct 2026 12:50:44 -0400 Subject: [PATCH] Fix panning sounds after scrolling. --- .../Arch-Linux/cthulhu-git/PKGBUILD | 2 +- internal/app/document_review.go | 20 ++++ internal/app/linked_headings_native_test.go | 70 ++++++++++++ internal/atspi/adapter.go | 22 ++-- internal/atspi/adapter_capture.c | 2 +- internal/atspi/adapter_document_native.go | 51 +++++++++ internal/atspi/adapter_native.h | 1 + internal/atspi/adapter_snapshot_native.go | 14 ++- internal/atspi/adapter_tree_native.go | 3 + internal/atspi/adapter_tree_test.go | 22 ++++ internal/review/document_sound_test.go | 103 ++++++++++++++++++ internal/review/document_worker.go | 40 +++++++ internal/review/worker_contract.go | 1 + tests/browser-linked-headings.html | 4 + 14 files changed, 338 insertions(+), 17 deletions(-) create mode 100644 internal/review/document_sound_test.go diff --git a/distro-packages/Arch-Linux/cthulhu-git/PKGBUILD b/distro-packages/Arch-Linux/cthulhu-git/PKGBUILD index 763c80a..0687ca2 100644 --- a/distro-packages/Arch-Linux/cthulhu-git/PKGBUILD +++ b/distro-packages/Arch-Linux/cthulhu-git/PKGBUILD @@ -4,7 +4,7 @@ pkgbase=cthulhu-git pkgname=cthulhu-git _pkgname=cthulhu -pkgver=2026.10.14.r5.g6613237 +pkgver=2026.10.14.r6.gb2f558c pkgrel=1 pkgdesc="Desktop-agnostic screen reader with speech, Braille, and Tentacle support" url="https://git.stormux.org/storm/cthulhu" diff --git a/internal/app/document_review.go b/internal/app/document_review.go index 7db0ba1..a3aa8ad 100644 --- a/internal/app/document_review.go +++ b/internal/app/document_review.go @@ -320,6 +320,7 @@ func presentDocumentResult(logger *slog.Logger, speech speechEnqueuer, result re "source_path", result.DocumentPosition.Object.ID().Path, "virtual_caret_offset", result.DocumentPosition.Offset, "caret_sync_error", result.CaretErr, + "sound_geometry_error", result.DocumentSoundErr, "braille_line_available", result.DocumentBrailleLine != nil, "braille_read_error", result.DocumentBrailleErr) } @@ -406,6 +407,25 @@ func presentDocumentResult(logger *slog.Logger, speech speechEnqueuer, result re fallbackText = fallbackSpeech.Text } } + if len(cues) > 0 && logger.Enabled(context.Background(), slog.LevelDebug) { + // Record the captured geometry actually used for cue generation. + // Linked headings use the link's geometry independently of the heading. + attributes := []any{ + "bus", result.DocumentPosition.Snapshot.BusName, + "source_path", result.DocumentPosition.Snapshot.Path, + "role", result.DocumentPosition.Snapshot.Role, + "spatialize_object_sounds", options.SpatializeObjectSounds, + "sound_geometry", result.DocumentPosition.Snapshot.SoundGeometry, + "cues", cues, + } + if link := result.DocumentPosition.LinkedHeading; link != nil { + attributes = append(attributes, + "linked_heading_bus", link.BusName, + "linked_heading_path", link.Path, + "linked_heading_sound_geometry", link.SoundGeometry) + } + logger.Debug("document navigation sound positioning", attributes...) + } role = result.DocumentPosition.Snapshot.Role if result.DocumentWrapped { if result.DocumentBackwards { diff --git a/internal/app/linked_headings_native_test.go b/internal/app/linked_headings_native_test.go index 1cc9fbc..2d7f115 100644 --- a/internal/app/linked_headings_native_test.go +++ b/internal/app/linked_headings_native_test.go @@ -11,6 +11,7 @@ import ( "time" "git.stormux.org/storm/cthulhu/internal/atspi" + "git.stormux.org/storm/cthulhu/internal/presentation" "git.stormux.org/storm/cthulhu/internal/review" ) @@ -73,6 +74,9 @@ func TestNativeLinkedHeadings(t *testing.T) { t.Fatal(ctx.Err()) } } + t.Run("sound geometry after scrolling", func(t *testing.T) { + testLinkedHeadingSoundGeometry(t, ctx, adapter, focus) + }) for _, tc := range []struct { level int name, title string @@ -142,6 +146,72 @@ func TestNativeLinkedHeadings(t *testing.T) { } } +func testLinkedHeadingSoundGeometry(t *testing.T, ctx context.Context, adapter *atspi.Adapter, focus atspi.ObjectID) { + t.Helper() + tree, err := adapter.CaptureDocument(ctx, focus) + if err != nil { + t.Fatal(err) + } + cursor, err := review.NewDocumentCursor(tree) + if err != nil { + t.Fatal(err) + } + if _, err := cursor.SyncFocus(ctx, focus); err != nil { + t.Fatal(err) + } + cold, err := cursor.Move(ctx, review.DocumentHeading, 6, false) + if err != nil || !cold.Moved || cold.Position.LinkedHeading == nil { + t.Fatalf("cold offscreen heading = %+v, %v", cold, err) + } + if cold.Position.LinkedHeading.SoundGeometry.ObjectKnown { + t.Fatal("fixture heading must start offscreen without captured sound geometry") + } + worker, err := review.NewWorker(adapter, 2*time.Second) + if err != nil { + t.Fatal(err) + } + worker.SyncFocus(focus) + if err := worker.Start(ctx); err != nil { + t.Fatal(err) + } + defer worker.Stop(context.Background()) + for _, step := range []struct { + backwards bool + name string + }{ + {false, "Offscreen first topic"}, + {false, "Offscreen second topic"}, + {true, "Offscreen first topic"}, + } { + if err := worker.MoveDocument(review.DocumentHeading, 6, step.backwards); err != nil { + t.Fatal(err) + } + select { + case result := <-worker.Results(): + if result.Err != nil || result.CaretErr != nil || result.DocumentSoundErr != nil || + !result.Moved || result.DocumentPosition.Snapshot.Name != step.name { + t.Fatalf("scrolled heading result = %+v", result) + } + link := result.DocumentPosition.LinkedHeading + if link == nil || !link.SoundGeometry.ObjectKnown || !link.SoundGeometry.ReferenceKnown { + t.Fatalf("scrolled link lacks fresh geometry: %+v", link) + } + if !result.DocumentPosition.Snapshot.SoundGeometry.ObjectKnown { + t.Fatal("scrolled heading lacks fresh geometry") + } + cues := presentation.SoundCues(*link, presentation.Options{ + EnableSound: true, PlaySoundForRole: true, SpatializeObjectSounds: true, + }) + if len(cues) != 1 || cues[0].Pan >= -0.3 { + t.Fatalf("left-column link lost leftward sound: %+v", cues) + } + t.Logf("%s after scroll: bounds=%+v pan=%g", step.name, link.SoundGeometry.Object, cues[0].Pan) + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + } +} + func linkedHeadingFixtureFocus(tree *atspi.AccessibleTree) atspi.ObjectID { root, err := tree.Root() if err != nil { diff --git a/internal/atspi/adapter.go b/internal/atspi/adapter.go index 431833c..d0cb815 100644 --- a/internal/atspi/adapter.go +++ b/internal/atspi/adapter.go @@ -170,19 +170,21 @@ type nativeDocumentWindow struct { } type treeCaptureResult struct { - item TransferItem - actions ActionSet - point PointReview - ocr OCRGeometry - attributes CharacterAttributes - content DocumentContent - tree *AccessibleTree - owner DocumentContext - caret DocumentCaretText - err error + soundGeometry [2]SoundGeometry + item TransferItem + actions ActionSet + point PointReview + ocr OCRGeometry + attributes CharacterAttributes + content DocumentContent + tree *AccessibleTree + owner DocumentContext + caret DocumentCaretText + err error } type treeCaptureRequest struct { + soundObjects [2]ObjectReference transferItem bool listActions bool actionTarget *ActionTarget diff --git a/internal/atspi/adapter_capture.c b/internal/atspi/adapter_capture.c index 4bc847b..f1dd73e 100644 --- a/internal/atspi/adapter_capture.c +++ b/internal/atspi/adapter_capture.c @@ -125,7 +125,7 @@ static gboolean cthulhu_capture_sound_rect(AtspiAccessible *source, gint *x, gin return TRUE; } -static void cthulhu_capture_sound_geometry(AtspiAccessible *source, cthulhu_snapshot *snapshot) { +void cthulhu_capture_sound_geometry(AtspiAccessible *source, cthulhu_snapshot *snapshot) { snapshot->objectGeometryKnown = cthulhu_capture_sound_rect( source, &snapshot->objectX, &snapshot->objectY, &snapshot->objectWidth, &snapshot->objectHeight); diff --git a/internal/atspi/adapter_document_native.go b/internal/atspi/adapter_document_native.go index 491bbe9..cd01d44 100644 --- a/internal/atspi/adapter_document_native.go +++ b/internal/atspi/adapter_document_native.go @@ -83,6 +83,57 @@ func (adapter *Adapter) SetDocumentPosition(ctx context.Context, position Object return err } +// CaptureDocumentSoundGeometry refreshes only the presented object and its +// optional linked heading target, after navigation has scrolled them into view. +// It never changes the cached document semantics or retains new native handles. +func (adapter *Adapter) CaptureDocumentSoundGeometry(ctx context.Context, object, link ObjectReference) ([2]SoundGeometry, error) { + if !object.Valid() { + return [2]SoundGeometry{}, ErrStaleObject + } + if !adapter.captureSound.Load() { + return [2]SoundGeometry{}, nil + } + result, err := adapter.documentOperation(ctx, treeCaptureRequest{soundObjects: [2]ObjectReference{object, link}}) + return result.soundGeometry, err +} + +func (adapter *Adapter) captureNativeDocumentSoundGeometry(ctx context.Context, objects [2]ObjectReference) ([2]SoundGeometry, error) { + var geometry [2]SoundGeometry + if !adapter.captureSound.Load() { + return geometry, nil + } + adapter.treeMutex.Lock() + tree := adapter.documentTree + adapter.treeMutex.Unlock() + if tree == nil { + return geometry, ErrStaleObject + } + for index, object := range objects { + if !object.Valid() { + continue + } + if err := ctx.Err(); err != nil { + return [2]SoundGeometry{}, err + } + if _, err := tree.Resolve(object); err != nil { + return [2]SoundGeometry{}, err + } + source := adapter.nativeDocument[object.ID()] + if source == nil { + return [2]SoundGeometry{}, ErrStaleObject + } + // SHOWING can still reflect the pre-scroll viewport in libatspi's cache. + C.atspi_accessible_clear_cache_single(source) + var captured C.cthulhu_snapshot + C.cthulhu_capture_sound_geometry(source, &captured) + geometry[index] = soundGeometryFromNative(captured) + } + if err := ctx.Err(); err != nil { + return [2]SoundGeometry{}, err + } + return geometry, nil +} + // SetDocumentCaretPosition also places the browser caret at the virtual browse // offset, so native Tab traversal starts from the announced location. func (adapter *Adapter) SetDocumentCaretPosition(ctx context.Context, position ObjectReference, focus ObjectID, offset int) error { diff --git a/internal/atspi/adapter_native.h b/internal/atspi/adapter_native.h index 4f56963..bdb1630 100644 --- a/internal/atspi/adapter_native.h +++ b/internal/atspi/adapter_native.h @@ -88,6 +88,7 @@ cthulhu_snapshot cthulhu_capture_snapshot(AtspiAccessible *source, gboolean capt gboolean captureDocumentMetadata, gboolean captureSoundGeometry, gboolean captureApplicationName); +void cthulhu_capture_sound_geometry(AtspiAccessible *source, cthulhu_snapshot *snapshot); gchar *cthulhu_collect_notification_text(AtspiAccessible *source); gchar *cthulhu_collect_simple_prompt_text(AtspiAccessible *source); gchar *cthulhu_radio_group_name(AtspiAccessible *source); diff --git a/internal/atspi/adapter_snapshot_native.go b/internal/atspi/adapter_snapshot_native.go index a85f2aa..144a1fa 100644 --- a/internal/atspi/adapter_snapshot_native.go +++ b/internal/atspi/adapter_snapshot_native.go @@ -158,11 +158,7 @@ func snapshotFromNativeWithOptions(source *C.AtspiAccessible, text, caretCharact snapshot.MinimumIncrement = float64(captured.minimumIncrement) snapshot.ValueText = sanitizeSnapshotField(C.GoString(captured.valueText)) snapshot.TextBeforeCaret = sanitizeTextContext(C.GoString(captured.textBeforeCaret)) - snapshot.SoundGeometry = SoundGeometry{ - Object: Rectangle{X: int32(captured.objectX), Y: int32(captured.objectY), Width: int32(captured.objectWidth), Height: int32(captured.objectHeight)}, - Reference: Rectangle{X: int32(captured.referenceX), Y: int32(captured.referenceY), Width: int32(captured.referenceWidth), Height: int32(captured.referenceHeight)}, - ObjectKnown: captured.objectGeometryKnown != 0, ReferenceKnown: captured.referenceGeometryKnown != 0, - } + snapshot.SoundGeometry = soundGeometryFromNative(captured) C.g_free(C.gpointer(captured.applicationName)) C.g_free(C.gpointer(captured.name)) C.g_free(C.gpointer(captured.role)) @@ -177,6 +173,14 @@ func snapshotFromNativeWithOptions(source *C.AtspiAccessible, text, caretCharact return snapshot } +func soundGeometryFromNative(captured C.cthulhu_snapshot) SoundGeometry { + return SoundGeometry{ + Object: Rectangle{X: int32(captured.objectX), Y: int32(captured.objectY), Width: int32(captured.objectWidth), Height: int32(captured.objectHeight)}, + Reference: Rectangle{X: int32(captured.referenceX), Y: int32(captured.referenceY), Width: int32(captured.referenceWidth), Height: int32(captured.referenceHeight)}, + ObjectKnown: captured.objectGeometryKnown != 0, ReferenceKnown: captured.referenceGeometryKnown != 0, + } +} + func nativeObjectID(source *C.AtspiAccessible) ObjectID { return NewObjectID( C.GoString(C.cthulhu_accessible_bus_name(source)), diff --git a/internal/atspi/adapter_tree_native.go b/internal/atspi/adapter_tree_native.go index 3fdbdf8..dbffce8 100644 --- a/internal/atspi/adapter_tree_native.go +++ b/internal/atspi/adapter_tree_native.go @@ -376,6 +376,9 @@ func cthulhuAtspiTreeCaptureCallback(userData unsafe.Pointer) { } else if request.activate { err := adapter.activateNativeDocumentObject(request.ctx, request.position, request.reviewFocus) request.result <- treeCaptureResult{err: err} + } else if request.soundObjects[0].Valid() { + geometry, err := adapter.captureNativeDocumentSoundGeometry(request.ctx, request.soundObjects) + request.result <- treeCaptureResult{soundGeometry: geometry, err: err} } else if request.position.Valid() { err := adapter.setNativeDocumentPosition(request.ctx, request.position, request.reviewFocus, request.positionOffset, request.usePositionOffset) request.result <- treeCaptureResult{err: err} diff --git a/internal/atspi/adapter_tree_test.go b/internal/atspi/adapter_tree_test.go index 4bd9845..c2669f4 100644 --- a/internal/atspi/adapter_tree_test.go +++ b/internal/atspi/adapter_tree_test.go @@ -99,6 +99,28 @@ func TestCaptureDesktopRejectsUnavailableAdapterAndCanceledContext(t *testing.T) } } +func TestDocumentSoundGeometryHonorsCaptureGateAndCancellation(t *testing.T) { + adapter := New() + tree := testTree(t) + object, _ := tree.Reference(testID("/first")) + adapter.SetCaptureSoundGeometry(false) + geometry, err := adapter.CaptureDocumentSoundGeometry(context.Background(), object, ObjectReference{}) + if err != nil || geometry != [2]SoundGeometry{} { + t.Fatalf("disabled sound geometry accessed the stopped adapter: %+v, %v", geometry, err) + } + adapter.SetCaptureSoundGeometry(true) + canceled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := adapter.CaptureDocumentSoundGeometry(canceled, object, ObjectReference{}); !errors.Is(err, context.Canceled) { + t.Fatalf("canceled geometry capture error = %v", err) + } + adapter.documentTree = tree + tree.Invalidate(object.ID()) + if _, err := adapter.captureNativeDocumentSoundGeometry(context.Background(), [2]ObjectReference{object}); !errors.Is(err, ErrStaleObject) { + t.Fatalf("stale geometry reference accepted: %v", err) + } +} + func TestDocumentCaptureTreeIsIndependentAndInvalidated(t *testing.T) { for _, eventType := range []string{ChildrenAddedEvent, DefunctStateEvent, DocumentReloadEvent, "shutdown"} { t.Run(eventType, func(t *testing.T) { diff --git a/internal/review/document_sound_test.go b/internal/review/document_sound_test.go new file mode 100644 index 0000000..ec7d4c1 --- /dev/null +++ b/internal/review/document_sound_test.go @@ -0,0 +1,103 @@ +// Copyright (c) 2026 Storm Dragon +// Authors: Storm Dragon, Angus Kola +// SPDX-License-Identifier: GPL-3.0-or-later + +package review + +import ( + "context" + "errors" + "reflect" + "testing" + "time" + + "git.stormux.org/storm/cthulhu/internal/atspi" +) + +type soundGeometryNavigator struct { + *fakeDocumentNavigator + geometry [2]atspi.SoundGeometry + err error + reads int + t *testing.T +} + +func (navigator *soundGeometryNavigator) CaptureDocumentSoundGeometry(_ context.Context, object, link atspi.ObjectReference) ([2]atspi.SoundGeometry, error) { + navigator.reads++ + select { + case placed := <-navigator.calls: + if placed.position != object || link.ID() != webID("/topic_link") { + navigator.t.Fatal("geometry refresh did not follow placement of the selected heading and link") + } + default: + navigator.t.Fatal("geometry read before scrolling the target") + } + return navigator.geometry, navigator.err +} + +func TestDocumentSoundGeometryRefreshPreservesSemanticsAndPlacement(t *testing.T) { + oldGeometry := atspi.SoundGeometry{ObjectKnown: true, Object: atspi.Rectangle{X: 900, Y: 3000, Width: 50, Height: 15}} + freshGeometry := atspi.SoundGeometry{ + ObjectKnown: true, ReferenceKnown: true, + Object: atspi.Rectangle{X: 100, Y: 300, Width: 50, Height: 15}, + Reference: atspi.Rectangle{Width: 1000, Height: 600}, + } + for _, name := range []string{"fresh", "unavailable", "refresh failed", "placement failed"} { + t.Run(name, func(t *testing.T) { + node := func(path, role string, children ...atspi.ObjectID) atspi.TreeNode { + snapshot := atspi.NewSnapshot(path, "Topic", role, "", "") + snapshot.BusName = webID(path).BusName + snapshot.SoundGeometry = oldGeometry + return atspi.TreeNode{Snapshot: snapshot, Children: children} + } + tree, err := atspi.NewAccessibleTree(webID("/doc"), []atspi.TreeNode{ + node("/doc", "document web", webID("/heading")), + node("/heading", "heading", webID("/topic_link")), + node("/topic_link", "link"), + }) + if err != nil { + t.Fatal(err) + } + cursor, _ := NewDocumentCursor(tree) + document, _ := tree.Root() + heading, _ := tree.Reference(webID("/heading")) + linkRef, _ := tree.Reference(webID("/topic_link")) + snapshot, _ := tree.Resolve(heading) + link, _ := tree.Resolve(linkRef) + navigator := &soundGeometryNavigator{fakeDocumentNavigator: newFakeDocumentNavigator(), t: t} + want := atspi.SoundGeometry{} + switch name { + case "fresh": + navigator.geometry = [2]atspi.SoundGeometry{freshGeometry, freshGeometry} + want = freshGeometry + case "refresh failed": + navigator.err = context.DeadlineExceeded + case "placement failed": + navigator.placementErr <- atspi.ErrStaleObject + want = oldGeometry + } + worker, _ := NewWorker(navigator, time.Second) + worker.SyncFocus(webID("/doc")) + result := worker.placeDocument(context.Background(), cursor, Result{ + Moved: true, DocumentEpoch: worker.DocumentEpoch(), DocumentFocus: webID("/doc"), + DocumentPosition: DocumentPosition{Document: document, Object: heading, Snapshot: snapshot, LinkedHeading: &link}, + }) + if result.Err != nil || !errors.Is(result.DocumentSoundErr, navigator.err) { + t.Fatalf("sound refresh affected navigation: %+v", result) + } + if result.DocumentPosition.Snapshot.SoundGeometry != want || result.DocumentPosition.LinkedHeading.SoundGeometry != want { + t.Fatalf("geometry after %s = %+v", name, result.DocumentPosition) + } + if link.SoundGeometry != oldGeometry { + t.Fatal("refresh mutated the previously captured link snapshot") + } + cached, _ := tree.Resolve(heading) + if !reflect.DeepEqual(cached, snapshot) { + t.Fatal("refresh changed cached document semantics or geometry") + } + if name == "placement failed" && navigator.reads != 0 { + t.Fatal("failed caret placement still queried sound geometry") + } + }) + } +} diff --git a/internal/review/document_worker.go b/internal/review/document_worker.go index b5dc852..fbbc79c 100644 --- a/internal/review/document_worker.go +++ b/internal/review/document_worker.go @@ -488,5 +488,45 @@ func (worker *Worker) placeDocument(ctx context.Context, cursor *DocumentCursor, deadline, cancel := context.WithTimeout(ctx, worker.captureTimeout) defer cancel() result.CaretErr = navigator.SetDocumentPosition(deadline, result.DocumentPosition.Object, focus) + if result.CaretErr == nil && worker.CurrentDocumentResult(result) { + result = worker.refreshDocumentSoundGeometry(deadline, cursor, result) + } + return result +} + +func (worker *Worker) refreshDocumentSoundGeometry(ctx context.Context, cursor *DocumentCursor, result Result) Result { + reader, ok := worker.nativeCapturerFor(result.DocumentFocus).(interface { + CaptureDocumentSoundGeometry(context.Context, atspi.ObjectReference, atspi.ObjectReference) ([2]atspi.SoundGeometry, error) + }) + if !ok { + return result + } + // Geometry belongs to this presentation, not the document's initial viewport. + // Failed refreshes must not reuse an old position after scrolling or resizing. + result.DocumentPosition.Snapshot.SoundGeometry = atspi.SoundGeometry{} + link := atspi.ObjectReference{} + if previous := result.DocumentPosition.LinkedHeading; previous != nil { + copy := *previous + copy.SoundGeometry = atspi.SoundGeometry{} + result.DocumentPosition.LinkedHeading = © + if cursor == nil { + result.DocumentSoundErr = atspi.ErrStaleObject + return result + } + var err error + link, err = cursor.tree.Reference(copy.ID()) + if err != nil { + result.DocumentSoundErr = err + return result + } + } + geometry, err := reader.CaptureDocumentSoundGeometry(ctx, result.DocumentPosition.Object, link) + result.DocumentSoundErr = err + if err == nil { + result.DocumentPosition.Snapshot.SoundGeometry = geometry[0] + if result.DocumentPosition.LinkedHeading != nil { + result.DocumentPosition.LinkedHeading.SoundGeometry = geometry[1] + } + } return result } diff --git a/internal/review/worker_contract.go b/internal/review/worker_contract.go index 9d54266..cdcfca8 100644 --- a/internal/review/worker_contract.go +++ b/internal/review/worker_contract.go @@ -77,6 +77,7 @@ type Result struct { DocumentCaretContext []atspi.Snapshot DocumentBrailleLine *atspi.DocumentCaretText DocumentBrailleErr error + DocumentSoundErr error DocumentBackwards bool DocumentLevel int DocumentWrapped bool diff --git a/tests/browser-linked-headings.html b/tests/browser-linked-headings.html index 435c021..6be5b96 100644 --- a/tests/browser-linked-headings.html +++ b/tests/browser-linked-headings.html @@ -11,4 +11,8 @@

Ordinary heading

Choose: First link and Second link

First destination

Second destination

+
Offscreen first topic
+

First sound destination

+
Offscreen second topic
+

Second sound destination