Reduce GIF frame window from 30ms to 20ms #3886
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pajlada
changed the title
pajlada
requested changes
Jul 31, 2022
There was a problem hiding this comment.
Please apply the following patch, then make your PR up-to-date with the latest main branch
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f37e9b4b..6f6a0949 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,7 +2,6 @@
## Unversioned
-- Minor: Fixed Animated emote frame delays (#3886)
- Major: Added multi-channel searching to search dialog via keyboard shortcut. (Ctrl+Shift+F by default) (#3694, #3875)
- Minor: Added option to display tabs on the right and bottom. (#3847)
- Minor: Added `is:first-msg` search option. (#3700)
@@ -25,6 +24,7 @@
- Minor: Added `/copy` command. Usage: `/copy <text>`. Copies provided text to clipboard - can be useful with custom commands. (#3763)
- Minor: Removed total views from the usercard, as Twitch no longer updates the number. (#3792)
- Minor: Add Quick Switcher item to open a channel in a new popup window. (#3828)
+- Minor: Reduced GIF frame window from 30ms to 20ms, causing fewer frame skips in animated emotes. (#3886)
- Bugfix: Fix crash that can occur when closing and quickly reopening a split, then running a command. (#3852)
- Bugfix: Connection to Twitch PubSub now recovers more reliably. (#3643, #3716)
- Bugfix: Fix crash that can occur when changing channels. (#3799)
TroyKomodo
force-pushed
the
troy/fix-frame-timings
branch
from
deff6ed
to
4ef0f7c
Compare
Felanbird
approved these changes
Jul 31, 2022
|
Unsure why ci fails. |
|
New clang-tide change that seems to have broken |
|
|
Felanbird
added a commit
to Felanbird/chatterino2
that referenced
this pull request
Aug 3, 2022
This reverts commit ebc7852 which broke emote speed on windows
|
Heads up @TroyKomodo - Since this PR ended up causing more issues than intended we reverted it in 76a891c I'm happy to have this fix in eventually, but we need to test it better on different platforms. |
|
Sure, sorry i have been unable to debug this since i dont have a windows build env, ive been setting one up however. |
|
I suspect its related to https://stackoverflow.com/questions/19958788/qt-5-1-1-and-opengl-rendering-speed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request checklist:
CHANGELOG.mdwas updated, if applicableDescription
Currently the check for the next animated frame is every 30ms which happens to be too slow for the gif standard, the maximum speed of a gif is 20ms per frame.
An edge case would be when a gif only has 2 frames
The total frame duration is 40ms
In the current implementation, if we start at a 0 offset
As you can see the frames are duplicated when they shouldn't be.
Its like a train station with trains ever 30ms and we are only ready to board every 20ms, and since we are impatient we miss our train.
Perhaps a smarter way of detecting which frame to select is a more performant approach however I haven't thought of how to do that. Or we could just say that 30ms is the min frame timing instead of 20ms
fixes SevenTV#85