Fix player position performance issue #606
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
karniv00l
commented
Aug 1, 2023
| @@ -48,7 +48,6 @@ class PlayerControls extends HookConsumerWidget { | |||
|
|
|||
| final playing = | |||
| useStream(audioPlayer.playingStream).data ?? audioPlayer.isPlaying; | |||
| final buffering = useStream(audioPlayer.bufferingStream).data ?? true; | |||
There was a problem hiding this comment.
no clue why, but this is also causing re-renders
karniv00l
commented
Aug 1, 2023
| @@ -89,215 +88,208 @@ class PlayerControls extends HookConsumerWidget { | |||
| iconSize: compact ? 18 : 24, | |||
| ); | |||
|
|
|||
| return RepaintBoundary( | |||
There was a problem hiding this comment.
Only RepaintBoundary and buffering removed
karniv00l
commented
Aug 1, 2023
| @@ -62,99 +62,95 @@ class PlayerOverlay extends HookConsumerWidget { | |||
| child: AnimatedOpacity( | |||
| duration: const Duration(milliseconds: 250), | |||
| opacity: canShow ? 1 : 0, | |||
| child: RepaintBoundary( | |||
There was a problem hiding this comment.
Only RepaintBoundary removed
karniv00l
commented
Aug 1, 2023
| // Duration future is needed for getting the duration of the song | ||
| // as stream can be null when no event occurs (Mostly needed for android) | ||
| final durationFuture = useFuture(audioPlayer.duration); |
There was a problem hiding this comment.
both useFutures cause hook to re-create every tick
1 task
karniv00l
commented
Aug 1, 2023
| @@ -9,21 +8,29 @@ import 'package:spotube/services/audio_player/audio_player.dart'; | |||
| Duration duration, | |||
| double bufferProgress | |||
| }) useProgress(WidgetRef ref) { | |||
| final playlist = ref.watch(ProxyPlaylistNotifier.provider); | |||
RaptaG
added
bug
|
That's awesome work 🚀. Let me quickly check |
|
The CPU usage has dramatically reduced now 🚀 |
|
This is when all music is paused |
1 task
meenbeese
pushed a commit
to meenbeese/spotube
that referenced
this pull request
Sep 20, 2023
meenbeese
pushed a commit
to meenbeese/spotube
that referenced
this pull request
Sep 20, 2023
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.
Fixes #507
I'm not sure whether this is a correct fix and why it's not present on all platforms, devtools show re-renders every tick:
After this fix, when the song is playing, re-render only happens ever 1s (update freq of the position slider), when the song is stopped - zero re-renders