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

Fixes for initialization and glasses state management #65

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

patrickdown
Copy link
Collaborator

In the process of working on a Linux version some problems were found in initialization and glasses state management. Since they are relevant to other platforms I've pulled these fixes together here. I have excluded changes to build-on-push.yml because the Linux version of the plugin still has some issues to work out.

Problems and fixes:

  1. A race condition was found in initialization that would cause rendering to start before frame textures had been created. This was caused when the READY state flipped between the update_connection() call and the get_events() call. The solution implemented was to move the graphics initialization into the start_display() function call chain. Since this is the function that starts rendering it can ensure that textures are setup first.
  1. Although it has never seemed to come up, there are situations where previous state variable might be reset before changes could be detected. The fix for this was stop using a single var for this and track separate vars in each place changes needed to be detected.

  2. There are a few minor changes to account for differences in C++ library functions between Windows and Linux.

  3. Update SConstruct for Linux build

In the process of working on a Linux version some problems were found in initialization and glasses state management. Since they are relevant to other platforms I've pulled these fixes together here. I have excluded changes to `build-on-push.yml` because the Linux version of the plugin still has some issues to work out.

Problems and fixes:
1) A race condition was found in initialization that would cause rendering to start before frame textures had been created. This was caused when the READY state flipped between the `update_connection()` call and the `get_events()` call. The solution implemented was to move the graphics initialization into the `start_display()` function call chain. Since this is the function that starts rendering it can ensure that textures are setup first.

2. Although it has never seemed to come up, there are situations where previous state variable might be reset before changes could be detected. The fix for this was stop using a single var for this and track separate vars in each place changes needed to be detected.

3. Finally there are a few minor changes to account for differences in C++ library functions between Windows and Linux.
Copy link
Collaborator

@Malcolmnixon Malcolmnixon left a comment

Choose a reason for hiding this comment

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

Looks good to me. It's a pain we had to drop back from std::format to snprintf, but if GCC doesn't like it then oh well. We need to have good compiler support.

@patrickdown patrickdown merged commit 758e5c4 into GodotVR:main Dec 6, 2023
2 checks passed
@patrickdown patrickdown deleted the fix_initialization_order branch December 6, 2023 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants