diff options
author | Daniel van Vugt <daniel.van.vugt@canonical.com> | 2018-11-02 15:47:58 +0800 |
---|---|---|
committer | Georges Basile Stavracas Neto <georges.stavracas@gmail.com> | 2019-05-16 22:13:54 +0000 |
commit | 45244852acc214a7a9d01dc96896ad0c079b437c (patch) | |
tree | 7e04fdf631db2d0b7a277d6ba7d541fba49bf978 /clutter | |
parent | e96136e418ff670f80736e267aa37d160b1339ff (diff) | |
download | mutter-45244852acc214a7a9d01dc96896ad0c079b437c.tar.gz |
clutter/stage-cogl: Don't skip over the next frame
The `last_presentation_time` is usually a little in the past (although
sometimes in the future depending on the driver). When it's over 2ms
(`sync_delay`) in the past that would trigger the while loop to count up so
that the next `update_time` is in the future.
The problem with that is for common values of `last_presentation_time`
which are only a few milliseconds ago, incrementing `update_time` by
`refresh_interval` also means counting past the next physical frame that
we haven't rendered yet. And so mutter would skip that frame.
**Example**
Given:
```
last_presentation_time = now - 3ms
sync_delay = 2ms
refresh_interval = 16ms
next_presentation_time = last_presentation_time + refresh_interval
= now + 13ms
-3ms now +13ms +29ms +45ms
----|--+------------|---------------|---------------|----
: :
last_presentation_time next_presentation_time
```
Old algorithm:
```
update_time = last_presentation_time + sync_delay
= now - 1ms
while (update_time < now)
(now - 1ms < now)
update_time = now - 1ms + 16ms
update_time = now + 15ms
next_presentation_time = now + 13ms
available_render_time = next_presentation_time - max(now, update_time)
= (now + 13ms) - (now + 15ms)
= -2ms so the next frame will be skipped.
-3ms now +13ms +29ms +45ms
----|--+------------|-+-------------|---------------|----
: : :
: : update_time (too late)
: :
last_presentation_time next_presentation_time (a missed frame)
```
New algorithm:
```
min_render_time_allowed = refresh_interval / 2
= 8ms
max_render_time_allowed = refresh_interval - sync_delay
= 14ms
target_presentation_time = last_presentation_time + refresh_interval
= now - 3ms + 16ms
= now + 13ms
while (target_presentation_time - min_render_time_allowed < now)
(now + 13ms - 8ms < now)
(5ms < 0ms)
# loop is never entered
update_time = target_presentation_time - max_render_time_allowed
= now + 13ms - 14ms
= now - 1ms
next_presentation_time = now + 13ms
available_render_time = next_presentation_time - max(now, update_time)
= (now + 13ms) - now
= 13ms which is plenty of render time.
-3ms now +13ms +29ms +45ms
----|-++------------|---------------|---------------|----
: : :
: update_time :
: :
last_presentation_time next_presentation_time
```
The reason nobody noticed these missed frames very often was because
mutter has some accidental workarounds built-in:
* Prior to 3.32, the offending code was only reachable in Xorg sessions.
It was never reached in Wayland sessions because it hadn't been
implemented yet (till e9e4b2b72).
* Even though Wayland support is now implemented the native backend
provides a `last_presentation_time` much faster than Xorg sessions
(being in the same process) and so is less likely to spuriously enter
the while loop to miss a frame.
* For Xorg sessions we are accidentally triple buffering (#334). This
is a good way to avoid the missed frames, but is also an accident.
* `sync_delay` is presently just high enough (2ms by coincidence is very
close to common values of `now - last_presentation_time`) to push the
`update_time` into the future in some cases, which avoids entering the
while loop. This is why the same missed frames problem was also noticed
when experimenting with `sync_delay = 0`.
v2: adjust variable names and code style.
Fixes: https://bugzilla.gnome.org/show_bug.cgi?id=789186
and most of https://gitlab.gnome.org/GNOME/mutter/issues/571
https://gitlab.gnome.org/GNOME/mutter/merge_requests/520
Diffstat (limited to 'clutter')
-rw-r--r-- | clutter/clutter/cogl/clutter-stage-cogl.c | 17 |
1 files changed, 14 insertions, 3 deletions
diff --git a/clutter/clutter/cogl/clutter-stage-cogl.c b/clutter/clutter/cogl/clutter-stage-cogl.c index 2a2a17e05..1ed7e01c4 100644 --- a/clutter/clutter/cogl/clutter-stage-cogl.c +++ b/clutter/clutter/cogl/clutter-stage-cogl.c @@ -152,6 +152,9 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, gint64 now; float refresh_rate; gint64 refresh_interval; + int64_t min_render_time_allowed; + int64_t max_render_time_allowed; + int64_t next_presentation_time; if (stage_cogl->update_time != -1) return; @@ -184,10 +187,18 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, if (refresh_interval == 0) refresh_interval = 16667; /* 1/60th second */ - stage_cogl->update_time = stage_cogl->last_presentation_time + 1000 * sync_delay; + min_render_time_allowed = refresh_interval / 2; + max_render_time_allowed = refresh_interval - 1000 * sync_delay; - while (stage_cogl->update_time < now) - stage_cogl->update_time += refresh_interval; + if (min_render_time_allowed > max_render_time_allowed) + min_render_time_allowed = max_render_time_allowed; + + next_presentation_time = stage_cogl->last_presentation_time + refresh_interval; + + while (next_presentation_time < now + min_render_time_allowed) + next_presentation_time += refresh_interval; + + stage_cogl->update_time = next_presentation_time - max_render_time_allowed; } static gint64 |