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.

Sign up for GitHub

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

Jump to bottom

Fix subtitles disappear when resizing #28

Merged
merged 3 commits into from
May 14, 2022
Merged

Fix subtitles disappear when resizing #28

merged 3 commits into from
May 14, 2022

Conversation

dmitrylyzo
Copy link
Collaborator

@dmitrylyzo dmitrylyzo commented May 6, 2022 •
edited
Loading

Original PR libass#132 doesn't work with RenderAhead mode.
Our mode resizes the canvas according to the rendered viewport, and the delayed resize breaks the display again.
It also turns out that when using the buttons in the Titan example to change the player size, we need to update the stylized size.
So I had to rewrite the fix.

Changes

  • Apply styles without checking.
  • Add private variables to store the previous size.

Steps To Reproduce

  1. Open VideoJS example.
  2. Start playback and pause when subtitles are visible.
  3. Go fullscreen.
  4. No subtitles are visible. They blink.

@dmitrylyzo dmitrylyzo changed the title Fix resize Fix subtitles disappear when resizing May 6, 2022
@dmitrylyzo dmitrylyzo marked this pull request as draft May 7, 2022 19:20
@dmitrylyzo dmitrylyzo force-pushed the fix-resize branch 3 times, most recently from a0aa8ef to bcecaed Compare May 8, 2022 15:26
@dmitrylyzo dmitrylyzo marked this pull request as ready for review May 8, 2022 16:45
@dmitrylyzo dmitrylyzo added the bug Something isn't working label May 8, 2022
@dmitrylyzo dmitrylyzo force-pushed the fix-resize branch 2 times, most recently from a83dc3b to 52eb62c Compare May 10, 2022 13:45
dmitrylyzo
dmitrylyzo commented May 10, 2022
View reviewed changes
src/subtitles-octopus.js Outdated Show resolved Hide resolved
@dmitrylyzo
js: fix subtitles disappear when resizing
2b1e507 
1. Missing `px` in offset comparison - deferred resizing resets
the canvas by writing the same values to `width` and `height`.
[turuslan <[email protected]>] found this bug.

2. Since RenderAhead resizes the canvas according to the
rendered frame, the native properties will be overwritten, and
we need additional properties for comparison.
@dmitrylyzo
Fix manual cache reset
d3ba00d 
Cache reset should use the render target size (which has been
passed to the worker).
@dmitrylyzo
Copy link
Collaborator Author

I made resetRenderAheadCache use the render target size (which has been passed to the worker) for the case when it is called with isResizing: true by the user. But we need to remove this ability - JSO monitors size.

@dmitrylyzo dmitrylyzo merged commit 5cf1afa into jellyfin:new-master May 14, 2022
@dmitrylyzo dmitrylyzo deleted the fix-resize branch May 14, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@dmitrylyzo