Fix overlapping subtitles with negative line numbers#3151
Fix overlapping subtitles with negative line numbers#3151johnpc wants to merge 1 commit intoandroidx:mainfrom
Conversation
|
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. |
|
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 |
libraries/ui/src/test/java/androidx/media3/ui/CanvasSubtitleOutputTest.java
Outdated
Show resolved
Hide resolved
libraries/ui/src/main/java/androidx/media3/ui/SubtitlePainter.java
Outdated
Show resolved
Hide resolved
8f87eee to
b312b30
Compare
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
b312b30 to
38536d3
Compare
|
@icbaker I've addressed all your feedback - updated copyright years, renamed the method to |
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,SubtitlePaintercalculates each cue's vertical position using onlyfirstLineHeight(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:
This is particularly common in anime subtitles where overlapping timestamps are used for effects like showing both dialogue and on-screen text.
Solution
getLastDrawnTextHeight()toSubtitlePainterto expose the actual rendered height of a text cueCanvasSubtitleOutput.dispatchDraw()to track cumulative height of bottom-stacked cues (those with negativeLINE_TYPE_NUMBER) and adjust the drawing boundary for subsequent cuesTesting
Added unit tests for:
SubtitlePainterTest: Tests forgetLastDrawnTextHeight()with various cue typesCanvasSubtitleOutputTest: Tests for rendering overlapping cues without visual overlapFixes #2237