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

No possibility of changing the surface PresentMode without recreating the whole Pixels instance #372

Closed
Kanabenki opened this issue Aug 7, 2023 · 3 comments · Fixed by #373
Labels
enhancement New feature or request

Comments

@Kanabenki
Copy link
Contributor

Pixels version: 0.13.0

I'm working on a project in which I would like to be able to toggle vsync on/off after my Pixels instance has been initialized. Currently, the only solution I found was to recreate my Pixels instance with this setting changed, using the Option<Pixels> workaround mentioned in #238 (it seems that problem in particular has been fixed on the main branch though)

Would it be possible for example to add some enable_vsync and set_present_mode methods on Pixels, similarly to what is currently exposed in PixelsBuilder. Those would set the present mode and reconfigure the wgpu surface. If that seems an ok proposal, I can submit a PR 🙂

@parasyte parasyte added the enhancement New feature or request label Aug 8, 2023
@parasyte
Copy link
Owner

parasyte commented Aug 8, 2023

Hi!

That seems like a feature that might be useful for some scenarios. I'd accept a PR.

I would caution about trying to simplify wgpu presentation modes to just a boolean VSync flag. We've had some problems from doing this in the past: #263. Particularly, VSync does not exist in web targets.

(it seems that problem in particular has been fixed on the main branch though)

Hmm, that is definitely not the case! #238 is still open for a reason.

@Kanabenki
Copy link
Contributor Author

I would caution about trying to simplify wgpu presentation modes to just a boolean VSync flag. We've had some problems from doing this in the past: #263. Particularly, VSync does not exist in web targets.

Indeed I saw this issue, for my use case this is not a problem though (I'm targeting desktop only). I'll submit a PR mirroring the builder API so that it can be set with the exact mode or a simple toggle, unless you plan to remove enable_vsync on the builder in the future.

Hmm, that is definitely not the case! #238 is still open for a reason.

My bad ! I saw 9325078 and thought it fixed that but it seems there are still some problems then ^^

@parasyte
Copy link
Owner

parasyte commented Aug 9, 2023

I saw 9325078 and thought it fixed that but it seems there are still some problems then ^^

Ah, yes. That's in an experimental branch! It can potentially fix the problem, but it's incomplete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants