enable_vsync(false) is not guaranteed

[wiz21b]

Hello,

I've tried to use enable_vsync(false) on the builder and it seems it is not acknowledged.

    let mut pixels = PixelsBuilder::new(SCREEN_WIDTH, SCREEN_HEIGHT, surface_texture)
	.request_adapter_options(RequestAdapterOptions {
            power_preference: PowerPreference::HighPerformance,
	    force_fallback_adapter: false,
            compatible_surface: None,
	})
	.enable_vsync(false)
	//.present_mode(PresentMode::Immediate)
	.build().expect("Something went wrong");

When rendering, this specific code :

		let now_pixels = Instant::now();
                if pixels
		    .render()
		    .map_err(|e| error!("pixels.render() failed: {}", e))
		    .is_err()
                {
		    *control_flow = ControlFlow::Exit;
		    return;
                }
		let pixels_elapsed = now_pixels.elapsed();

always makes sure the VBL is waited for (and it shouldn't since vbsync is false).

Now, looking at wgpu's documentation, one can read : "FIFO is the only guaranteed to be supported, though other formats will automatically fall back to FIFO.".

So maybe that's the issue ?

Could enable_vsync() say something when other formats are not supported ? That'd help new comers like me, it took me 2 hours to figure this out (but using your lib spared me days of development, so all in all, it's a huge win :-) )

Thanks !

[parasyte]

Unfortunately, we are at the mercy of what wgpu offers, and wgpu is at the mercy of what Vulkan and GPU drivers offer.

In retrospect, enable_vsync() may not have been an ideal API method, since it is a boolean and the underlying library actually has a tristate with some of those states being platform dependent. We already have present_mode() as a suitable replacement; it gives full access to set the wgpu presentation mode tristate. As you mention, it also requires the caller to familiarize themselves with wgpu requirements and limitation of the available presentation modes. That is the primary reason that enable_vsync() tries to simplify it to an analog that game devs are likely to be more familiar with.

The best solution might be removing enable_vsync() entirely, since it is guaranteed misleading on some platforms (especially mobile and web). And try to improve documentation around present_mode() to set expectations.

it took me 2 hours to figure this out (but using your lib spared me days of development, so all in all, it's a huge win :-) )

Sorry that this caused your time loss! I truly understand that and have felt the impact of similar issues ... well, pretty much every single day, if I'm honest. But I'm glad you also took the time to report this, so that we can make improvements!

[parasyte]

With wgpu 0.14, we have some additional options for how to handle vertical sync. PresentMode now has two new variants: AutoVsync and AutoNoVsync which will attempt to choose the best available presentation mode that matches the expectation of whether it blocks for the display's vertical refresh.

Fixing this issue should be as simple as choosing either of these variants based on the value of the enable_vsync bool.

[parasyte]

I also added a comment to the docs about how Vsync cannot be disabled on Web targets, which is in alignment with the original comment in this thread.