Feature: don't cap bevy framerate when out of focus #52

2025-08-17T16:20:07-04:00

nondescriptpointer commented

2025-08-17 16:20:07 -04:00

(Migrated from github.com)

Bevy's default behavior limits the framerate when the window is out of focus. Does not seem relevant for our use case, especially during debugging and profiling.

Summary by CodeRabbit

New Features
- Automatically enables flatscreen mode when a desktop display environment is detected.
- Ensures continuous updates in flatscreen mode even when the app window is unfocused, improving smoothness.
- Improves input handling for flatscreen mode for a more responsive desktop experience.
Refactor
- Centralizes flatscreen mode detection behind a single environment-aware flag to streamline initialization and plugin setup.

Bevy's default behavior limits the framerate when the window is out of focus. Does not seem relevant for our use case, especially during debugging and profiling.  ## Summary by CodeRabbit * **New Features** * Automatically enables flatscreen mode when a desktop display environment is detected. * Ensures continuous updates in flatscreen mode even when the app window is unfocused, improving smoothness. * Improves input handling for flatscreen mode for a more responsive desktop experience. * **Refactor** * Centralizes flatscreen mode detection behind a single environment-aware flag to streamline initialization and plugin setup.

coderabbitai[bot] commented

2025-08-17 16:20:12 -04:00

(Migrated from github.com)

📝 Walkthrough

Walkthrough

Compute an environment-aware flatscreenmode from CLI args or DISPLAY/WAYLAND_DISPLAY and, when true, register WinitPlugin (WakeUp, run_on_any_thread), add FlatscreenInputPlugin, and insert WinitSettings with focused/unfocused UpdateMode::Continuous; also extend imports for WinitSettings and UpdateMode.

Changes

Cohort / File(s)	Summary of edits
Runtime mode & Winit integration `src/main.rs`	Compute `flatscreenmode = args.flatscreen

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI args
    participant Env as Environment
    participant App as App builder
    participant Winit as WinitPlugin
    participant Input as FlatscreenInputPlugin

    CLI->>App: parse args.flatscreen
    Env->>App: check DISPLAY / WAYLAND_DISPLAY
    App->>App: flatscreenmode = args.flatscreen || DISPLAY || WAYLAND_DISPLAY
    alt flatscreenmode == true
        App->>Winit: add_plugin(WinitPlugin{WakeUp, run_on_any_thread})
        App->>App: insert_resource(WinitSettings{focused_mode=Continuous, unfocused_mode=Continuous})
        App->>Input: add_plugin(FlatscreenInputPlugin)
    else
        App->>App: proceed without Winit/Flatscreen plugins
    end
    App->>App: continue existing initialization

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

Visit our Status Page to check the current availability of CodeRabbit.
Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

<details> <summary>📝 Walkthrough</summary> ## Walkthrough Compute an environment-aware flatscreenmode from CLI args or DISPLAY/WAYLAND_DISPLAY and, when true, register WinitPlugin (WakeUp, run_on_any_thread), add FlatscreenInputPlugin, and insert WinitSettings with focused/unfocused UpdateMode::Continuous; also extend imports for WinitSettings and UpdateMode. ## Changes | Cohort / File(s) | Summary of edits | |---|---| | **Runtime mode & Winit integration**<br>`src/main.rs` | Compute `flatscreenmode = args.flatscreen || DISPLAY || WAYLAND_DISPLAY`. When true: add `WinitPlugin` (WakeUp, `run_on_any_thread`), add `FlatscreenInputPlugin`, insert `WinitSettings` with `focused_mode` and `unfocused_mode` set to `UpdateMode::Continuous`. Extend imports to include `WinitSettings` and `UpdateMode`. Other initialization unchanged. | ## Sequence Diagram(s) ```mermaid sequenceDiagram participant CLI as CLI args participant Env as Environment participant App as App builder participant Winit as WinitPlugin participant Input as FlatscreenInputPlugin CLI->>App: parse args.flatscreen Env->>App: check DISPLAY / WAYLAND_DISPLAY App->>App: flatscreenmode = args.flatscreen || DISPLAY || WAYLAND_DISPLAY alt flatscreenmode == true App->>Winit: add_plugin(WinitPlugin{WakeUp, run_on_any_thread}) App->>App: insert_resource(WinitSettings{focused_mode=Continuous, unfocused_mode=Continuous}) App->>Input: add_plugin(FlatscreenInputPlugin) else App->>App: proceed without Winit/Flatscreen plugins end App->>App: continue existing initialization ``` ## Estimated code review effort 🎯 3 (Moderate) | ⏱️ ~20 minutes </details>   > [!TIP] > <details> > <summary>🔌 Remote MCP (Model Context Protocol) integration is now available!</summary> > > Pro plan users can now connect to remote MCP servers from the [Integrations](https://app.coderabbit.ai/integrations) page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. > > </details>      <details> <summary>✨ Finishing Touches</summary> - [ ]  📝 Generate Docstrings <details> <summary>🧪 Generate unit tests</summary> - [ ]  Create PR with unit tests - [ ]  Post copyable unit tests in a comment </details> </details>   --- Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> <details> <summary>🪧 Tips</summary> ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=StardustXR/server&utm_content=52): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit <commit_id>, please review it.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` ### Support Need help? Create a ticket on our [support page](https://www.coderabbit.ai/contact-us/support) for assistance with any issues or questions. ### CodeRabbit Commands (Invoked using PR/Issue comments) Type `@coderabbitai help` to get the list of available commands. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Status, Documentation and Community - Visit our [Status Page](https://status.coderabbit.ai) to check the current availability of CodeRabbit. - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements. </details>

coderabbitai[bot] (Migrated from github.com) reviewed 2025-08-17 16:25:26 -04:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 1

♻️ Duplicate comments (1)

src/main.rs (1)

322-328: Avoid enabling FlatscreenInput/Winit when XR is selected elsewhere; unify gating as above

This block uses flatscreenmode, but the later XR/flatscreen selection still uses args.flatscreen. If env vars enable flatscreenmode while args.flatscreen is false, you’ll attach flatscreen input and Winit and then proceed down the XR path.

Align the gating (see previous comment) to prevent mixed-mode initialization.

🧹 Nitpick comments (1)

src/main.rs (1)
381-387: Insert WinitSettings before plugin registration to guarantee it’s in place when Winit initializes

This achieves the PR goal (no unfocused cap) by setting both focused and unfocused to Continuous. That said:

To ensure Winit picks it up deterministically, insert the resource before app.add_plugins(...) (i.e., before WinitPlugin is built).

If you only care about unfocused behavior, you can keep the focused setting at Bevy’s default (usually already Continuous), but setting both won’t hurt.

Apply within this hunk:
-    if flatscreenmode {
-            app.insert_resource(WinitSettings {
-            focused_mode: UpdateMode::Continuous,
-            unfocused_mode: UpdateMode::Continuous,
-            ..Default::default()
-        });
-    }
+    // moved earlier (see snippet below) to ensure it's present before plugin construction
And add (outside this hunk), just after computing flatscreenmode and before app.add_plugins(...):
if flatscreenmode {
    app.insert_resource(WinitSettings {
        focused_mode: UpdateMode::Continuous,
        unfocused_mode: UpdateMode::Continuous,
        ..Default::default()
    });
}
Note: Continuous + AutoNoVsync can significantly increase CPU/GPU utilization when unfocused. If that’s intentional (as per PR), this is fine.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 877a32ab09 and d1043f7b8c.

📒 Files selected for processing (1)

src/main.rs (3 hunks)

🔇 Additional comments (1)

src/main.rs (1)

30-30: Imports LGTM; aligned with Bevy’s winit configuration APIs

WinitSettings and UpdateMode are the right knobs for decoupling framerate from window focus.

**Actionable comments posted: 1** <details> <summary>♻️ Duplicate comments (1)</summary><blockquote> <details> <summary>src/main.rs (1)</summary><blockquote> `322-328`: **Avoid enabling FlatscreenInput/Winit when XR is selected elsewhere; unify gating as above** This block uses `flatscreenmode`, but the later XR/flatscreen selection still uses `args.flatscreen`. If env vars enable `flatscreenmode` while `args.flatscreen` is false, you’ll attach flatscreen input and Winit and then proceed down the XR path. Align the gating (see previous comment) to prevent mixed-mode initialization. </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>src/main.rs (1)</summary><blockquote> `381-387`: **Insert `WinitSettings` before plugin registration to guarantee it’s in place when Winit initializes** This achieves the PR goal (no unfocused cap) by setting both focused and unfocused to `Continuous`. That said: - To ensure Winit picks it up deterministically, insert the resource before `app.add_plugins(...)` (i.e., before WinitPlugin is built). - If you only care about unfocused behavior, you can keep the focused setting at Bevy’s default (usually already Continuous), but setting both won’t hurt. Apply within this hunk: ```diff - if flatscreenmode { - app.insert_resource(WinitSettings { - focused_mode: UpdateMode::Continuous, - unfocused_mode: UpdateMode::Continuous, - ..Default::default() - }); - } + // moved earlier (see snippet below) to ensure it's present before plugin construction ``` And add (outside this hunk), just after computing `flatscreenmode` and before `app.add_plugins(...)`: ```rust if flatscreenmode { app.insert_resource(WinitSettings { focused_mode: UpdateMode::Continuous, unfocused_mode: UpdateMode::Continuous, ..Default::default() }); } ``` Note: Continuous + AutoNoVsync can significantly increase CPU/GPU utilization when unfocused. If that’s intentional (as per PR), this is fine. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 877a32ab09ce1fb77b0a8e79433e85465ecee288 and d1043f7b8cb23ae76592410ffd0b58f9e6060b6f. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `src/main.rs` (3 hunks) </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>src/main.rs (1)</summary> `30-30`: **Imports LGTM; aligned with Bevy’s winit configuration APIs** `WinitSettings` and `UpdateMode` are the right knobs for decoupling framerate from window focus. </details> </blockquote></details> </details>

src/main.rs

coderabbitai[bot] (Migrated from github.com) commented

2025-08-17 16:25:25 -04:00

💡 Verification agent

🧩 Analysis chain

Gating inconsistency: flatscreenmode vs args.flatscreen can enable Winit + flatscreen input while still choosing the XR path

You compute flatscreenmode from CLI args or DISPLAY/WAYLAND_DISPLAY, but later the XR/flatscreen branch still keys off args.flatscreen (Lines 330–364). This can produce a mixed configuration:

WinitPlugin + FlatscreenInputPlugin get added when flatscreenmode == true due to env vars,
but the XR plugin path is still taken if args.flatscreen == false.

That risks running XR with a Winit event loop and flatscreen input enabled, which is likely unintended and could cause subtle event loop/input conflicts.

Action:

Use the same gating variable consistently. Either:
- Replace if !args.flatscreen { ... } else { ... } with if !flatscreenmode { ... } else { ... }, or
- Revert the earlier Winit/FlatscreenInput additions to gate strictly on args.flatscreen.

Additionally, please confirm that auto-enabling “flatscreen mode” based on env vars alone is desired behavior for your use case—this modifies previous semantics and broadens when flatscreen plugins are active, which goes beyond the PR objective of lifting the unfocused framerate cap.

Example (outside this hunk) for the branch below at Lines 330–364:

// Use flatscreenmode here for consistency
app.add_plugins(
    if !flatscreenmode {
        // XR path
        add_xr_plugins(plugins)
            /* ... */
    } else {
        // flatscreen path
        plugins = plugins.add(XrSessionPlugin { auto_handle: false });
        bevy_dmabuf::wgpu_init::add_dmabuf_init_plugin(plugins)
    }
    /* ... */
);

🏁 Script executed:

#!/usr/bin/env bash
# Verify where flatscreenmode is used
rg -n 'if flatscreenmode' -C3 --type=rs src/main.rs

# Verify where args.flatscreen is used for gating
rg -n 'if !args\.flatscreen' -C5 --type=rs src/main.rs

Length of output: 164

🏁 Script executed:

#!/usr/bin/env bash
# Locate flatscreenmode definition and usage
rg -n 'flatscreenmode' -C3 src/main.rs

# Locate args.flatscreen checks for gating
rg -n 'args\.flatscreen' -C5 src/main.rs

Length of output: 1559

Unify gating to use flatscreenmode
Your code computes flatscreenmode (from args.flatscreen or DISPLAY/WAYLAND env vars) but later gates the XR path on only args.flatscreen. This allows Winit + FlatscreenInputPlugin to be enabled via env vars while still taking the XR branch, leading to mixed, likely unintended configurations.

Please:

In src/main.rs around line 330, change:

- if !args.flatscreen {
+ if !flatscreenmode {

Ensure all plugin‐selection branches (Winit/FlatscreenInputPlugin and XR) use the same flatscreenmode variable.
Confirm that auto-enabling flatscreen mode purely via DISPLAY/WAYLAND_DISPLAY is desired (this broadens plugin activation beyond the CLI flag).

🤖 Prompt for AI Agents

In src/main.rs around lines 318 to ~330, the code computes flatscreenmode but
later only checks args.flatscreen when choosing between
Winit/FlatscreenInputPlugin and the XR branch, allowing env-var-triggered
flatscreen to enable Winit while still taking the XR path; change the gating so
all plugin-selection branches use the computed flatscreenmode variable instead
of args.flatscreen (i.e., replace any remaining checks of args.flatscreen with
flatscreenmode) so the Winit/FlatscreenInputPlugin vs XR choice is consistently
controlled by the same flag and confirm this behavior is intended for
DISPLAY/WAYLAND_DISPLAY auto-enabling.

_💡 Verification agent_ <details> <summary>🧩 Analysis chain</summary> **Gating inconsistency: `flatscreenmode` vs `args.flatscreen` can enable Winit + flatscreen input while still choosing the XR path** You compute `flatscreenmode` from CLI args or `DISPLAY`/`WAYLAND_DISPLAY`, but later the XR/flatscreen branch still keys off `args.flatscreen` (Lines 330–364). This can produce a mixed configuration: - WinitPlugin + FlatscreenInputPlugin get added when `flatscreenmode == true` due to env vars, - but the XR plugin path is still taken if `args.flatscreen == false`. That risks running XR with a Winit event loop and flatscreen input enabled, which is likely unintended and could cause subtle event loop/input conflicts. Action: - Use the same gating variable consistently. Either: - Replace `if !args.flatscreen { ... } else { ... }` with `if !flatscreenmode { ... } else { ... }`, or - Revert the earlier Winit/FlatscreenInput additions to gate strictly on `args.flatscreen`. Additionally, please confirm that auto-enabling “flatscreen mode” based on env vars alone is desired behavior for your use case—this modifies previous semantics and broadens when flatscreen plugins are active, which goes beyond the PR objective of lifting the unfocused framerate cap. Example (outside this hunk) for the branch below at Lines 330–364: ```rust // Use flatscreenmode here for consistency app.add_plugins( if !flatscreenmode { // XR path add_xr_plugins(plugins) /* ... */ } else { // flatscreen path plugins = plugins.add(XrSessionPlugin { auto_handle: false }); bevy_dmabuf::wgpu_init::add_dmabuf_init_plugin(plugins) } /* ... */ ); ``` --- 🏁 Script executed: ```shell #!/usr/bin/env bash # Verify where flatscreenmode is used rg -n 'if flatscreenmode' -C3 --type=rs src/main.rs # Verify where args.flatscreen is used for gating rg -n 'if !args\.flatscreen' -C5 --type=rs src/main.rs ``` Length of output: 164 --- 🏁 Script executed: ```shell #!/usr/bin/env bash # Locate flatscreenmode definition and usage rg -n 'flatscreenmode' -C3 src/main.rs # Locate args.flatscreen checks for gating rg -n 'args\.flatscreen' -C5 src/main.rs ``` Length of output: 1559 --- </details> **Unify gating to use `flatscreenmode`** Your code computes `flatscreenmode` (from `args.flatscreen` or DISPLAY/WAYLAND env vars) but later gates the XR path on **only** `args.flatscreen`. This allows Winit + FlatscreenInputPlugin to be enabled via env vars while still taking the XR branch, leading to mixed, likely unintended configurations. Please: - In `src/main.rs` around line 330, change: ```diff - if !args.flatscreen { + if !flatscreenmode { ``` - Ensure **all** plugin‐selection branches (Winit/FlatscreenInputPlugin and XR) use the same `flatscreenmode` variable. - Confirm that auto-enabling flatscreen mode purely via `DISPLAY`/`WAYLAND_DISPLAY` is desired (this broadens plugin activation beyond the CLI flag). <details> <summary>🤖 Prompt for AI Agents</summary> ``` In src/main.rs around lines 318 to ~330, the code computes flatscreenmode but later only checks args.flatscreen when choosing between Winit/FlatscreenInputPlugin and the XR branch, allowing env-var-triggered flatscreen to enable Winit while still taking the XR path; change the gating so all plugin-selection branches use the computed flatscreenmode variable instead of args.flatscreen (i.e., replace any remaining checks of args.flatscreen with flatscreenmode) so the Winit/FlatscreenInputPlugin vs XR choice is consistently controlled by the same flag and confirm this behavior is intended for DISPLAY/WAYLAND_DISPLAY auto-enabling. ``` </details>

Schmarni-Dev (Migrated from github.com) approved these changes 2025-08-17 18:20:26 -04:00

src/main.rs

						
				@@ -378,6 +378,13 @@ fn bevy_loop(

						}),

Schmarni-Dev (Migrated from github.com) commented

2025-08-17 18:19:43 -04:00

i don't think there's a reason to gate this behind flatscreen mode? i think it would be better to just insert that resource always

nondescriptpointer (Migrated from github.com) reviewed 2025-08-18 12:22:06 -04:00

src/main.rs

						
				@@ -378,6 +378,13 @@ fn bevy_loop(

						}),

nondescriptpointer (Migrated from github.com) commented

2025-08-18 12:22:05 -04:00

If not in flatscreen mode bevy_oxr already inserts this resource:
https://github.com/awtterpip/bevy_oxr/blob/main/crates/bevy_openxr/src/openxr/init.rs

If not in flatscreen mode bevy_oxr already inserts this resource: https://github.com/awtterpip/bevy_oxr/blob/main/crates/bevy_openxr/src/openxr/init.rs

coderabbitai[bot] (Migrated from github.com) reviewed 2025-08-18 12:25:42 -04:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 0

♻️ Duplicate comments (2)

src/main.rs (2)
321-328: Prevent mixed runtime: unify gating for Winit + flatscreen input and the XR vs flatscreen branch.

As previously noted, this can lead to WinitPlugin and FlatscreenInputPlugin being added (due to env vars) while still taking the XR branch (because the branch below checks args.flatscreen). That risks conflicting runners/event loops and input pipelines.

Apply this minimal fix to make the XR/flatscreen selection consistent with how you add Winit/FlatscreenInput:
-    if !args.flatscreen {
+    if !flatscreenmode {
You can verify there are no remaining checks on args.flatscreen controlling this branch with:
#!/usr/bin/env bash
rg -n -C3 'if\s*!args\.flatscreen\b' src/main.rs
381-386: Limit scope to the PR goal and reduce power draw: only set unfocused_mode to Continuous.

Setting both focused and unfocused to Continuous exceeds the PR objective and forces max-rate updates even when focused. Prefer leaving focused mode at its default and only lifting the unfocused cap. Also, there’s no harm inserting this resource unconditionally.

Recommended change (unconditional, focused defaults preserved):
-    app.insert_resource(WinitSettings {
-        focused_mode: UpdateMode::Continuous,
-        unfocused_mode: UpdateMode::Continuous,
-    });
+    app.insert_resource(WinitSettings {
+        unfocused_mode: UpdateMode::Continuous,
+        ..default()
+    });
If you want to keep it gated on flatscreenmode:
-    app.insert_resource(WinitSettings {
-        focused_mode: UpdateMode::Continuous,
-        unfocused_mode: UpdateMode::Continuous,
-    });
+    app.insert_resource(WinitSettings {
+        unfocused_mode: UpdateMode::Continuous,
+        ..default()
+    });

🧹 Nitpick comments (2)

src/main.rs (2)
318-321: Confirm intent: auto-enabling flatscreen mode via DISPLAY/WAYLAND_DISPLAY broadens behavior.

This changes semantics beyond “remove unfocused framerate cap”: flatscreenmode will be true on any X11/Wayland session even if the CLI flag is false. If that’s intended, all good. If not, consider scoping this PR to the unfocused framerate only and keep flatscreen gating tied to the CLI flag.

Would you like me to provide a minimal patch that restricts auto-detection to a follow-up PR?

88-91: CLI help typos.

Small fixes to user-facing help text.
-    /// Creates a transparent window fot the flatscreen mode
+    /// Creates a transparent window for the flatscreen mode
@@
-    /// If monado insists on emulating , set this flag...we want the raw input
+    /// If monado insists on emulating hands, set this flag...we want the raw input
Also applies to: 95-97

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d1043f7b8c and fbf31e1df7.

📒 Files selected for processing (1)

src/main.rs (3 hunks)

🔇 Additional comments (1)

src/main.rs (1)

30-30: Winit import additions look correct.

WinitSettings and UpdateMode are the right types for controlling unfocused update behavior.

**Actionable comments posted: 0** <details> <summary>♻️ Duplicate comments (2)</summary><blockquote> <details> <summary>src/main.rs (2)</summary><blockquote> `321-328`: **Prevent mixed runtime: unify gating for Winit + flatscreen input and the XR vs flatscreen branch.** As previously noted, this can lead to WinitPlugin and FlatscreenInputPlugin being added (due to env vars) while still taking the XR branch (because the branch below checks args.flatscreen). That risks conflicting runners/event loops and input pipelines. Apply this minimal fix to make the XR/flatscreen selection consistent with how you add Winit/FlatscreenInput: ```diff - if !args.flatscreen { + if !flatscreenmode { ``` You can verify there are no remaining checks on args.flatscreen controlling this branch with: ```shell #!/usr/bin/env bash rg -n -C3 'if\s*!args\.flatscreen\b' src/main.rs ``` --- `381-386`: **Limit scope to the PR goal and reduce power draw: only set unfocused_mode to Continuous.** Setting both focused and unfocused to Continuous exceeds the PR objective and forces max-rate updates even when focused. Prefer leaving focused mode at its default and only lifting the unfocused cap. Also, there’s no harm inserting this resource unconditionally. Recommended change (unconditional, focused defaults preserved): ```diff - app.insert_resource(WinitSettings { - focused_mode: UpdateMode::Continuous, - unfocused_mode: UpdateMode::Continuous, - }); + app.insert_resource(WinitSettings { + unfocused_mode: UpdateMode::Continuous, + ..default() + }); ``` If you want to keep it gated on flatscreenmode: ```diff - app.insert_resource(WinitSettings { - focused_mode: UpdateMode::Continuous, - unfocused_mode: UpdateMode::Continuous, - }); + app.insert_resource(WinitSettings { + unfocused_mode: UpdateMode::Continuous, + ..default() + }); ``` </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>src/main.rs (2)</summary><blockquote> `318-321`: **Confirm intent: auto-enabling flatscreen mode via DISPLAY/WAYLAND_DISPLAY broadens behavior.** This changes semantics beyond “remove unfocused framerate cap”: flatscreenmode will be true on any X11/Wayland session even if the CLI flag is false. If that’s intended, all good. If not, consider scoping this PR to the unfocused framerate only and keep flatscreen gating tied to the CLI flag. Would you like me to provide a minimal patch that restricts auto-detection to a follow-up PR? --- `88-91`: **CLI help typos.** Small fixes to user-facing help text. ```diff - /// Creates a transparent window fot the flatscreen mode + /// Creates a transparent window for the flatscreen mode @@ - /// If monado insists on emulating , set this flag...we want the raw input + /// If monado insists on emulating hands, set this flag...we want the raw input ``` Also applies to: 95-97 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d1043f7b8cb23ae76592410ffd0b58f9e6060b6f and fbf31e1df7089c76aeb3eb4e6d08ddd243776641. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `src/main.rs` (3 hunks) </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>src/main.rs (1)</summary> `30-30`: **Winit import additions look correct.** WinitSettings and UpdateMode are the right types for controlling unfocused update behavior. </details> </blockquote></details> </details>

Schmarni-Dev commented

2025-08-18 21:28:23 -04:00

(Migrated from github.com)

i am unsure if this is the behavior we actually want at all? i think unless we have an active OpenXR session we want to limit framerate based on the parent compositor? we don't really want an uncapped framerate since clients take a while to update and send back changes, granted flatscreen doesn't have proper event timings at all right now but i don't see a reason to waste resources

nondescriptpointer commented

2025-08-19 05:03:53 -04:00

(Migrated from github.com)

i am unsure if this is the behavior we actually want at all? i think unless we have an active OpenXR session we want to limit framerate based on the parent compositor? we don't really want an uncapped framerate since clients take a while to update and send back changes, granted flatscreen doesn't have proper event timings at all right now but i don't see a reason to waste resources

For me it is not really an issue for keeping the default "game" behavior except when profiling as the framerate is reduced when you are interacting with the profiler because the Stardust window goes out of focus. So perhaps we only set it to continuous when profile_tokio or profile_app has been enabled?

> i am unsure if this is the behavior we actually want at all? i think unless we have an active OpenXR session we want to limit framerate based on the parent compositor? we don't really want an uncapped framerate since clients take a while to update and send back changes, granted flatscreen doesn't have proper event timings at all right now but i don't see a reason to waste resources For me it is not really an issue for keeping the default "game" behavior except when profiling as the framerate is reduced when you are interacting with the profiler because the Stardust window goes out of focus. So perhaps we only set it to continuous when profile_tokio or profile_app has been enabled?

Schmarni-Dev commented

2025-08-19 12:26:50 -04:00

(Migrated from github.com)

yeah seems reasonable, actually we can just always set it to max speed, we need to figure out how to do the timing ourself anyways since we need to do it instead of the xr wait ideally

This pull request has changes conflicting with the target branch.

src/main.rs

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin nondescriptpointer/feat/bevy-framerate-out-of-focus:nondescriptpointer/feat/bevy-framerate-out-of-focus

git checkout nondescriptpointer/feat/bevy-framerate-out-of-focus

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: StardustXR/server#52

				`@@ -378,6 +378,13 @@ fn bevy_loop(`
				`}),`

				`@@ -378,6 +378,13 @@ fn bevy_loop(`
				`}),`

Feature: don't cap bevy framerate when out of focus #52

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Chat

Support

CodeRabbit Commands (Invoked using PR/Issue comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Status, Documentation and Community

Checkout

CodeRabbit Configuration File (`.coderabbit.yaml`)