Calculate the refresh period from GLX_OML_sync_control.

Calculate the refresh period from GLX_OML_sync_control.

glXGetSyncValuesOML gives us a count of the number of refreshes that have occurred (media stream counter). We can use this and the time between successive UST counters to measure the period of the screen refresh.

BUG=none
TEST=by hand on stumpy and alex using xrandr to force different refresh rates


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175234

[jonathan.backer, 2013-01-02 21:07:10]

Seems accurate to within about 10us on alex and stumpy.

Do we want to clamp the computer interval to a reasonable range (say 5ms to
50ms) or do we just assume GIGO?

[jonathan.backer, 2013-01-02 21:39:12]

On 2013/01/02 21:07:10, jonathan.backer wrote:
> Seems accurate to within about 10us on alex and stumpy.
> 
> Do we want to clamp the computer interval to a reasonable range (say 5ms to
> 50ms) or do we just assume GIGO?

s/computer/computed :D

[ajuma, 2013-01-04 15:34:43]

The approach looks good to me and is a big improvement over assuming 60 fps.

https://codereview.chromium.org/11752002/diff/4001/ui/gl/gl_surface_glx.cc
File ui/gl/gl_surface_glx.cc (right):

https://codereview.chromium.org/11752002/diff/4001/ui/gl/gl_surface_glx.cc#ne...
ui/gl/gl_surface_glx.cc:151: int64 last_good_interval_;
Nit: making this a TimeDelta instead of an int64 will save a couple of
conversions to/from microseconds.

[jonathan.backer, 2013-01-04 16:26:45]

https://codereview.chromium.org/11752002/diff/4001/ui/gl/gl_surface_glx.cc
File ui/gl/gl_surface_glx.cc (right):

https://codereview.chromium.org/11752002/diff/4001/ui/gl/gl_surface_glx.cc#ne...
ui/gl/gl_surface_glx.cc:151: int64 last_good_interval_;
On 2013/01/04 15:34:43, ajuma wrote:
> Nit: making this a TimeDelta instead of an int64 will save a couple of
> conversions to/from microseconds.

Done.

[ajuma, 2013-01-04 16:28:33]

LGTM

On 2013/01/04 16:26:45, jonathan.backer wrote:
> https://codereview.chromium.org/11752002/diff/4001/ui/gl/gl_surface_glx.cc
> File ui/gl/gl_surface_glx.cc (right):
> 
>
https://codereview.chromium.org/11752002/diff/4001/ui/gl/gl_surface_glx.cc#ne...
> ui/gl/gl_surface_glx.cc:151: int64 last_good_interval_;
> On 2013/01/04 15:34:43, ajuma wrote:
> > Nit: making this a TimeDelta instead of an int64 will save a couple of
> > conversions to/from microseconds.
> 
> Done.

[commit-bot: I haz the power, 2013-01-04 16:56:53]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[jonathan.backer, 2013-01-04 17:02:24]

+gman for a full committer review

ajuma@ wrote the original code, but he's not yet a full committer.

[apatrick_chromium, 2013-01-04 21:21:09]

Just nits. LGTM.

https://codereview.chromium.org/11752002/diff/15001/ui/gl/gl_surface_glx.cc
File ui/gl/gl_surface_glx.cc (right):

https://codereview.chromium.org/11752002/diff/15001/ui/gl/gl_surface_glx.cc#n...
ui/gl/gl_surface_glx.cc:61: last_timebase_(),
nit: this line is redundant.

https://codereview.chromium.org/11752002/diff/15001/ui/gl/gl_surface_glx.cc#n...
ui/gl/gl_surface_glx.cc:63: last_good_interval_() {
and here

https://codereview.chromium.org/11752002/diff/15001/ui/gl/gl_surface_glx.cc#n...
ui/gl/gl_surface_glx.cc:133: if (!last_timebase_.is_null()) {
indentation is out

[jonathan.backer, 2013-01-04 21:29:22]

https://codereview.chromium.org/11752002/diff/15001/ui/gl/gl_surface_glx.cc
File ui/gl/gl_surface_glx.cc (right):

https://codereview.chromium.org/11752002/diff/15001/ui/gl/gl_surface_glx.cc#n...
ui/gl/gl_surface_glx.cc:61: last_timebase_(),
On 2013/01/04 21:21:09, apatrick_chromium wrote:
> nit: this line is redundant.

Done.

https://codereview.chromium.org/11752002/diff/15001/ui/gl/gl_surface_glx.cc#n...
ui/gl/gl_surface_glx.cc:63: last_good_interval_() {
On 2013/01/04 21:21:09, apatrick_chromium wrote:
> and here

Done.

https://codereview.chromium.org/11752002/diff/15001/ui/gl/gl_surface_glx.cc#n...
ui/gl/gl_surface_glx.cc:133: if (!last_timebase_.is_null()) {
On 2013/01/04 21:21:09, apatrick_chromium wrote:
> indentation is out

Done.

[commit-bot: I haz the power, 2013-01-04 21:30:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/backer@chromium.org/11752002/19001

[commit-bot: I haz the power, 2013-01-04 23:37:58]

Change committed as 175234

[piman, 2013-01-08 21:08:06]

lgtm