Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not run faster than 60 FPS #25312

Closed
wants to merge 1 commit into from
Closed

Conversation

@paulrouget
Copy link
Contributor

paulrouget commented Dec 17, 2019

Fix #25305

@atouchet
Copy link
Contributor

atouchet commented Dec 17, 2019

Does this limit Servo to running at 60 FPS? Would it make sense to instead use the display's refresh rate?

@paulrouget
Copy link
Contributor Author

paulrouget commented Dec 17, 2019

If the monitor is cadenced at 120 FPS, it will force Servo to run at 60 FPS.

We could rely on https://docs.rs/winit/0.20.0-alpha4/winit/monitor/struct.VideoMode.html#method.refresh_rate

We should also maybe only sleep if no call to compositor::composite happen.

This is an improvement non the less.

@atouchet
Copy link
Contributor

atouchet commented Dec 17, 2019

If the monitor is cadenced at 120 FPS, it will force Servo to run at 60 FPS.

What about displays that don't run at multiples of 60 (ex. 75, 90, 144, etc.)?

@paulrouget
Copy link
Contributor Author

paulrouget commented Dec 17, 2019

Servo will always run at 60FPS max.

This is not the best approach. Open to suggestions.

@atouchet
Copy link
Contributor

atouchet commented Dec 17, 2019

Using that winit method instead of assuming 60 seems like a reasonable idea to me.

@Manishearth
Copy link
Member

Manishearth commented Dec 17, 2019

cc @asajeffrey thoughts on this?

Copy link
Member

Manishearth left a comment

Approach seems fine by me but I'm not sure if this is the right way to do this.

@asajeffrey
Copy link
Member

asajeffrey commented Dec 19, 2019

Can we avoid blocking the main thread? The embedder may have things they want to do on the main thread at more than 60fps. If nothing else we should make the number 60 configurable, e.g. for devices that run at 120fps.

@paulrouget
Copy link
Contributor Author

paulrouget commented Feb 20, 2020

Closing this as this is starting to look more complex that I initially imagined. See #25305.

@paulrouget paulrouget closed this Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.