Add VideoStreamPlayer UpdateMode to not process the invisible#114517
Open
smix8 wants to merge 1 commit intogodotengine:masterfrom
Open
Add VideoStreamPlayer UpdateMode to not process the invisible#114517smix8 wants to merge 1 commit intogodotengine:masterfrom
smix8 wants to merge 1 commit intogodotengine:masterfrom
Conversation
Adds UpdateMode to VideoStreamPlayer for better performance and control in gui heavy setups with lots of small video streams.
a862bd5 to
334396e
Compare
Member
|
Did you try VisibleOnScreenEnabler2D? Put one as child of VideoStreamPlayer and it should have the same effect. |
Contributor
Author
Working with extra nodes is not a viable solution because it causes node and tree related issues, especially when running tool scripts for custom video players. The node-forced video playback is flawed in many ways but that is mostly addressed by fixing the script exposed playback functions with #114515. This PR here is mostly to protect the inexperienced Godot user from running into performance issues by having a more sensible default behavior. |
This file contains hidden or 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
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.
Adds UpdateMode to VideoStreamPlayer for better performance and control in gui heavy setups with lots of small video streams.
Performance
These performance spikes seen here are with 10+ smaller video stream players active that are all from different gui elements off-screen and not visible on a 144 hz monitor with uncapped frame rate. Part of the big jump is also caused by running into a physics death spiral due to all the senseless processing for invisible videos going on behind the scene.
The changes in this PR with visibility check is what causes the performance to reset to manageable levels because it stops processing all the videos that are in hidden menus or off-screen automatically (in this example case when the graph is at its lowest just 1 or zero videos are processing because those can actually be seen). There are other performance issues with the VideoStreamPlayer node for other PRs to fix but this is a first step.
Update modes
Same as paused but without changing properties that would cause unwanted side effects in code.
The recommended winner mode that only processes what can actually be seen on screen. Works by using the node visibility and the CanvasItem visibility notifier.
The current default that updates and processes everything all the time no matter what.
While UPDATE_ALWAYS would match with the current way the VideoStreamPlayer is behaving I recommend using the UPDATE_WHEN_VISIBLE as the new default.
It commonly does not make that much sense to process videos while not at all visible. That situation frequently is found in Control based gui menus where users park menus with video backdrops for later use. Since the video player has no dedicated debugger all users see is an increased process() time not understanding what is causing it.
I encountered these performance issues many times over in Godot projects where users would never free their menu backdrop movies because it would cause stutters when loading the (quick) menu at runtime so they entire gameplay time they would just process in the background movies that no one sees wasting performance. Sure they could add scripts to pause and un-set the Stream but they are commonly not doing that and it would be better to do it automatically.