Fix Merge two subtitles: keep ASSA styles and split tracks by layer#11257
Merged
Conversation
When merging two subtitles to Advanced SubStation Alpha, the result lost its styling: the grid showed only a single style and no paragraph carried a style. Two causes in MainViewModel.ShowToolsMergeTwoSubtitles: - The ASSA format was selected after SetSubtitles built the row view models, so SubtitleLineViewModel never copied the style (Extra) into Style (it only does so for AdvancedSubStationAlpha). Set the format before building the rows. - The merged header (Style1/Style2) was never transferred to _subtitle, so only the default header style survived. Carry the header over, like the other tools do. Also put each merged track on its own layer (0 and 1) so the two streams do not collide/stack against each other when shown at the same time. The top/bottom positioning still comes from the per-style alignment (an8/an2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes ASSA style/header loss when using Tools → Merge two subtitles. The output format is now selected before building row view models so per-paragraph styles are copied, the merged header (with Style1/Style2) is propagated to _subtitle, and each track is placed on its own layer to prevent visual collision.
Changes:
- Reorder
SetSubtitleFormat/SetSubtitlesand copyResultSubtitle.Headerto_subtitleinShowToolsMergeTwoSubtitles. - Assign distinct layers (0 and 1) to merged paragraphs in
BuildAssaMerge.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ui/Features/Main/MainViewModel.cs | Set format before building rows; carry merged header to _subtitle. |
| src/ui/Features/Tools/MergeTwoSubtitles/MergeTwoSubtitlesViewModel.cs | Assign per-track layers (0/1) to avoid overlap. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@
Problem
When using Tools → Merge two subtitles with the Advanced SubStation Alpha output, the merged result lost its styling after being loaded into the main grid: only a single style appeared and no paragraph carried a style, so top/bottom positioning could not work.
Root cause
In
MainViewModel.ShowToolsMergeTwoSubtitles:SetSubtitlesbuilt the row view models.SubtitleLineViewModelonly copies the style (Extra) into itsStyleproperty when the active format isAdvancedSubStationAlpha, so the rows ended up with an emptyStyleandToParagraphwrote no style on save.Style1/Style2) was never transferred to_subtitle, so only the default header style survived. The other merge tools already do_subtitle.Header = result.ResultSubtitle.Header; this one did not.The merge logic itself (
MergeTwoSubtitlesViewModel.BuildAssaMerge) was verified to produce correct output (two styles withan8/an2alignment, events referencing the right style) — the loss happened only on consumption.Fix
_subtitle.0and1) so the two streams do not collide/stack against each other when shown at the same time. Vertical placement still comes from the per-style alignment (an8top /an2bottom).Testing
🤖 Generated with Claude Code
@