Skip to content

Perf: single-raise timing notifications + cheaper ASSA event-line parsing#12073

Merged
niksedk merged 2 commits into
mainfrom
perf/grid-notify-and-assa-parse
Jul 2, 2026
Merged

Perf: single-raise timing notifications + cheaper ASSA event-line parsing#12073
niksedk merged 2 commits into
mainfrom
perf/grid-notify-and-assa-parse

Conversation

@niksedk

@niksedk niksedk commented Jul 2, 2026

Copy link
Copy Markdown
Member

Two hot-path fixes from the second performance review round.

Subtitle line timing notifications (grid / waveform drags)

  • TextBackgroundBrush was raised from OnEndTimeChanged, OnDurationChanged, and UpdateDuration, but its getter depends only on Text (line length / pixel width / line count) — and it strips HTML and shapes text with HarfBuzz per evaluation. Waveform drags assign StartTime/EndTime per pointer move (60–120 Hz), so realized rows re-shaped their text up to ~3× per mouse event for no reason.
  • OnEndTimeChanged first called UpdateDuration (which raises CPS/WPM/brushes) and then raised the same set again — every end-time assignment notified twice.
  • OnGapChanged raised DurationBackgroundBrush, which doesn't read Gap.

UpdateDuration is now the single raise site for timing-derived properties (all timing mutations — OnStartTimeChanged, OnEndTimeChanged, SetTimes, SetStartTimeOnly — funnel through it). It also gains CpsBackgroundBrush/WpmBackgroundBrush, fixing a latent staleness bug: dragging the start edge changed CPS but never repainted the CPS/WPM cell backgrounds.

TextBackgroundBrush keeps its two correct raise sites: OnTextChanged and the settings-dependent refresh.

ASSA LoadSubtitle

  • Every event line was copied with ToLowerInvariant() just for a handful of prefix checks (format:, dialogue:, comment:) — for a 100k-line .ass that's 100k full-line lowercase copies, doubled by the IsMine pre-parse. Now uses StartsWith(..., OrdinalIgnoreCase); only the short format field list is lowercased.
  • The text-field rejoin (text += "," + part per comma in the dialogue text) is now a pooled StringBuilder.
  • Reuses the already-computed trimmedLine instead of three extra TrimStart() calls per line.

All 997 tests pass (libse 485, UI 512).

🤖 Generated with Claude Code

niksedk and others added 2 commits July 2, 2026 15:23
…g changes

TextBackgroundBrush depends only on Text, but was raised from
OnEndTimeChanged, OnDurationChanged and UpdateDuration - its getter
strips HTML and shapes the text with HarfBuzz, and waveform drags
assign times at pointer-move rate, so realized rows re-shaped text
several times per mouse event. OnEndTimeChanged also duplicated all
of UpdateDuration's raises, doubling every notification per EndTime
assignment. OnGapChanged raised DurationBackgroundBrush, which does
not read Gap.

Make UpdateDuration the single raise site for the timing-derived
properties and add CpsBackgroundBrush/WpmBackgroundBrush there, so
start-time drags now repaint them too (previously only end-time
changes did).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…title

Every event line was copied via ToLowerInvariant just for a few prefix
checks - use case-insensitive StartsWith instead and lowercase only the
short format field list. The text field rejoin used repeated string
concatenation per comma; use a pooled StringBuilder. Also reuse the
already-trimmed line instead of three extra TrimStart calls.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@niksedk niksedk merged commit 4b52793 into main Jul 2, 2026
1 of 3 checks passed
@niksedk niksedk deleted the perf/grid-notify-and-assa-parse branch July 2, 2026 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant