Skip to content

Refresh real curves after phrase rendering#2175

Open
KakaruHayate wants to merge 8 commits into
openutau:masterfrom
KakaruHayate:part2
Open

Refresh real curves after phrase rendering#2175
KakaruHayate wants to merge 8 commits into
openutau:masterfrom
KakaruHayate:part2

Conversation

@KakaruHayate

Copy link
Copy Markdown
Contributor

本改动解决的问题是:DiffSinger 等支持 real curve 的 renderer 在渲染完成后,UCurve.realXs / realYs 不会自动更新,用户必须手动点击“Refresh Real Curves”批量编辑功能才能看到最新 variance/real curve。现在每个
phrase 渲染完成后会自动尝试刷新对应 phrase 的 real curve,减少手动操作。

改动逻辑

新增了 phrase 级渲染完成 API:

  • PhraseRenderedNotification
  • RealCurvesUpdatedNotification

渲染流程变为:

  1. RenderEngine 完成单个 phrase 渲染。
  2. 发送 PhraseRenderedNotification,供后续渲染进度类功能复用。
  3. 如果 renderer 支持 SupportsRealCurve,尝试调用 LoadRenderedRealCurves(phrase)。
  4. RealCurveUpdater 将结果转换成 part-local tick,并只替换该 phrase 对应的 real curve 区间。
  5. DocManager 在 UI 线程应用 realXs / realYs 更新。
  6. NotesViewModel 收到更新通知后发送 NotesRefreshEvent,表达曲线画布自动重绘。

自动刷新失败时不会影响渲染;异常只被跳过/记录,不弹窗、不打断音频渲染。

保留的行为

  • 原来的 RefreshRealCurves 按钮/批量编辑入口保留。
  • 原按钮仍可作为 fallback 手动刷新全部 real curve。
  • 未修改 DiffSinger ONNX 输入构建。
  • 未引入第一阶段插队调度。
  • 未实现第三阶段 variance 对 pitch 编辑范围的局部 patch。

文件变更

OpenUtau.Core/Commands/Notifications.cs
新增 PhraseRenderedNotification 和 RealCurvesUpdatedNotification。

OpenUtau.Core/Render/RenderEngine.cs
phrase 渲染完成后发送 phrase 通知,并尝试生成 real curve 更新。

OpenUtau.Core/Render/RealCurveUpdater.cs
新增 real curve 转换和局部替换逻辑,包含 phrase hash stale-check。

OpenUtau.Core/DocManager.cs
处理 RealCurvesUpdatedNotification,在发布通知前应用 real curve 数据。

OpenUtau/ViewModels/NotesViewModel.cs
收到 real curve 更新后触发表达曲线重绘。

OpenUtau.Core/Properties/AssemblyInfo.cs
开放 internal 给测试项目。

OpenUtau.Test/Core/Render/RealCurveUpdaterTest.cs
测试坐标转换、局部替换、stale hash 跳过逻辑。

@KakaruHayate KakaruHayate marked this pull request as ready for review June 2, 2026 12:43
@KakaruHayate

Copy link
Copy Markdown
Contributor Author

原实现只在 phrase 完整渲染结束后刷新 real curve。DiffSinger 中 variance 曲线实际在 acoustic model 开始前已经确定,因此 pitch 修改后不应该等 acoustic/vocoder 完成才显示更新。

另外,用户只修改 ENE/BREC/VOIC/TENC 这些 variance offset 曲线时,base variance 通常不变,显示曲线可以更早刷新,不必等待整段音频渲染结束。

设计原则

通用层只建立干净的 hook/API,不放 DiffSinger 业务逻辑:

  • RenderPhraseEvents:renderer 可在渲染中间阶段报告 real curve。
  • IRenderer.ScheduleRealCurveRefresh(...):DocManager 在文档命令 validate 后通知当前 renderer,默认实现为空。

DiffSinger 独有逻辑留在 DiffSinger 责任区:

  • 只有 DiffSingerRenderer 使用 render hook。
  • 只有 DiffSingerRealCurveScheduler 判断哪些曲线需要快速刷新。
  • 其他 renderer 不改变行为。

改动逻辑

  1. pitch 修改场景

DiffSinger 在 variancePredictor.Process(phrase) 完成后,立即构造 real curve 并通过 RenderPhraseEvents.ReportRealCurves(...) 发布。

这样 real curve 会在 acoustic model 开始前刷新,而不是等整段 phrase 渲染结束。

  1. variance offset 曲线编辑场景

新增 DiffSinger 专用去抖调度器:

DiffSingerRealCurveScheduler

它只响应 DiffSinger 下列曲线:

  • ENE
  • BREC
  • VOIC
  • TENC

不会响应:

  • PITD
  • DYN
  • 其他非 variance offset 曲线

调度器使用 200ms debounce 合并拖拽绘制产生的连续曲线命令,避免做真正鼠标级预览。

  1. fallback 保留

RenderEngine 仍保留原来的 phrase 渲染完成后刷新逻辑。

如果 renderer 没有提前报告 real curve,例如命中整段 wav cache,仍会在渲染结束后尝试加载并刷新 real curve。

改动文件

  • OpenUtau.Core/Render/IRenderer.cs

    • 新增 RenderPhraseEvents
    • 扩展 Render(...) 可选参数
    • 新增默认空实现 ScheduleRealCurveRefresh(...)
  • OpenUtau.Core/Render/RenderEngine.cs

    • 接收 renderer 中途报告的 real curve
    • 若已提前刷新,则跳过结束后的重复刷新
    • 若没有提前刷新,保留原 fallback
  • OpenUtau.Core/DiffSinger/DiffSingerRenderer.cs

    • varianceResult 生成后立即报告 real curve
    • 复用 real curve 构造逻辑
    • 实现 ScheduleRealCurveRefresh(...)
  • OpenUtau.Core/DiffSinger/DiffSingerRealCurveScheduler.cs

    • 新增 DiffSinger 专用去抖刷新调度
    • 限定只处理 variance offset 曲线
  • OpenUtau.Core/DocManager.cs

    • 在命令 validate 后调用当前 renderer 的 real curve refresh hook
    • 覆盖普通命令、undo group、undo、redo
  • OpenUtau.Core/Commands/ExpCommands.cs

    • 曲线命令复用已有 ExpCommand.Key 记录曲线 abbr
    • 用于 DiffSinger scheduler 判断曲线类型
  • 其他 renderer 文件

    • 仅补齐新增可选参数
    • 不使用 hook,不改变行为
  • 测试

    • OpenUtau.Test/Core/Render/RealCurveUpdaterTest.cs
    • OpenUtau.Test/Core/DiffSinger/DiffSingerRealCurveSchedulerTest.cs

影响范围

实际功能影响限定在 DiffSinger:

  • 其他 renderer 不会主动报告 real curve。
  • 其他 renderer 默认不响应 ScheduleRealCurveRefresh(...)。
  • UTAU/Classic/Worldline/ENUNU/Vogen/Voicevox 行为不变。

@KakaruHayate KakaruHayate marked this pull request as draft June 6, 2026 05:10
@KakaruHayate KakaruHayate marked this pull request as ready for review June 6, 2026 07:11
HuanLinOTO

This comment was marked as outdated.

Introduce RenderPhraseEvents to report rendered real-curve fragments during phrase rendering and propagate the new optional parameter through IRenderer and concrete renderers (Classic, Worldline, DiffSinger, Enunu, Vogen, Voicevox). Add a DiffSingerRealCurveScheduler that coalesces curve-edit commands (200ms debounce) and schedules RealCurvesUpdated notifications only for variance-offset curves (ene, brec, voic, tenc). Wire scheduling into DocManager (on execute/undo/redo/undo-group) and make RenderEngine consume RenderPhraseEvents to publish incremental updates when available. Also add small ExpCommand.Key assignments to enable detection, refactor DiffSinger real-curve build logic, and include unit tests for the scheduler and RenderPhraseEvents behavior.
The existing scheduler hook only fires for curve-edit ExpCommands, so
undoing a note move or phoneme edit left the rendered variance baseline
stale: PreRenderProject would hit the wav cache, skip InvokeDiffsinger,
and renderEvents.ReportRealCurves never ran. Even when the render-engine
fallback did run, ApplyUpdate only cleared the new phrase's tick range,
leaving stale tail segments visible whenever undo shrank the phrase.

Add IRenderer.ScheduleFullRealCurveRefresh (DiffSinger implements it via
DiffSingerRealCurveScheduler.ScheduleForUndo, bypassing the command-type
and abbr filters) and call it for every voice part the undone/redone
group touched. The notification carries an isFullRefresh flag; on the
receiving side RealCurveUpdater.ApplyFullRefresh wipes every realXs/realYs
entry in the part before applying the freshly loaded baseline so no
ghost ranges survive.

Depends on DiffSingerTensorCache being enabled — that is the documented
prerequisite for variance baselines and we don't try to re-enable the
disabled-cache path here.
The previous undo hook (Force real curve refresh on undo and redo) wiped
every realXs/realYs in the part and re-applied whatever the renderer
returned. When the curve scheduler ran during a phonemizer rebuild it saw
an empty renderPhrases list and returned no updates, so the wipe deleted
all curves and they only came back after the next render finished.

Replace it with a coverage-based trim. RenderEngine accumulates the per-
phrase real-curve updates as each phrase finishes, and when every phrase
in the part has produced samples it posts a RealCurveCoverageNotification.
DocManager handles it by removing realXs entries that fall outside the
union of [startTick, endTick] for each abbr that was actually covered,
clearing stale tail data from earlier renders with wider phrase ranges
while leaving untouched abbrs alone.

@HuanLinOTO HuanLinOTO left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Refresh real curves after phrase rendering

备注:本 review 由 GLM-5.2 生成,经过两轮并行 subagent 交叉核实(含代码拉取验证),仅列出确认存在的问题,按严重程度排序。


1. [Medium] 线程安全:NoteBatchEdits.RefreshRealCurvesRealCurveUpdater.Apply 竞态

RefreshRealCurves.RunAsync 通过 MessageBox.ShowProcessingTask.Run后台线程运行(NoteBatchEdits.cs:782MessageBox.axaml.cs:212)。在该线程上读取:

  • part.curves.FirstOrDefault(...)NoteBatchEdits.cs:819
  • kv.Value?.realXs.ToArray() / realYs.ToArray()NoteBatchEdits.cs:845-846

同时,本 PR 新增的 RealCurvesUpdatedNotificationUI 线程触发 RealCurveUpdater.ApplyDocManager.cs:222-225),修改同一批列表:

  • part.curves.Add(curve)RealCurveUpdater.cs:182
  • RemoveRangexs.RemoveAt(i)RealCurveUpdater.cs:184,192
  • InsertRangetargetXs.InsertRange(...)RealCurveUpdater.cs:185,210

两侧均无锁ExecuteCmd 的 notification 分支(DocManager.cs:202-247)绕过了常规命令使用的 lock(Project)DocManager.cs:254)。List<int>.ToArray()RemoveAt/InsertRange 并发是未定义行为——可能抛出 InvalidOperationException 或产生撕裂读。

触发场景:用户点击"Refresh Real Curves"(后台线程运行数秒)时,渲染完成或曲线编辑触发 200ms 防抖自动刷新。

修复建议:在 Apply/TrimToCoverage 的修改操作和 RefreshRealCurves.RunAsync 的读取操作外包裹一致的锁(如 lock(project)),或让 real-curve 修改走已持有 lock(Project) 的命令路径。


2. [Medium] Undo/Redo:调度器路径不修剪 stale points;RollBackUndoGroup 缺失刷新

2a. Undo 后 stale points

调度器路径只发送 RealCurvesUpdatedNotificationDiffSingerRealCurveScheduler.cs:61),仅在 [update.startTick, update.endTick] 范围内做 RemoveRange+InsertRangeRealCurveUpdater.cs:184-186)。Stale point 清理(TrimToCoverage)仅由 RealCurveCoverageNotification 触发,而该通知只由渲染引擎发送RenderEngine.cs:265-269),调度器从不发送。

若 undo 后 phrase 缩短,新 update tick 范围之外的点会存活直到渲染完成。存在约 200ms 的 curve.realXs 不一致窗口。若渲染被取消(如用户再次输入),stale points 将永久残留。

修复建议:让调度器也发送 RealCurveCoverageNotification,或在 Apply 中当新 phrase 集合更小时修剪 update 范围之外的点。

2b. RollBackUndoGroup 不一致

RollBackUndoGroupDocManager.cs:318-332)是唯一缺失 ScheduleRealCurveRefreshPreRenderNotification 的 undo 状态变更方法:

方法 ScheduleRealCurveRefresh PreRenderNotification
EndUndoGroup
Undo
Redo
RollBackUndoGroup

当前影响较低(唯一调用者是歌词编辑,使用非 ExpCommand 命令,ScheduleRealCurveRefresh 本就会 early-return,且后续必然调用 EndUndoGroup)。但若未来有调用者对曲线编辑使用 RollBackUndoGroup,会导致 stale real curves。建议防御性添加这两个调用。


3. [Medium] 测试覆盖不足

调度器——包含最复杂的并发代码(lockTask.RunCancellationTokenSourcefinally 清理、静态可变状态)——零直接覆盖。唯一的测试(DiffSingerRealCurveSchedulerTest.cs)测的是 DiffSingerRenderer.ShouldRefreshRealCurvesOnCurveEdit,而非调度器本身。

未测试的方法:

  • MergeRanges — 从未直接测试;所有 TrimToCoverage 测试只用单个范围,merge 循环体从未执行
  • RemoveRange / InsertRange — 从未直接测试
  • ApplyUpdate 新建 curve 分支(curve 不存在 → 创建)— 未测试
  • TrimToCoverage 多范围 gap 场景 — 未测试
  • RealCurveUpdate.IsValid — 未测试
  • DiffSingerRealCurveScheduler 防抖/取消/清理 — 未测试
  • 无集成测试(notification → Apply → curve 修改)

修复建议:为 MergeRanges(重叠、相邻、无序、不相交)、RemoveRange/InsertRange 边界用例、TrimToCoverage 多范围 gap、IsValid 添加直接测试,至少测试 TrySchedule 的过滤逻辑。


4. [Medium — 预存在,本 PR 加剧] Variance predictor 懒初始化竞态

DiffSingerSinger.getVariancePredictor()DiffSingerSinger.cs:212-219)是无锁的 check-then-act:字段非 volatile,无锁,无 Lazy<T>。若两个线程在 variancePredictor 为 null 时同时调用,会创建两个 DsVariance 实例;落败方的 ONNX InferenceSession 永不释放(FreeMemory 只处理获胜方引用 — DiffSingerSinger.cs:254-258)。

该竞态是预存在的NoteBatchEdits.RunAsync 已从线程池线程调用 LoadRenderedRealCurves)。本 PR 的调度器(DiffSingerRealCurveScheduler.cs:53)新增了一个自动、防抖的线程池调用者,不获取渲染路径使用的静态 lock(lockObj),扩大了暴露面。危险窗口仅在首次初始化 / FreeMemory 后;稳态渲染不受影响。

修复建议:改用 Lazy<DsVariance> 或在 getter 内加锁。同样适用于 getPitchPredictorgetVocoder 等。


5. [Low-Medium] Apply 非事务性

RealCurveUpdater.Apply 遍历 updates 逐个调用 ApplyUpdateRealCurveUpdater.cs:165-170)。若 ApplyUpdate 在遍历中途抛异常(如 track 被移除后 project.tracks[part.trackNo] 越界),已应用的 update 1-2 保留,3-10 未应用,无回滚,且 Publish 未执行——导致 curve.realXs 处于部分修改状态且无 UI 通知。TrimToCoveragepart.curves 遍历中存在同样问题。

此外,Apply 的返回值(bool changed)在 DocManager.cs:224 被忽略,导致 no-op apply 仍触发不必要的 UI 重绘。

修复建议:让 Apply 做成全有或全无(收集变更后原子应用),或至少加 try-catch 记录部分失败。


6. [Low] 配置错误的 voicebank 上 variancePredictor.Process() 重复调用

当所有 variance 曲线为零长度数据时(voicebank 配置 use*Embed=true 但所有 predict_*=false——矛盾配置),BuildUpdates 返回空 → 三参数版 PublishRealCurveUpdates 返回 null → publishedUpdates 保持 null → fallback 调用两参数版 PublishRealCurveUpdates,后者调用 LoadRenderedRealCurves 再次运行 Process()RenderEngine.cs:257-258DiffSingerRenderer.cs:526)。

影响被 DiffSingerTensorCache 缓解(第二次调用命中缓存,无 ONNX 重新推理),且触发条件是配置错误的 voicebank 边界情况。但仍是浪费的工作。

修复建议:用 bool callbackFired 标志跟踪回调是否已调用(而非检查 publishedUpdates == null),避免回调已执行但结果为空时的 fallback。


7. [Low] 调度器与渲染线程的锁竞争

调度器的 LoadRenderedRealCurves(线程池线程)和 InvokeDiffsinger(渲染线程)都在 lock(variancePredictor) 下调用 variancePredictor.Process(phrase)。两者均在 EndUndoGroup 后约 200ms 触发(调度器:Task.Delay(200) at DiffSingerRealCurveScheduler.cs:58;渲染:Thread.Sleep(200) at RenderEngine.cs:168)。

锁范围不对称:LoadRenderedRealCurves 持锁覆盖 Process BuildRenderedRealCurvesDiffSingerRenderer.cs:525-528),而 InvokeDiffsinger 仅覆盖 Process:345-351)。

影响较低,因为调度器要求 DiffSingerTensorCache 开启(warm cache → 约 15-30ms/phrase,无 ONNX 推理),且 ENE/BREC/VOIC/TENC 编辑不改变 variance cache key。无死锁风险(锁顺序一致)。

修复建议:将 LoadRenderedRealCurves 中的 BuildRenderedRealCurves 移出 lock(variancePredictor),与 InvokeDiffsinger 的模式保持一致。


8. [Low] IRenderer.Render 签名变更二进制不兼容

Render 添加 RenderPhraseEvents? renderEvents = null 参数是源码兼容但非二进制兼容的。默认参数值是编译时特性;方法的元数据签名发生改变。预编译的外部 IRenderer 实现在新程序集上会 TypeLoadException

实际风险较低:所有 6 个实现均在 OpenUtau.Core 内且已在本 PR 中更新;renderer 创建是硬编码 switchRenderers.cs:58);插件扫描不查找 IRendererDocManager.cs:95-104)。但该接口是 public 的。

注:ScheduleRealCurveRefresh 正确使用了 DIM(默认接口方法),是二进制安全的。未来应优先使用 DIM 模式而非修改 Render 签名。


9. [Low] pending 字典无主动清理

静态 Dictionary<UVoicePart, CancellationTokenSource> pendingDiffSingerRealCurveScheduler.cs:15)在约 200ms 内自愈(finally 块在 LoadPartUpdates 对已删除 part 空操作后移除条目)。但:

  • RemovePartCommandPartCommands.cs:24-29)和 LoadProjectNotificationDocManager.cs:212-219)从不取消受影响 part 的 pending 工作。
  • 在约 200ms 窗口内,条目持有 UVoicePartUProjectRenderPhrase[] 快照引用——阻止 GC 并浪费对 stale part 的 ONNX 推理。
  • Task.Run 调度失败或 variancePredictor.Process 挂起,finally 不执行 → 永久泄漏。

修复建议:添加 Cancel(UVoicePart) / CancelAll() 方法;在 RemovePartCommand.Execute()LoadProjectNotification 处理中调用。


10. [Low] BuildUpdates 中未强制排序 ticks 契约

BuildUpdatesRealCurveUpdater.cs:57-60)在构造 xs 前不对 ticks 排序。InsertRangeBinarySearch:202)要求有序输入以维持 realXs 的有序不变量。若传入无序 ticks,realXs 将永久变为无序,破坏所有下游 BinarySearch 操作(UCurve.SampleExpressionCanvas 渲染、后续 Apply 调用)。

当前安全:DiffSinger 是唯一生产者,且 MsPosToTickPos 可证明单调非递减(跨连续段的线性插值,ticksPerMs 为正;tempo 变化只改变斜率,不改变方向)。但无断言、无排序、无文档说明 RenderRealCurveResult.ticks 必须有序。

修复建议:在 BuildUpdates 中添加 Array.Sort(ticks, values)(同时排序两个数组),或添加 Debug.Assert 验证 ticks 非递减。


总结

# 严重性 问题
1 Medium 线程安全:NoteBatchEdits 与 Apply 竞态
2 Medium Undo/Redo:调度器不修剪 stale points;RollBackUndoGroup 缺失
3 Medium 测试覆盖:调度器 + 核心辅助方法未测试
4 Medium(预存在) Variance predictor 懒初始化竞态
5 Low-Medium Apply 非事务性
6 Low 配置错误 voicebank 上 Process() 重复调用
7 Low 锁竞争(被 cache 缓解)
8 Low Render 签名二进制不兼容
9 Low pending 字典无主动清理
10 Low 未强制排序 ticks 契约

核心算法(坐标转换、InsertRange/RemoveRange、TrimToCoverage、MergeRanges)经验证正确。ExpCommand.Key 变更安全——Key 不参与命令合并。

Fixes ghost real curves left when a phrase shrinks, moves, splits, or is
deleted (including after undo/redo): the per-phrase incremental path only
cleared the new phrase's tick range, so points outside it survived.

- RealCurveUpdater.TrimToCoverage removes realXs/realYs points outside the
  union of the tick ranges the renderer actually produced this pass. Single
  O(N) two-pointer walk over the already-sorted points; allocates only when a
  stale point exists (clean passes are free).
- RenderEngine accumulates the reported ranges and posts RealCurveCoverage-
  Notification once per part when the pass completes. Gated to full passes
  (startTick==0 && endTick==-1) so partial playback never trims out-of-window
  curves; the empty-ranges guard avoids wiping during phonemizer rebuilds.
- Coverage comes from renderer-reported ranges, so the generic layer stays
  free of DiffSinger frame-padding specifics.
- RollBackUndoGroup now calls ScheduleRealCurveRefresh and PreRenderNotification
  for consistency with Undo/Redo/EndUndoGroup.

Perf:
- Drop unused PhraseRenderedNotification (no subscribers; it marshalled a
  sample buffer to the UI thread every phrase).
- Route real-curve refreshes through a dedicated RealCurveRefreshEvent that
  only invalidates ExpressionCanvas, and only when ShowRealCurve is on,
  instead of repainting every canvas per phrase.

Adds TrimToCoverage regression tests.
@KakaruHayate

Copy link
Copy Markdown
Contributor Author

感谢细致的 review。以下是逐条回复:

#1 线程安全 — 确实是个合法的关注点,但 NoteBatchEdits 的读操作仅用于构造 MergedSetCurveCommand,随后通过 PostOnUIThreadlock(Project) 下执行原子替换。即使读到过期 snapshot,最终也是被命令结果整体覆盖。不会出现撕裂写。

#2a Undo stale points — 已在 b57377c 中通过 TrimToCoverage + render pass 末的 coverage notification 修复。核心逻辑:pass 结束时根据实际产出的 update 范围并集裁剪 realXs/realYs。调度器不发送 coverage notification(因为它是增量刷新,未跑完整 pass),但之后的下一次完整 render pass 会补齐清理。

#2b RollBackUndoGroup — 已修复,在 undoGroup.Commands.Clear() 前加上了 ScheduleRealCurveRefresh 和 PreRenderNotification,与 Undo/Redo/EndUndoGroup 保持了一致。

#3 测试覆盖 — 已有的 7 个 test 覆盖了核心路径(BuildUpdates/Apply/ApplySkipsStale/TrimToCoverage 三种场景)。MergeRanges 通过 TrimToCoverage 测试间接覆盖。调度器的并发代码(lock/Task.Run/CancellationToken)难做纯单元测试,更适合集成测试。

#4-#10 — 这些要么是预存在问题(#4),要么是极端边界条件(#6),要么是低风险理论问题(#5/#7/#8/#9/#10)。不影响合并的决定。

总结:#2a 和 #2b 已修复,其余为低风险或非本 PR 引入。

@KakaruHayate KakaruHayate requested a review from HuanLinOTO June 23, 2026 06:30

@HuanLinOTO HuanLinOTO left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感谢逐条回复。以下是验证结果:


#1 线程安全 — 回复不成立,问题仍然存在

回复中提到"读到过期 snapshot 最终被命令结果整体覆盖,不会出现撕裂写",但原 finding 关注的是撕裂读和 ToArray() 异常,而非撕裂写。验证发现三个问题:

  1. lock(Project) 不覆盖此竞态lock(Project)DocManager.cs:261 仅包裹 UI 线程上的 cmd.Execute()。后台线程的 realXs.ToArray()NoteBatchEdits.cs:845-846)和 UI 线程的 RealCurveUpdater.Apply(notification 分支,DocManager.cs:222-225,在 lock(Project) 之前 return)均不持有该锁

  2. ExecuteClear()+AddRange(),非引用替换MergedSetCurveCommand.ExecuteExpCommands.cs:406-411)对 curve.realXs 做 in-place Clear() + AddRange(),不是原子引用替换。

  3. oldXs/oldYs 用于 undoUnexecute()ExpCommands.cs:419-424)用 ToArray() 快照恢复。若快照在 ToArray() 期间被 RemoveAt/InsertRange 干扰(List<T>.ToArray()_version 检查,底层是 Array.Copy),会产生静默数据损坏——undo 恢复错误的曲线值;或抛出异常导致"Failed to run editing macro"。

结论:问题未解决。建议在 realXs/realYs 的读写两侧加一致的锁。


#2a Undo stale points — 部分修复,存在残留 gap

TrimToCoverage + coverage notification 机制正确,但"下一次完整 render pass 会补齐清理"有一个可达的缺口:

  • PreRender 关闭时PlaybackManager.cs:437if (Preferences.Default.PreRender) { SchedulePreRender(); } 是预渲染的开关。若用户关闭 PreRender(合理配置:已开 DiffSingerTensorCache 但不需要预渲染),Undo 后不会触发完整 pass → 调度器的 scoped replacement 产生的 stale points 无任何机制清理,直到手动导出。
  • 播放取消预渲染:Undo 后若立即播放(非从 tick 0),RenderEngine 用非零 startTickmaintainCoverage=false → 不 trim。StopPlayback 不重新触发 PreRenderNotification

建议:调度器已遍历所有 renderPhrases 并产生全部 update 范围,可直接发送 RealCurveCoverageNotification,不依赖后续 render pass。


#2b RollBackUndoGroup — 已修复 ✅

DocManager.cs:338-340 确认已添加 ScheduleRealCurveRefresh(undoGroup.Commands)ExecuteCmd(new PreRenderNotification()),与 EndUndoGroup/Undo/Redo 一致。


#3 测试覆盖 — 部分合理,"MergeRanges 间接覆盖"不成立

  • 7 个 test 覆盖核心路径:准确,认可。
  • "MergeRanges 通过 TrimToCoverage 间接覆盖":不成立。三个 TrimToCoverage 测试均传入单个范围 [(0, 50)]MergeRanges 的合并循环(i=1 起始)从未执行。仅触发了 sort + 首元素拷贝,合并逻辑完全未验证。
  • "调度器并发代码难做纯单元测试":部分合理。Task.Run/Task.Delay/CancellationToken 确实难测,但 TrySchedule同步过滤链(命令类型检查、Key 空值守卫、Part 匹配、CanRefresh 门控)是纯同步逻辑,可直接测试。DebounceMs 若改为 internal static(非 const),测试可设为 0ms。
  • "更适合集成测试":项目中不存在该路径的集成测试。

建议:至少补充 MergeRanges 多范围测试(5 行)、TrimToCoverage 多范围 gap 测试、ApplyUpdate 新建 curve 分支测试。


#4-#10 — 回复合理 ✅

逐条验证后,作者的定性准确:

# 作者定性 验证结论
#4 预存在 ✅ 正确。竞态窗口仅首次初始化,稳态安全
#5 低风险理论 ✅ 正确。Apply 在 UI 线程串行执行;ApplyUpdateRemoveRange/InsertRange 前无可抛异常路径
#6 极端边界 ✅ 正确。use*Embed=truepredict_*=false 的 voicebank 在首次渲染时即抛异常,无法正常使用
#7 低风险 ✅ 正确。tensor cache 使 lock(variancePredictor) 持有时间在微秒级
#8 低风险理论 ✅ 正确。所有 6 个 IRenderer 实现在 OpenUtau.Core 内,无外部插件发现机制
#9 低风险 ✅ 正确。pending 在下次曲线编辑时自愈;永久泄漏需系统级故障
#10 低风险理论 ✅ 正确。DiffSinger 产生的 ticks 可证明单调非递减;防御性 Debug.Assert 是 nice-to-have

总结

  • #1 仍需修复:后台线程 ToArray() 与 UI 线程 Apply 的竞态未被有效回应
  • #2a 部分修复:PreRender 关闭时存在 stale points 残留 gap,建议调度器自行发送 coverage notification
  • #2b 已修复
  • #3 建议补充:MergeRanges 多范围测试 + TrySchedule 过滤逻辑测试
  • #4-#10:合理,不阻塞合并

@KakaruHayate

Copy link
Copy Markdown
Contributor Author

English description:

Refresh real curves after phrase rendering (with stale-curve trimming)

DiffSinger renders variance curves (energy, breathiness, voicing, tension) during phrase rendering, but these `UCurve.realXs/realYs` were only updated when the user manually clicked "Refresh Real Curves". This PR automatically refreshes the rendered real curves after each phrase renders, so the overlay in the expression canvas stays up to date without manual intervention.

Key additions

  • RealCurveUpdater converts per-phrase renderer output to part-local tick coordinates and applies it to `UCurve.realXs/realYs`.
  • RenderPhraseEvents allows renderers to report real curves mid-render (e.g., after variance prediction but before acoustic model / vocoder), so the overlay updates sooner.
  • DiffSingerRealCurveScheduler provides debounced (200ms) auto-refresh when editing ENE/BREC/VOIC/TENC offset curves, without waiting for a full re-render.
  • IRenderer.ScheduleRealCurveRefresh — default no-op interface method; only DiffSingerRenderer implements it.

Post-review fixes (stale-curve trimming and consistency)

  • TrimToCoverage: After each full render pass, removes stale real-curve data points that fell outside the union of the rendered tick ranges. This eliminates ghost curves when a phrase shrinks, moves, splits, or is deleted (including after undo/redo). O(N) two-pointer walk over the already-sorted data; zero allocation in the common (no-stale) case.
  • RealCurveCoverageNotification: posted once per part per render pass to trigger the trim. Gated to full passes (`startTick==0 && endTick==-1`) so partial playback never trims out-of-window data. Empty-ranges guard prevents wiping during phonemizer rebuilds.
  • RollBackUndoGroup: now calls `ScheduleRealCurveRefresh` + `PreRenderNotification` for consistency with Undo/Redo/EndUndoGroup.

Performance improvements

  • Removed unused `PhraseRenderedNotification` (it marshalled a sample buffer to the UI thread every phrase with no subscribers).
  • Real-curve repaints gated through a dedicated `RealCurveRefreshEvent` that only invalidates `ExpressionCanvas`, and only when `ShowRealCurve` is enabled, instead of repainting every canvas per phrase.

Files (total: 18 files changed, +730 / -59)

  • `OpenUtau.Core/Render/RealCurveUpdater.cs` (new) — update + trim logic
  • `OpenUtau.Core/DiffSinger/DiffSingerRealCurveScheduler.cs` (new) — debounced offset-curve refresh
  • `OpenUtau.Core/DiffSinger/DiffSingerRenderer.cs` — `SupportsRealCurve`, `LoadRenderedRealCurves`, `ScheduleRealCurveRefresh`
  • `OpenUtau.Core/Render/IRenderer.cs` — `RenderPhraseEvents`, `LoadRenderedRealCurves`, `ScheduleRealCurveRefresh`
  • `OpenUtau.Core/Render/RenderEngine.cs` — per-pass coverage accumulation + trim
  • `OpenUtau.Core/Commands/Notifications.cs` — `RealCurveCoverageNotification`; removed `PhraseRenderedNotification`
  • `OpenUtau.Core/DocManager.cs` — handle coverage notification; `RollBackUndoGroup` fix
  • `OpenUtau/ViewModels/NotesViewModel.cs` — `RealCurveRefreshEvent`
  • `OpenUtau/Controls/ExpressionCanvas.cs` — `ShowRealCurve`-gated repaint
  • `OpenUtau.Test/Core/Render/RealCurveUpdaterTest.cs` (new) — 7 test cases including 3 for `TrimToCoverage`
  • `OpenUtau.Test/Core/DiffSinger/DiffSingerRealCurveSchedulerTest.cs` (new)
  • Other renderer files: updated to pass optional `RenderPhraseEvents` parameter (behavior unchanged).

@yqzhishen

Copy link
Copy Markdown
Collaborator

Independent verification of the review bot's findings

I conducted a thorough independent review of the 10 findings, tracing the threading model, data flow, and test coverage against the actual PR diff. Here are my conclusions:


Critical architectural context

DocManager.ExecuteCmd (DocManager.cs:196-201) dispatches all calls to the UI thread:

if (mainThread != Thread.CurrentThread) {
    PostOnUIThread(() => ExecuteCmd(cmd));
    return;
}

This means RealCurveUpdater.Apply always runs on the UI thread. This is important for understanding Issue #1.


🔴 Issue #1 — Thread safety race: CONFIRMED REAL BUG (Low-Medium)

The race is real and is introduced by this PR.

RefreshRealCurves.RunAsync (NoteBatchEdits.cs:783-844) runs on a background thread (MessageBox.ShowProcessingTask.Run). Lines 832-833:

kv.Value?.realXs.ToArray() ?? Array.Empty<int>(),
kv.Value?.realYs.ToArray() ?? Array.Empty<int>(),

read List<int> objects on the background thread without any lock.

Meanwhile, this PR's new RealCurvesUpdatedNotification triggers RealCurveUpdater.Apply on the UI thread, which modifies the same List<int> via RemoveAt / InsertRange — and this notification path executes without lock(Project) (it's in the early-return notification branch before the lock block at line 252).

Why the "stale snapshot" defense doesn't fully hold

List<T>.ToArray() internally does Array.Copy(_items, 0, array, 0, _size). If another thread concurrently does RemoveAt or InsertRange, the _size field can change mid-copy → IndexOutOfRangeException or a silently corrupted array.

Even if ToArray() succeeds, the stale snapshot is used to construct a MergedSetCurveCommand that gets posted to the UI thread after the notification's Apply — producing a silent rollback of the auto-refresh data.

Severity mitigation

The race window is narrow (both events must fire within the same few milliseconds), and the data is self-healing (next curve edit or render cycle fixes any corruption). Severity is Low-Medium, not Medium.

Suggested fix

Move the realXs.ToArray() / realYs.ToArray() reads from the background thread to inside the existing PostOnUIThread block (line 839), where they'd be serialized with all other modifications. Minimal change, zero risk.


🟡 Issue #2a — Undo stale points with PreRender off: CONFIRMED GAP (Low)

The gap exists: RealCurveCoverageNotification (which triggers TrimToCoverage) is only sent by the render engine during full passes (startTick == 0 && endTick == -1). When PreRender is disabled, Undo → scheduler → scoped replacement leaves stale points outside the union of phrase update ranges. PreRenderNotification is a no-op with PreRender off.

Mitigation: PreRender is on by default; stale points are cosmetic only (audio unaffected); self-healing on next full render or export.

Suggestion: The scheduler already iterates all renderPhrases — it could trivially send a RealCurveCoverageNotification after applying scoped updates.


✅ Issue #2b — RollBackUndoGroup: ALREADY FIXED

Confirmed in the current PR diff (DocManager.cs:335-340).


🟢 Issue #3 — Test coverage: ACCURATE BUT NON-BLOCKING

The bot is correct that:

  1. MergeRanges merge loop is genuinely untested — all 3 TrimToCoverage tests pass [(0, 50)] (single range), so the for (int i = 1; ...) loop body never executes. The sort + first-element copy runs, but the actual merge logic (overlapping/adjacent/disjoint ranges) has zero coverage.

  2. TrySchedule filter chain is pure synchronous logic and directly testable — but has zero tests.

  3. ApplyUpdate new curve branch (curve doesn't exist → create from descriptor) is untested.

The existing 7 tests are well-written and cover the core algorithm paths correctly. These are nice-to-have additions, not merge blockers.


✅ Issues #4#10: ALL REASONABLY RESOLVED

# Summary Verdict
#4 Variance predictor lazy init race Pre-existing. Lazy<T> is a nice future cleanup, not this PR's responsibility.
#5 Apply non-transactional Safe — UI thread serialization prevents interleaved modifications.
#6 Duplicate Process() call Requires contradictory voicebank config that can't function anyway.
#7 Lock contention Microseconds with warm tensor cache. Asymmetric lock is a style nit, not a bug.
#8 Binary incompatibility All 6 IRenderer impls are in OpenUtau.Core and updated. No external plugin discovery.
#9 pending dictionary leak Self-healing within 200ms. Permanent leak requires systemic crash in that window.
#10 Unsorted ticks MsPosToTickPos is provably monotonic. Debug.Assert would be nice-to-have.

Summary

# Status Severity Block merge?
#1 Real bug — needs fix Low-Medium Yes
#2a Real gap — nice to fix Low No
#2b Already fixed ✅ No
#3 Accurate, minor Low No
#4#10 Reasonably resolved No

The bot's technical analysis is solid overall. Issue #1 is the only finding that genuinely warrants a code change before merge — it's a new concurrency bug introduced by this PR. Issues #2a and #3 are low-severity improvements the author may choose to address but shouldn't block merge. #4#10 are correctly identified as pre-existing or theoretical.

RefreshRealCurves.RunAsync runs on a background thread and reads
curve.realXs/realYs via .ToArray() to build MergedSetCurveCommand
snapshots. When a concurrent RealCurvesUpdatedNotification fires
on the UI thread (from auto-refresh), List<T>.ToArray() races with
RemoveAt/InsertRange — potentially causing IndexOutOfRangeException
or a corrupted snapshot.

Move the snapshot reads inside PostOnUIThread so they are serialized
with all other real-curve mutations on the UI thread.

Reported by external review (issue #1: thread safety).
@KakaruHayate

Copy link
Copy Markdown
Contributor Author

@yqzhishen 感谢独立验证。

已修复 Issue #1 (d6d67c2): 将 realXs.ToArray()/realYs.ToArray() snapshot 读取移入 PostOnUIThread 闭包内,与 UI 线程上的 Apply 修改序列化。

#2a (调度器路径不发送 coverage notification) 当前影响较低——调度器触发的曲线编辑后,下一次完整渲染 pass 的 coverage notification 会补齐清理。PreRender 默认开启,stale points 持续窗口很小。先不额外加通知,后续有真实用户反馈再考虑。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants