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.

Sign up for GitHub

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

Jump to bottom

Changed type of vsync from bool to Option<bool> #1230

Closed
wants to merge 1 commit into from
Closed

Changed type of vsync from bool to Option<bool> #1230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions glutin/src/api/egl/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1086,19 +1086,21 @@ impl<'a> ContextPrototype<'a> {

if let Some(surface) = surface {
// VSync defaults to enabled; disable it if it was not requested.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EGL by default has VSync enabled, and you have to explicitly disable it. Hope that makes sense.

if !self.opengl.vsync {
let _guard = MakeCurrentGuard::new(
self.display,
surface,
surface,
context,
)
.map_err(|err| CreationError::OsError(err))?;

let egl = EGL.as_ref().unwrap();
unsafe {
if egl.SwapInterval(self.display, 0) == ffi::egl::FALSE {
panic!("finish_impl: eglSwapInterval failed: 0x{:x}", egl.GetError());
if let Some(vsync) = self.opengl.vsync {
if !vsync {
let _guard = MakeCurrentGuard::new(
self.display,
surface,
surface,
context,
)
.map_err(|err| CreationError::OsError(err))?;

let egl = EGL.as_ref().unwrap();
unsafe {
if egl.SwapInterval(self.display, 0) == ffi::egl::FALSE {
panic!("finish_impl: eglSwapInterval failed: 0x{:x}", egl.GetError());
}
}
}
}
Expand Down Expand Up @@ -1294,8 +1296,12 @@ where
}

// We're interested in those configs which allow our desired VSync.
let desired_swap_interval = if opengl.vsync {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

1
let desired_swap_interval = if let Some(vsync) = opengl.vsync {
if vsync {
1
} else {
0
}
} else {
0
};
Expand Down
88 changes: 45 additions & 43 deletions glutin/src/api/glx/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,51 +413,53 @@ impl<'a> ContextPrototype<'a> {
let (extra_functions, context) = self.create_context()?;

// vsync
if self.opengl.vsync {
let _guard = MakeCurrentGuard::new(&self.xconn, window, context)
.map_err(|err| CreationError::OsError(err))?;

if check_ext(&self.extensions, "GLX_EXT_swap_control")
&& extra_functions.SwapIntervalEXT.is_loaded()
{
// this should be the most common extension
unsafe {
extra_functions.SwapIntervalEXT(
self.xconn.display as *mut _,
window,
1,
);
}
if let Some(vsync) = self.opengl.vsync {
if vsync {
let _guard = MakeCurrentGuard::new(&self.xconn, window, context)
.map_err(|err| CreationError::OsError(err))?;

if check_ext(&self.extensions, "GLX_EXT_swap_control")
&& extra_functions.SwapIntervalEXT.is_loaded()
{
// this should be the most common extension
unsafe {
extra_functions.SwapIntervalEXT(
self.xconn.display as *mut _,
window,
1,
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: formating?

}

let mut swap = unsafe { std::mem::zeroed() };
unsafe {
glx.QueryDrawable(
self.xconn.display as *mut _,
window,
ffi::glx_extra::SWAP_INTERVAL_EXT as i32,
&mut swap,
);
}
let mut swap = unsafe { std::mem::zeroed() };
unsafe {
glx.QueryDrawable(
self.xconn.display as *mut _,
window,
ffi::glx_extra::SWAP_INTERVAL_EXT as i32,
&mut swap,
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: formating?

}

if swap != 1 {
return Err(CreationError::OsError(format!("Couldn't setup vsync: expected interval `1` but got `{}`", swap)));
}
} else if check_ext(&self.extensions, "GLX_MESA_swap_control")
&& extra_functions.SwapIntervalMESA.is_loaded()
{
unsafe {
extra_functions.SwapIntervalMESA(1);
}
} else if check_ext(&self.extensions, "GLX_SGI_swap_control")
&& extra_functions.SwapIntervalSGI.is_loaded()
{
unsafe {
extra_functions.SwapIntervalSGI(1);
}
} else {
return Err(CreationError::OsError(
"Couldn't find any available vsync extension".to_string(),
));
if swap != 1 {
return Err(CreationError::OsError(format!("Couldn't setup vsync: expected interval `1` but got `{}`", swap)));
}
} else if check_ext(&self.extensions, "GLX_MESA_swap_control")
&& extra_functions.SwapIntervalMESA.is_loaded()
{
unsafe {
extra_functions.SwapIntervalMESA(1);
}
} else if check_ext(&self.extensions, "GLX_SGI_swap_control")
&& extra_functions.SwapIntervalSGI.is_loaded()
{
unsafe {
extra_functions.SwapIntervalSGI(1);
}
} else {
return Err(CreationError::OsError(
"Couldn't find any available vsync extension".to_string(),
));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: formating?

}
}
}

Expand Down
6 changes: 3 additions & 3 deletions glutin/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl<'a, T: ContextCurrentState> ContextBuilder<'a, T> {
///
/// By default, vsync is not enabled.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs should be updated to say that None equals don't care.

#[inline]
pub fn with_vsync(mut self, vsync: bool) -> Self {
pub fn with_vsync(mut self, vsync: Option<bool>) -> Self {
self.gl_attr.vsync = vsync;
self
}
Expand Down Expand Up @@ -705,7 +705,7 @@ pub struct GlAttributes<S> {
/// screen tearing.
///
/// The default is `false`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs should be updated to say that None equals don't care.

pub vsync: bool,
pub vsync: Option<bool>,
}

impl<S> GlAttributes<S> {
Expand Down Expand Up @@ -748,7 +748,7 @@ impl<S> Default for GlAttributes<S> {
profile: None,
debug: cfg!(debug_assertions),
robustness: Robustness::NotRobust,
vsync: false,
vsync: Some(false),
}
}
}
Expand Down