1138627 - Convert layout.frame_rate into a boolean ASAP mode flag

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Description

[Mason Chang [Inactive] [:mchang]]

From https://bugzilla.mozilla.org/show_bug.cgi?id=1128690#c12

Instead of having a specific frame rate, we should only have two paths:

1) Vsync rate
2) Talos ASAP mode

Convert the compositor and refresh driver code which detects which rate to use and instead just convert them to either be in vsync mode or ASAP mode.

Comment 1

[Avi Halachmi (:avih)]

Actually, changing the semantics (and type) of an existing pref is undesirable IMO.

Talos can be modified to use some new silk-ASAP pref, and to stay backward compatible with older Firefox builds, it can set both the layout.frame_rate one AND the new silk-ASAP pref - that is assuming the new silk code ignores the old pref completely.

Also, regardless of ASAP, can the SW silk generator use some arbitrary rate from a pref the same way which layout.frame_rate can? As far as I can tell, the three gfx.vsync.* prefs are all boolean. IMO it's desirable to keep such feature.

So when combining the above, why not just keep the semantics of layout.frame_rate as is?

-1 == auto (HW with fallback 60 Hz SW)
 0 == ASAP
NN == Custom SW rate

Comment 2

[Mason Chang [Inactive] [:mchang]]

(In reply to Avi Halachmi (:avih) from comment #1)
> Actually, changing the semantics (and type) of an existing pref is
> undesirable IMO.
> 
> Talos can be modified to use some new silk-ASAP pref, and to stay backward
> compatible with older Firefox builds, it can set both the layout.frame_rate
> one AND the new silk-ASAP pref - that is assuming the new silk code ignores
> the old pref completely.

It doesn't really ignore the preference right now, but maybe we can change that. Sure we can do a new pref so talos changes can ride the changes as well. Do we have separate talos configurations per release? e.g. 39 is different than 40 right now.

> 
> Also, regardless of ASAP, can the SW silk generator use some arbitrary rate
> from a pref the same way which layout.frame_rate can? As far as I can tell,
> the three gfx.vsync.* prefs are all boolean. IMO it's desirable to keep such
> feature.

Yes, the Software silk generator can use arbitrary rates, but we really want to test hardware vsync. Software really shouldn't be used, and we can't control the hardware vsync rates.

Comment 3

[Mason Chang [Inactive] [:mchang]]

Noting that we're targeting Gecko 42 for the removal of non-silk mechanisms.

Comment 4

[Avi Halachmi (:avih)]

Assuming Silk will not be able to use HW vsync on all systems (e.g. Windows XP, some Linux systems, possibly others), and on such case it will fallback always to 60Hz, how important is it to keep the custom rate configurable and not fixed to 60?

The approach this bug suggest is to eliminate the option to use rates other than 60 hz (unless the user rebuilds firefox and use other hardcoded rate). Custom rates are useful in testing, but much more importantly on systems where HW vsync is not available.

I think we should not remove custom rates (as a pref).

Thoughts?

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

I'd like to have a pref for custom rates.

Comment 6

[J-Mackerel]

Hi! Thanks for all your hard work. Silk on nightly is an amazing experience! However, I have some small concern about getting rid of layout.frame_rate

I have used (and tested) some systems that cannot support HW v-sync, and are also connected to displays with weird refresh rates. A hard-coded 60 Hz refresh rate would yield an inferior experience. Would it be possible to use this pref as a fallback rate that defaults to 60 Hz, in case hw v-sync fails? If that's confusing, perhaps the pref could be renamed to "layout.user_specified_frame_rate". 

I think the following steps would insure an optimal Silk experience for almost every platform:

If Firefox is in Chaos Mode, choose a random, variable refresh rate (1-1000)
Else if the number in "layout.user_specified_frame_rate" is between (1-300), use that exclusively.
Else if HW vsync succeeds, use it and set "layout.user_specified_frame_rate" to -1
Else if HW vsync fails, ask the OS for the monitor refresh rate and use it to generate vsync events (note that "layout.user_specified_frame_rate" is still -1)
Else everything has failed, set "layout.user_specified_frame_rate" to 60 Hz and start Silk

I'm sorry this is kind of bug spam, but I'm not brave enough for IRC. Would it be ok if I filed a separate, enhancement bug for this?

Thank you.

Comment 7

[Mason Chang [Inactive] [:mchang]]

(In reply to J-Mackerel from comment #6)
> Hi! Thanks for all your hard work. Silk on nightly is an amazing experience!
> However, I have some small concern about getting rid of layout.frame_rate
> 
> I have used (and tested) some systems that cannot support HW v-sync, and are
> also connected to displays with weird refresh rates. A hard-coded 60 Hz
> refresh rate would yield an inferior experience. Would it be possible to use
> this pref as a fallback rate that defaults to 60 Hz, in case hw v-sync
> fails? If that's confusing, perhaps the pref could be renamed to
> "layout.user_specified_frame_rate". 

I think that is the intended idea. If hardware vsync fails, we'll use a configurable software vsync. This preference exists today as is. Previously, before silk, we would still default to 60 fps. Are you currently changing the frame rate and using it on a daily basis, or is it limited to testing?

> 
> I think the following steps would insure an optimal Silk experience for
> almost every platform:
> 
> If Firefox is in Chaos Mode, choose a random, variable refresh rate (1-1000)

See bug 1135136.

> Else if the number in "layout.user_specified_frame_rate" is between (1-300),
> use that exclusively.
> Else if HW vsync succeeds, use it and set "layout.user_specified_frame_rate"
> to -1
> Else if HW vsync fails, ask the OS for the monitor refresh rate and use it
> to generate vsync events (note that "layout.user_specified_frame_rate" is
> still -1)
> Else everything has failed, set "layout.user_specified_frame_rate" to 60 Hz
> and start Silk
> 
> I'm sorry this is kind of bug spam, but I'm not brave enough for IRC. Would
> it be ok if I filed a separate, enhancement bug for this?
> 

No problem! As long as you're contributing! :) Sure if you really need this preference. I'd be interested to see how often you are using this preference and when / why. What's the experience like on non-60 HZ machines. Is the problem mostly on high refresh rate monitors? 

> Thank you.

Comment 8

[J-Mackerel]

(In reply to Mason Chang [:mchang] from comment #7)
> I think that is the intended idea. If hardware vsync fails, we'll use a
> configurable software vsync. This preference exists today as is. Previously,
> before silk, we would still default to 60 fps. Are you currently changing
> the frame rate and using it on a daily basis, or is it limited to testing?

I support weird A/V gear (TVs and projectors @ 24Hz and 30Hz), and some workstations (75 - 120 Hz displays). I change the value frequently for testing. However, after discovering an optimal refresh rate, I usually just "set and forget", because we don't replace projectors and signage displays that often. Because I do not control OS images, sometimes, we get stuck with a busted driver that is (CORRECTLY) blocked by Firefox. When Silk reaches release, I'd still like to be able to force a different layout refresh rate for software mode on fast computers.

> See bug 1135136.

Thanks!

> No problem! As long as you're contributing! :) Sure if you really need this
> preference. I'd be interested to see how often you are using this preference
> and when / why. What's the experience like on non-60 HZ machines. Is the
> problem mostly on high refresh rate monitors? 
> 

60Hz on a low refresh display causes a weird smearing effect, because multiple frames blur together. Manually lowering the refresh rate seems to fix that. 60 Hz on a high refresh display that is NOT a multiple of 60 cause a very subtle sort of judder, because the frames do not display consistently. Firefox is still responsive, but feels like it's lagging behind the desktop and other apps. It's subtle, but hard to ignore when one notices it.

Comment 9

[J-Mackerel]

Filed bug 1163301 and 1163303 as enhancements. Apologies for any bugspam.

Comment 10

[Mason Chang [Inactive] [:mchang]]

From https://bugzilla.mozilla.org/show_bug.cgi?id=1163303#c1, I think I'll keep the custom rates and if a custom rate exists, default to software vsync at that custom rate. Closing this as WONTFIX since we'll still keep custom rates around.