Closed Bug 1233576 Opened 7 years ago Closed 7 years ago

Vsync timestamps are not monotonic (Windows 10)


(Core :: Graphics, defect)

Windows 10
Not set



Tracking Status
firefox45 --- fixed
firefox46 --- fixed


(Reporter: kats, Assigned: mchang)



(1 file)

While investigating bug 1222190 I found a problem with the vsync timestamps on windows. To repro, build m-c with the following patch, which logs the time delta between consecutive vsync timestamps.

diff --git a/gfx/thebes/gfxWindowsPlatform.cpp b/gfx/thebes/gfxWindowsPlatform.cpp
index 64c1a31..8b66569 100755
--- a/gfx/thebes/gfxWindowsPlatform.cpp
+++ b/gfx/thebes/gfxWindowsPlatform.cpp
@@ -2887,6 +2887,10 @@ public:
           // Large parts of gecko assume that the refresh driver timestamp
           // must be <= Now() and cannot be in the future.
           MOZ_ASSERT(vsync <= TimeStamp::Now());
+static TimeStamp lastsync = vsync;
+printf_stderr("vsync delta %f %f\n", (vsync - lastsync).ToMilliseconds(),
+(TimeStamp::Now() - vsync).ToMilliseconds());
+lastsync = vsync;

           // DwmComposition can be dynamically enabled/disabled

The output that I got is like this:

vsync delta 29211.678826 0.002415
vsync delta -29178.324157 29197.131846
vsync delta 29211.362205 0.333782
vsync delta 16.700367 1.760108
vsync delta 16.791071 0.909661
vsync delta 16.727641 0.272365
vsync delta 16.710950 0.646087
vsync delta 16.732464 0.599236
vsync delta 16.701010 1.199634
vsync delta 16.676389 1.316924
vsync delta 16.797283 0.504336
vsync delta 16.697694 0.482595
vsync delta 16.713161 0.523871
vsync delta 16.746834 0.788056
vsync delta 16.736019 0.711691
vsync delta 16.640654 0.683601
vsync delta 180.083829 0.001811
vsync delta -146.581049 150.912740
vsync delta 167.151306 0.974095
vsync delta 16.773642 0.531682

Those negative deltas shouldn't be there.
OS: Unspecified → Windows 10
Assignee: nobody → mchang
The problem with the log occurred only when we enabled vsync. On Windows 10, since DWMGetCompositionTimingInfo returns the upcoming vsync timestamp in the future, we always used the previous vsync timestamp to actually tick the refresh driver / compositor. When we enable vsync, we'd tick with TimeStamp::Now(), then the previous vsync timestamp that happened before we disabled vsync for the second tick, then the third tick would use the proper vsync timestamp. 

This patch sets the previous timestamp to Now() + 16 ms when we enable vsync. Thus the first iteration of the loop will use Now(), the second Now() + 16, the third will use the actual vsync timestamp.
Attachment #8699767 - Flags: review?(bugmail.mozilla)
Comment on attachment 8699767 [details] [diff] [review]
Ensure vsync timestamps on windows 10 are monotonic

Review of attachment 8699767 [details] [diff] [review]:

Attachment #8699767 - Flags: review?(bugmail.mozilla) → review+
/cc vlad as fyi
Comment on attachment 8699767 [details] [diff] [review]
Ensure vsync timestamps on windows 10 are monotonic

Approval Request Comment
[Feature/regressing bug #]: Project Silk
[User impact if declined]: Windows 10 users may get jankier animations / scrolling when coming from an idle state.
[Describe test coverage new/current, TreeHerder]: Manual / treeherder
[Risks and why]: Low - This fixes a bug on Windows 10 where we would ignore a vsync timestamp, causing jank when coming from an idle state.
[String/UUID change made/needed]: None
Attachment #8699767 - Flags: approval-mozilla-aurora?
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment on attachment 8699767 [details] [diff] [review]
Ensure vsync timestamps on windows 10 are monotonic

Improve the behavior of our product on Windows 10, taking it.
Attachment #8699767 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.