Skip to content

Fix overlapping subtitles with negative line numbers#3151

Open
johnpc wants to merge 1 commit intoandroidx:mainfrom
johnpc:fix-overlapping-subtitles
Open

Fix overlapping subtitles with negative line numbers#3151
johnpc wants to merge 1 commit intoandroidx:mainfrom
johnpc:fix-overlapping-subtitles

Conversation

@johnpc
Copy link
Copy Markdown

@johnpc johnpc commented Apr 1, 2026

Summary

Fixes overlapping subtitle rendering when multiple WebVTT cues have overlapping timestamps and are assigned negative line numbers.

Problem

When multiple WebVTT cues overlap in time, WebvttSubtitle.getCues() correctly assigns them negative line numbers (-1, -2, -3, etc.) to stack them from the bottom. However, SubtitlePainter calculates each cue's vertical position using only firstLineHeight (the height of a single text line), not accounting for the actual multi-line height of preceding cues.

This causes cues to visually overlap when:

  1. Multiple cues are displayed simultaneously
  2. Any of the cues wrap to multiple lines

This is particularly common in anime subtitles where overlapping timestamps are used for effects like showing both dialogue and on-screen text.

Solution

  1. Added getLastDrawnTextHeight() to SubtitlePainter to expose the actual rendered height of a text cue
  2. Modified CanvasSubtitleOutput.dispatchDraw() to track cumulative height of bottom-stacked cues (those with negative LINE_TYPE_NUMBER) and adjust the drawing boundary for subsequent cues

Testing

Added unit tests for:

  • SubtitlePainterTest: Tests for getLastDrawnTextHeight() with various cue types
  • CanvasSubtitleOutputTest: Tests for rendering overlapping cues without visual overlap

Fixes #2237

@google-cla
Copy link
Copy Markdown

google-cla bot commented Apr 1, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@icbaker
Copy link
Copy Markdown
Collaborator

icbaker commented Apr 2, 2026

Thanks for the PR - is there a reason you are specifically handling negative line numbers? I think the same problem can occur with positive line numbers too.

Would you be able to handle the positive line numbers case in this PR as well? I think a list of cues could have cues with both positive & negative line numbers (some at the top, some at the bottom of the screen, at the same time) - so handling both cases separately would probably be best. I think it makes sense to handle both cases at the same time so that we don't accidentally over-fit the solution to only one of the two cases.

Please also rebase the PR to target the main branch as that's where we merge contributions.

@johnpc johnpc force-pushed the fix-overlapping-subtitles branch from 8f87eee to b312b30 Compare April 3, 2026 12:44
@johnpc johnpc changed the base branch from release to main April 3, 2026 12:45
@johnpc johnpc requested a review from icbaker April 3, 2026 12:46
Fixes overlapping subtitle rendering when multiple WebVTT cues have
overlapping timestamps and are assigned line numbers.

Problem:
When multiple WebVTT cues overlap in time, they get assigned line numbers
(-1, -2, -3 for bottom-stacked or 0, 1, 2 for top-stacked). However,
SubtitlePainter calculates each cue's vertical position using only
firstLineHeight, not accounting for the actual multi-line height of
preceding cues. This causes cues to visually overlap.

Solution:
1. Added getLastDrawnTextBottom() to SubtitlePainter to expose the actual
   rendered height of a text cue
2. Modified CanvasSubtitleOutput.dispatchDraw() to track cumulative height
   of stacked cues and adjust the drawing boundary for subsequent cues
3. Handles both negative line numbers (bottom-stacked) and positive line
   numbers (top-stacked)

Fixes androidx#2237
@johnpc johnpc force-pushed the fix-overlapping-subtitles branch from b312b30 to 38536d3 Compare April 3, 2026 12:53
@johnpc
Copy link
Copy Markdown
Author

johnpc commented Apr 8, 2026

@icbaker I've addressed all your feedback - updated copyright years, renamed the method to getLastDrawnTextBottom() with clearer docs, added support for positive line numbers, and rebased onto main. Ready for another look when you have time!

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.

Overlapping subtitles with VTT file.

2 participants