Changed type of vsync from bool to Option<bool> #1230
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
goddessfreya
suggested changes
Oct 18, 2019
| @@ -181,7 +181,7 @@ impl<'a, T: ContextCurrentState> ContextBuilder<'a, T> { | |||
| /// | |||
| /// By default, vsync is not enabled. | |||
There was a problem hiding this comment.
Docs should be updated to say that None equals don't care.
| @@ -705,7 +705,7 @@ pub struct GlAttributes<S> { | |||
| /// screen tearing. | |||
| /// | |||
| /// The default is `false`. | |||
There was a problem hiding this comment.
Docs should be updated to say that None equals don't care.
| @@ -1086,19 +1086,21 @@ impl<'a> ContextPrototype<'a> { | |||
|
|
|||
| if let Some(surface) = surface { | |||
| // VSync defaults to enabled; disable it if it was not requested. | |||
There was a problem hiding this comment.
EGL by default has VSync enabled, and you have to explicitly disable it. Hope that makes sense.
| @@ -1294,8 +1296,12 @@ where | |||
| } | |||
|
|
|||
| // We're interested in those configs which allow our desired VSync. | |||
| let desired_swap_interval = if opengl.vsync { | |||
There was a problem hiding this comment.
This code as it exists contradicts the prior code. The prior code does not disable vsync when None is received, yet sets the desired_swap_interval to 0 (no vsync).
If we don't care about VSync, then it doesn't matter what the MIN_SWAP_INTERVAL/MAX_SWAP_INTERVAL equals, so we don't even need the filter.
| self.xconn.display as *mut _, | ||
| window, | ||
| 1, | ||
| ); |
| window, | ||
| ffi::glx_extra::SWAP_INTERVAL_EXT as i32, | ||
| &mut swap, | ||
| ); |
| } else { | ||
| return Err(CreationError::OsError( | ||
| "Couldn't find any available vsync extension".to_string(), | ||
| )); |
|
Merry Christmas. Closing this PR due to inactivity. Cheers! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
This is to address issue #631
I am not really sure about glutin/src/api/egl/mod.rs line 1089, if the Option is none, should it be treated as if the bool was false? The comment
// VSync defaults to enabled; disable it if it was not requested.seems to be at odds with the ones in lib.rs `/// By default, vsync is not enabled.