[cc] Disable frame throttling when frame rate is unlimited. [chromium/src : main]
Andrew Wolfers (Gerrit)
Attention is currently required from: Jonathan Ross.
Andrew Wolfers would like Jonathan Ross to review this change.
[cc] Disable frame throttling when frame rate is unlimited.
Running with the `--disable-frame-rate-limit` arg is supposed to allow
frames to be rendered at the maximum possible rate, but instead the
framerate is throttled such that it cannot exceed the display refresh
rate.
This CL prevents throttling when the `--disable-frame-rate-limit` arg
is set so that the framerate is correctly uncapped.
Bug: b:242953824
Change-Id: Ibdcd9c7217c51914f55e60aa33f894c633b6689f
---
M cc/scheduler/scheduler_state_machine.cc
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
index 0f9872b..e5e1a7b 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -5,11 +5,13 @@
#include "cc/scheduler/scheduler_state_machine.h"
#include "base/check_op.h"
+#include "base/command_line.h"
#include "base/format_macros.h"
#include "base/notreached.h"
#include "base/trace_event/trace_event.h"
#include "base/trace_event/traced_value.h"
#include "base/values.h"
+#include "components/viz/common/switches.h"
namespace cc {
@@ -1327,7 +1329,11 @@
}
bool SchedulerStateMachine::IsDrawThrottled() const {
- return pending_submit_frames_ >= kMaxPendingSubmitFrames;
+ static bool is_frame_rate_limit_disabled =
+ base::CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kDisableFrameRateLimit);
+ return pending_submit_frames_ >= kMaxPendingSubmitFrames &&
+ !is_frame_rate_limit_disabled;
}
void SchedulerStateMachine::SetVisible(bool visible) {
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Andrew Wolfers (Gerrit)
Attention is currently required from: Jonathan Ross.
Jonathan Ross (Gerrit)
Attention is currently required from: Andrew Wolfers.
2 comments:
File cc/scheduler/scheduler_state_machine.cc:
static bool is_frame_rate_limit_disabled =
base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableFrameRateLimit);Can this be added to `SchedulerSettings` as a param for this class?
`static bool` is discouraged for how it interacts with testing
Patch Set #1, Line 1335:
pending_submit_frames_ >= kMaxPendingSubmitFramesThis waits on ACKs from the GPU process. With `--disable-frame-rate-limit` on do we know which `Surface::FrameData::SendAckIfNeeded` was being skipped leading to this throttle being hit?
For example, if it was in `Surface::QueueFrame` do we know if these newly submitted frames are being processed/displayed?
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Andrew Wolfers (Gerrit)
Attention is currently required from: Jonathan Ross.
2 comments:
File cc/scheduler/scheduler_state_machine.cc:
static bool is_frame_rate_limit_disabled =
base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableFrameRateLimit);
Can this be added to `SchedulerSettings` as a param for this class? […]
Done
Patch Set #1, Line 1335:
pending_submit_frames_ >= kMaxPendingSubmitFrames
This waits on ACKs from the GPU process. […]
Yes it was `Surface::QueueFrame`, and I believe yes they are being processed in the new version. They aren't literally displayed since the display doesn't refresh that fast, but it my testing `requestAnimationFrame` is being invoked at 1000fps.
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Jonathan Ross (Gerrit)
Attention is currently required from: Andrew Wolfers.
1 comment:
File cc/scheduler/scheduler_state_machine.cc:
Patch Set #1, Line 1335:
pending_submit_frames_ >= kMaxPendingSubmitFramesYes it was `Surface::QueueFrame`, and I believe yes they are being processed in the new version. […]
Glad to hear that it leads to the uncapped `rAF`
I realize the displaying will still be tied to the display rate. I'm curious as to the interaction of the 1000fps `rAF` vs how Surface handles activating/committing/uncommitted frames.
IIRC the latest frame will display at the VSync time. However there is an internal cap on the `uncommitted` frames. So I'm wondering if this would lead to us dropping a large amount of the submitted frames.
Could you take a "Rendering" trace? There's a bunch of "viz" category traces in Surface I'd be interested in seeing.
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Andrew Wolfers (Gerrit)
Attention is currently required from: Jonathan Ross.
1 comment:
File cc/scheduler/scheduler_state_machine.cc:
Patch Set #1, Line 1335:
pending_submit_frames_ >= kMaxPendingSubmitFrames
Glad to hear that it leads to the uncapped `rAF` […]
Done (emailed to you). I think it does look like a large number of frames are dropped as you say. Very interested to hear what you learn from the trace.
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Jonathan Ross (Gerrit)
Attention is currently required from: Andrew Wolfers, Stefan Zager.
Jonathan Ross would like Stefan Zager to review this change authored by Andrew Wolfers.
[cc] Disable frame throttling when frame rate is unlimited.
Running with the `--disable-frame-rate-limit` arg is supposed to allow
frames to be rendered at the maximum possible rate, but instead the
framerate is throttled such that it cannot exceed the display refresh
rate.
This CL prevents throttling when the `--disable-frame-rate-limit` arg
is set so that the framerate is correctly uncapped.
Bug: b:242953824
Change-Id: Ibdcd9c7217c51914f55e60aa33f894c633b6689f
---
M cc/scheduler/scheduler_settings.cc
M cc/scheduler/scheduler_settings.h
M cc/scheduler/scheduler_state_machine.cc
M cc/trees/layer_tree_settings.cc
M cc/trees/layer_tree_settings.h
M third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc
M ui/compositor/compositor.cc
7 files changed, 36 insertions(+), 1 deletion(-)
Jonathan Ross (Gerrit)
Attention is currently required from: Andrew Wolfers, Stefan Zager.
2 comments:
Patchset:
+szager@ as an owner of third_party/blink/renderer/platform/OWNERS and cc/scheduling
This change sgtm. `disable-frame-rate-limit` already shifts Renderer/GPU process to use back-to-back BeginFrames. The GPU process however was still waiting to VSync before ACKing the accepted/activated frame.
Removing the cc:scheduler back-pressure mechanism when using this flag makes sense to me. Looking at the traces the Renderer was able to submit as it was ready in response to begin frames. While the GPU process was able to activate each frame in turn, and await VSync for actually drawing/displaying.
File cc/scheduler/scheduler_state_machine.cc:
Patch Set #1, Line 1335:
pending_submit_frames_ >= kMaxPendingSubmitFrames
Done (emailed to you). I think it does look like a large number of frames are dropped as you say. […]
From the trace, Viz was successfully activating each new CompositorFrame that was submitted. Which would result in the desired effect of the newest submitted frame being drawn/presented at VSync.
This sgtm, thanks!
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Stefan Zager (Gerrit)
Attention is currently required from: Andrew Wolfers.
1 comment:
File third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc:
Patch Set #2, Line 544:
settings.disable_frame_rate_limit =Can you make WidgetBase::RequestNewLayerTreeFrameSink use this setting now, rather than looking at the command line?
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Andrew Wolfers (Gerrit)
Attention is currently required from: Jonathan Ross, Stefan Zager.
2 comments:
File cc/scheduler/scheduler_state_machine.cc:
Patch Set #1, Line 1335:
pending_submit_frames_ >= kMaxPendingSubmitFrames
From the trace, Viz was successfully activating each new CompositorFrame that was submitted. […]
Thanks!
File third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc:
Patch Set #2, Line 544:
settings.disable_frame_rate_limit =
Can you make WidgetBase::RequestNewLayerTreeFrameSink use this setting now, rather than looking at t […]
Done
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Stefan Zager (Gerrit)
Attention is currently required from: Andrew Wolfers.
Patch set 3:Code-Review +1
1 comment:
Patchset:
lgtm
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Andrew Wolfers (Gerrit)
Attention is currently required from: Robert Flack.
Andrew Wolfers would like Robert Flack to review this change.
[cc] Disable frame throttling when frame rate is unlimited.
Running with the `--disable-frame-rate-limit` arg is supposed to allow
frames to be rendered at the maximum possible rate, but instead the
framerate is throttled such that it cannot exceed the display refresh
rate.
This CL prevents throttling when the `--disable-frame-rate-limit` arg
is set so that the framerate is correctly uncapped.
Bug: b:242953824
Change-Id: Ibdcd9c7217c51914f55e60aa33f894c633b6689f
---
M cc/scheduler/scheduler_settings.cc
M cc/scheduler/scheduler_settings.h
M cc/scheduler/scheduler_state_machine.cc
M cc/trees/layer_tree_settings.cc
M cc/trees/layer_tree_settings.h
M third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc
M third_party/blink/renderer/platform/widget/widget_base.cc
M ui/compositor/compositor.cc
8 files changed, 37 insertions(+), 5 deletions(-)
Andrew Wolfers (Gerrit)
Attention is currently required from: Robert Flack.
1 comment:
Patchset:
+@flackr for owners approval on ui/compositor/compositor.cc
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Robert Flack (Gerrit)
Attention is currently required from: Andrew Wolfers, Jonathan Ross.
1 comment:
File cc/scheduler/scheduler_state_machine.cc:
Patch Set #3, Line 1331:
!settings_.disable_frame_rate_limit;What happens when we submit more than 1 pending frame at a time? Are they all eventually drawn and acked or do we only draw the latest one?
I feel like this may have implications on how realistic the unlimited frame rate is.
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Jonathan Ross (Gerrit)
Attention is currently required from: Andrew Wolfers, Robert Flack.
1 comment:
File cc/scheduler/scheduler_state_machine.cc:
Patch Set #3, Line 1331:
!settings_.disable_frame_rate_limit;
What happens when we submit more than 1 pending frame at a time? Are they all eventually drawn and a […]
On non-WebView the GPU process will activate each newly arrived frame. Then await VSync for actually drawing/displaying. At which time the most recently submitted frame will be used.
There is some extra complexity if there is on-going changes to visual properties (like a window resize.) Where a different deadline is applied to activating surfaces that have dependencies on others. However we should still be activating the newest frame from the client.
WebView has some extra complexity with how it enqueues/activates frames. So there we can end up presenting one frame older than the newest submitted one.
aswolfers@ sent me a trace of this. There the Renderer was successfully submitting multiple frames within a VSync and the GPU only attempted to draw-and-swap once.
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Robert Flack (Gerrit)
Attention is currently required from: Andrew Wolfers, Jonathan Ross.
Patch set 3:Code-Review +1
1 comment:
File cc/scheduler/scheduler_state_machine.cc:
Patch Set #3, Line 1331:
!settings_.disable_frame_rate_limit;
On non-WebView the GPU process will activate each newly arrived frame. […]
Thanks, so IIUC we shouldn't end up with unacked submit frames accumulating, and at worst will be displaying slightly old frames which seems not a big deal for what is presumably a flag meant for benchmarking.
To view, visit change 3863575. To unsubscribe, or for help writing mail filters, visit settings.
Andrew Wolfers (Gerrit)
Attention is currently required from: Andrew Wolfers, Jonathan Ross.
Patch set 3:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
[cc] Disable frame throttling when frame rate is unlimited.
Running with the `--disable-frame-rate-limit` arg is supposed to allow
frames to be rendered at the maximum possible rate, but instead the
framerate is throttled such that it cannot exceed the display refresh
rate.
This CL prevents throttling when the `--disable-frame-rate-limit` arg
is set so that the framerate is correctly uncapped.
Bug: b:242953824
Change-Id: Ibdcd9c7217c51914f55e60aa33f894c633b6689f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3863575
Commit-Queue: Andrew Wolfers <aswo...@chromium.org>
Reviewed-by: Robert Flack <fla...@chromium.org>
Reviewed-by: Stefan Zager <sza...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1045852}
---
M cc/scheduler/scheduler_settings.cc
M cc/scheduler/scheduler_settings.h
M cc/scheduler/scheduler_state_machine.cc
M cc/trees/layer_tree_settings.cc
M cc/trees/layer_tree_settings.h
M third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc
M third_party/blink/renderer/platform/widget/widget_base.cc
M ui/compositor/compositor.cc
8 files changed, 42 insertions(+), 5 deletions(-)