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

SubtitleView custom layout measurement #4788

Closed
komissarovdenis opened this issue Sep 7, 2018 · 6 comments
Closed

SubtitleView custom layout measurement #4788

komissarovdenis opened this issue Sep 7, 2018 · 6 comments
Assignees
Labels

Comments

@komissarovdenis
Copy link

komissarovdenis commented Sep 7, 2018

When you have a SubtitleView with custom layout measurement (Parent view has match_parent for both dims and SubtitleView has smaller size and positioned in center) SubtitlePainter will draw cues somewhere outside of SubtitleView's viewport and you'll see nothing.

Bug is probably located in SubtitlePainter in method setupTextLayout() on the line 346
textTop = parentBottom - textHeight - (int) (parentHeight * bottomPaddingFraction);
it's necessary to decrement parentTop as well.
Should be: textTop = parentBottom - parentTop - textHeight - (int) (parentHeight * bottomPaddingFraction);

Version 2.8.4
Thanks

@tonihei
Copy link
Collaborator

tonihei commented Sep 7, 2018

Not sure this is correct. The text is positioned relative to the bottom edge of the parent. Thus we subtract the height of the text and the padding from the bottom edge to get the top of the text. This is independent of the top of the parent and it may mean that the position "overflows" the top.

Could it be that your SubtitleView is just too small to fit the text? Try changing the bottom padding fraction (subtitleView.setBottomPaddingFraction) and/or the text size (subtitleView.setFractionalTextSize or subtitleView.setFixedTextSize).

@tonihei tonihei self-assigned this Sep 7, 2018
@komissarovdenis
Copy link
Author

komissarovdenis commented Sep 7, 2018

Let me try to explain how views actually look:

  1. Video Texture fits all the screen, but actually rendering may not cover all it's area (for example: screen 1080x2220 and typical video 16:9 is rendering like 1080x608 and other area is just black). It's necessary to have such a big texture to be able make animated transitions/scales/crops/etc from one screen to another.
  2. If SubtitleView measured like 1080x2220 then we have a huge size of text and it's not I'am expect.
    If it is measured to video frame size 1080x608 and located in screen center over video then there's a bug when cues rendered somewhere bottom outside subtitle's view.

@tonihei
Copy link
Collaborator

tonihei commented Sep 7, 2018

I think I understand your problem. Please try to change the padding or text size as suggested above.
If you think this doesn't help, please try to explain your problem some other way.

@komissarovdenis
Copy link
Author

I tried to manipulate with paddings, fractions and text sizes - there's no difference at all.

So, I prepared a small test (based on your demo) to show in practice:
https://github.com/komissarovdenis/ExoPlayer/commit/0bea7972c8008c1be15f8c58b5c0eb2473d7d337

  1. I added an HLS link to content with subtitles
  2. Moved out subtitles view from ratio container to emulate case when subtitle view has different proportions comparing with surface
    You can try to compile code without decrementing parentTop and check "visibility" of cues.
    More over: this fix make no changes (shifts) for default use like a child of ratio layout.

@tonihei
Copy link
Collaborator

tonihei commented Oct 2, 2018

Thanks for demo (and sorry for the delay!). I now understand the issue and it seems indeed to be a bug in our code. We'll provide a fix for that soon.

@tonihei tonihei added bug and removed question labels Oct 2, 2018
ojw28 pushed a commit that referenced this issue Oct 3, 2018
SubtitleView forwards the cue box position to SubtitlePainter. This should be
the position relative to the canvas of the SubtitleView. Currently, however,
we forward the position relative to the parent of SubtitleView. That causes
problems if SubtitleView has a non-zero offset position to its parent.

Issue:#4788

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=215535281
@tonihei
Copy link
Collaborator

tonihei commented Oct 11, 2018

Closing because the issue is fixed by the commit above.

@tonihei tonihei closed this as completed Oct 11, 2018
ojw28 pushed a commit that referenced this issue Oct 20, 2018
SubtitleView forwards the cue box position to SubtitlePainter. This should be
the position relative to the canvas of the SubtitleView. Currently, however,
we forward the position relative to the parent of SubtitleView. That causes
problems if SubtitleView has a non-zero offset position to its parent.

Issue:#4788

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=215535281
@google google locked and limited conversation to collaborators May 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants