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

WebAssembly: fix the mouse issues when toggling the vsync mode and switching to full screen mode #9453

Merged
merged 33 commits into from
Feb 17, 2025

Conversation

oleg-derevenetz
Copy link
Collaborator

@oleg-derevenetz oleg-derevenetz commented Jan 15, 2025

Related to #9444 (comment)

This PR uses SDL_RenderSetVSync(), which is available since SDL 2.0.18 (that's why the Ubuntu version in the "Linux x86-64 SDL2 Release" workflow has been bumped from 20.04 to 22.04) instead of re-creating the renderer "on the fly" and also it removes a lot of cryptic code related to the SDL rendering.

Also it bumps the emsdk version to 4.0 latest and removes the workaround from our CI that was required to be able to build the multithreaded version of fheroes2.

As usual, you can see this PR running here:

https://oleg-derevenetz.github.io/fheroes2/
https://oleg-derevenetz.github.io/fheroes2/demo/

There are still some issues left - for instance, when trying to change the in-game resolution when the game running in full screen mode, there are some inadequate switching, stretching, etc., but I don't know yet what to do about it anyway.

@oleg-derevenetz oleg-derevenetz added improvement New feature, request or improvement Wasm WebAssembly version of the engine labels Jan 15, 2025
@oleg-derevenetz oleg-derevenetz added this to the 1.1.6 milestone Jan 15, 2025
@oleg-derevenetz oleg-derevenetz changed the title WebAssembly: fix the vsync and full screen modes WebAssembly: fix the vsync mode and switching to full screen mode Jan 16, 2025
@oleg-derevenetz oleg-derevenetz marked this pull request as ready for review January 16, 2025 16:27
@oleg-derevenetz oleg-derevenetz requested a review from ihhub January 16, 2025 16:27
Copy link
Owner

@ihhub ihhub left a comment

Choose a reason for hiding this comment

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

Hi @oleg-derevenetz , I put several comments in this pull request. Could you please take a look at them and see if they make sense?

@oleg-derevenetz oleg-derevenetz changed the title WebAssembly: fix the vsync mode and switching to full screen mode WebAssembly: fix the mouse issues when toggling the vsync mode and switching to full screen mode Jan 18, 2025
@oleg-derevenetz
Copy link
Collaborator Author

oleg-derevenetz commented Jan 18, 2025

@ihhub what do you think about playability of the wasm version in general (except for some rough edges maybe like the full-screen mode)? Is it good enough to consider this version as "general availability" and publish it on the project's website as a "demo" (in a way)? I'm not talking about the launcher right now, but only about the game itself.

@ihhub
Copy link
Owner

ihhub commented Jan 26, 2025

Hi @Branikolog , please prioritize this pull request.

@ihhub
Copy link
Owner

ihhub commented Feb 5, 2025

Hi @Branikolog , please complete your review by today.

@Branikolog
Copy link
Collaborator

Branikolog commented Feb 7, 2025

Hi!
Everything works fine, but I've noticed a strange thing, once I've switched to window mode and back to Fullscreen. When turning on/off v-sync screen partially shifts (I think it's related to my machine), and the window mode icon switches for a moment.

VID20250207174816.mp4

Not so critical issue, though. There're no any other problems on PC version.

@oleg-derevenetz
Copy link
Collaborator Author

once I've switched to window mode and back to Fullscreen

Full screen is seriously broken in wasm (and it's broken differently in different browsers), and I don't know yet how to cure it in a "cross-platform" way. It may have to be disabled altogether in the wasm version.

@Branikolog
Copy link
Collaborator

@oleg-derevenetz , hi.

once I've switched to window mode and back to Fullscreen

Full screen is seriously broken in wasm (and it's broken differently in different browsers), and I don't know yet how to cure it in a "cross-platform" way. It may have to be disabled altogether in the wasm version.

The issue I've filmed is valid for the PC version, since I'm not able to run application with browser.

And I'm convinced, that Fullscreen shouldn't be an option for the browser.

@oleg-derevenetz
Copy link
Collaborator Author

The issue I've filmed is valid for the PC version, since I'm not able to run application with browser.

Yes, I tried to run the game in the "true full-screen" mode (I mean, on Windows with hardware mouse cursor), and yes, when the SDL_RenderSetVSync() is called, there is a single frame rendered with some invalid contents. Sometimes it is apparently some "remembered" contents from the windowed mode, and, if app was never running in the windowed mode - i.e. it was started in the full-screen mode - it is just a black frame. This is especially noticeable during the step-by-step execution in the debugger. To be honest, I do not know if anything can be done about it. This does not happen in windowed mode or in the SDL_WINDOW_FULLSCREEN_DESKTOP ("non-true-fullscreen") mode.

@Branikolog
Copy link
Collaborator

@oleg-derevenetz

Yeah, forgot to mention, that the issue occurs for hardware cursor.
From my point it's mostly a cosmetic issue. I just mentioned it just to let you know and to check if nothing is broken there. 👍

Thanks for clarification!

@ihhub
Copy link
Owner

ihhub commented Feb 9, 2025

Hi @oleg-derevenetz , let's merge it after 1.1.6 release.

@ihhub ihhub modified the milestones: 1.1.6, 1.1.7 Feb 9, 2025
@ihhub ihhub merged commit 793ad1c into ihhub:master Feb 17, 2025
22 checks passed
@ihhub
Copy link
Owner

ihhub commented Feb 17, 2025

@oleg-derevenetz , thank you very much for the changes!

@oleg-derevenetz oleg-derevenetz deleted the ems-render branch February 17, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement New feature, request or improvement Wasm WebAssembly version of the engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants