fix: disable background throttling also in the viz::DisplayScheduler
#38924
Conversation
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
`viz::DisplayScheduler` is responsible for drawing and swapping frames in the `DisplayScheduler::DrawAndSwap` which is called from the `DisplayScheduler::AttemptDrawAndSwap` if the `DisplayScheduler::ShouldDraw` returns true. `ShouldDraw` depends on the `DisplayScheduler` visibility and when it is not visible then it returns false. In order to keep producing frames, disabling `backgroundThrottling` should also prevent changing `DisplayScheduler` visibility to false. `DisplayScheduler` lives in the `ui::Compositor` where every `electron::NativewWindow` has its own `Compositor`. `electron::NativewWindow` may be host of the multiple `electron::api::WebContents` instances which may have different `WebPreferences` settings. Therefore if at least one of the `WebContents` requires disabling throttling then all other `WebContents` using the same window will have it disabled in the `ui::Compositor`. BREAKING CHANGE: `backgroundThrottling` set to false will disable frames throttling in the `BrowserWindow` for all `WebContents` displayed by it. Close: [electron#31016](electron#31016)
zcbenz
approved these changes
Jul 13, 2023
codebytere
reviewed
Jul 13, 2023
|
I've addressed all issues and resolved conflict. same with I've no access to retry these builds. |
I think this may have lower chances to upstream than |
jkleinsc
requested changes
Sep 6, 2023
Patches updated, builds failed on |
|
@michal-pichlinski-openfin this has been fixed in latest |
jkleinsc
requested changes
Sep 14, 2023
There was a problem hiding this comment.
The breaking change should be documented in https://github.com/electron/electron/blob/main/docs/breaking-changes.md
|
Linux test runs failed due to: |
|
@michal-pichlinski-openfin that's a flake that occasionally happens - could you please rebase on main? |
|
This failure looks unrelated to my change: |
Addressed in: 4212433 |
jkleinsc
approved these changes
Sep 26, 2023
|
API LGTM |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
Release Notes Persisted
|
|
Can this be updated to electron26? |
|
@fanchenio, this change included a breaking change, so it can not be backported to Electron 26. |
MrHuangJser
pushed a commit
to MrHuangJser/electron
that referenced
this pull request
Dec 11, 2023
electron#38924) * fix: disable background throttling also in the `viz::DisplayScheduler` `viz::DisplayScheduler` is responsible for drawing and swapping frames in the `DisplayScheduler::DrawAndSwap` which is called from the `DisplayScheduler::AttemptDrawAndSwap` if the `DisplayScheduler::ShouldDraw` returns true. `ShouldDraw` depends on the `DisplayScheduler` visibility and when it is not visible then it returns false. In order to keep producing frames, disabling `backgroundThrottling` should also prevent changing `DisplayScheduler` visibility to false. `DisplayScheduler` lives in the `ui::Compositor` where every `electron::NativewWindow` has its own `Compositor`. `electron::NativewWindow` may be host of the multiple `electron::api::WebContents` instances which may have different `WebPreferences` settings. Therefore if at least one of the `WebContents` requires disabling throttling then all other `WebContents` using the same window will have it disabled in the `ui::Compositor`. BREAKING CHANGE: `backgroundThrottling` set to false will disable frames throttling in the `BrowserWindow` for all `WebContents` displayed by it. Close: [electron#31016](electron#31016) * fixup! fix: disable background throttling also in the `viz::DisplayScheduler` * fixup! fix: disable background throttling also in the `viz::DisplayScheduler` * fixup! fix: disable background throttling also in the `viz::DisplayScheduler` --------- Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
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.
Description of Change
viz::DisplayScheduleris responsible for drawing and swapping frames in theDisplayScheduler::DrawAndSwapwhich is called from theDisplayScheduler::AttemptDrawAndSwapif theDisplayScheduler::ShouldDrawreturns true.ShouldDrawdepends on theDisplaySchedulervisibility and when it is not visible then it returns false.In order to keep producing frames, disabling
backgroundThrottlingshould also prevent changingDisplaySchedulervisibility to false.DisplaySchedulerlives in theui::Compositorwhere everyelectron::NativewWindowhas its ownCompositor.electron::NativewWindowmay be host of the multipleelectron::api::WebContentsinstances which may have differentWebPreferencessettings. Therefore if at least one of theWebContentsrequires disabling throttling then all otherWebContentsusing the same window will have it disabled in theui::Compositor.BREAKING CHANGE:
backgroundThrottlingset to false will disable frames throttling in theBrowserWindowfor allWebContentsdisplayed by it.Closes: #31016
Checklist
npm testpassesRelease Notes
Notes: Fixed generating frames when the window is hidden and
backgroundThrottlingis disabled.