Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASS Fixes 4: Properly handle Margins, adjust for Padding Bug and manage Margins for Abs Positioning #404

Closed
wants to merge 2 commits into from

Conversation

softworkz
Copy link
Collaborator

SubtitleProviderSsaAss: Handle margins when absolute positioning
SubtitleProviderSsaAss: Reverse margins

@softworkz
Copy link
Collaborator Author

Explanations

WinUI Bug in Handling TimedTextPadding

I have seen that the margins from the defined styles (neither others) are not having the expected effect.
Eventually I was able to trace this down to a WinUI bug for which I have submitted a fix PR now: microsoft/microsoft-ui-xaml#9248

As long as this is not fixed, we need to workaround by switching Before/After and Start/End when using TimedTextPadding as done in this PR.

Style Margins and Dialogue Margins

The current code is ignoring margins from the style definition when only a single margin value is specified in a dialogue line.
This is incorrect, as the override needs to done for each value independently (so does AegiSub).

MarginV Handling

MarginV is a top margin for top-aligned blocks and a bottom margin for bottom-aligned blocks even though the Word doc tells this for style defined margins and otherwise for dialog lines, where it talks about bottom only - which is incorrect (according to AegiSub behavior).

Handling Margins in Case of Absolute Positioning

MarginV is unused in case of absolute posiioned elements, but MarginL and MarginR still need to be applied.
Unfortunately TimedTextRegions do not allow negative position values, so you already did the best of what is possible for region sizing and positioning. Only the horizontal margins were ignored.
Those are now applied in a way that it resembles the behavior of AegiSub (see comments in code).


// Now subtract the reduced width from the padding (which is meant to
// be applied to a 100% width box)
padding1.After -= (100 - extent.Width);
Copy link
Member

Choose a reason for hiding this comment

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

I do not really understand why the right margin needs to be reduced here (and below). But I guess you tested this with actual files, comparing with how Aegisub renders them, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I did. You can easily do for yourself like this:

In Aegisub, open any ASS file and then create a dummy video for it.
Detach the video window (just for ease).
Double-Click an event.
Then you get there:

image

By double-clicking, you can set the position.
By adding {\anX}, you can change the alignment.
And now you can play with different margins:

image

You'll see that Aegisub always calculates with a width of 100% and then it applies the L/R margins, but when left or right aligned, it adds both margins to the opposite side.

Since we cannot have a width of 100% but need to reduce it so the region fits into the video frame, we need to subtract this reduced width from the margins (because it's like as if we had already applied this reduced width as a margin).

Copy link
Member

@lukasf lukasf left a comment

Choose a reason for hiding this comment

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

Approved

@brabebhin brabebhin closed this Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants