Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jul 29, 2025

This PR implements single-view XRFrame support for monoscopic XR rendering and updates the desktop_opengl example with improved CLI arguments for different XR rendering modes.

Key Changes

1. Single-View XRFrame Support

  • Added mono XR rendering mode that renders only one view while staying in WebXR mode
  • Modified view count calculation to support 1 view (mono) or 2 views (stereo): (xrEnabled && !monoMode) ? 2 : 1
  • Updated rendering loops to handle dynamic view counts instead of hardcoded 2 views

2. Always-On XR Mode

  • Removed the -xr flag - XR is now always enabled
  • Eliminated non-XR rendering code path to ensure consistent WebXR behavior
  • All rendering now goes through the XR pipeline

3. Improved CLI Arguments

The desktop_opengl example now supports:

  • --mono (default): Monoscopic XR rendering with single view
  • --stereo (defaults to singlepass): Stereo rendering with optional mode specification
  • --stereo multipass: Stereo rendering using multiple passes
  • --stereo singlepass: Stereo rendering using single pass (explicit)
  • --samples: MSAA samples configuration
  • Robust error handling for invalid arguments

Example usage:

./jsar_desktop_opengl --mono                    # Default mono XR
./jsar_desktop_opengl --stereo                  # Stereo singlepass (default)
./jsar_desktop_opengl --stereo multipass        # Stereo multipass
./jsar_desktop_opengl --stereo singlepass       # Stereo singlepass (explicit)
./jsar_desktop_opengl --samples 8               # Custom MSAA samples

4. Enhanced Argument Parsing

  • Fixed --stereo parsing to properly handle edge cases like --stereo -w 100 or --stereo https://foobar
  • Added validation for width (-w) and height (-h) arguments to ensure they are positive integers
  • Improved error messages with specific feedback for validation failures
  • Made stereo mode parameter optional with singlepass as the sensible default

5. Technical Fixes

  • Projection Matrix: Fixed aspect ratio calculation for mono vs stereo modes
    • Mono: Uses full window aspect ratio
    • Stereo: Uses half aspect ratio for each eye
  • Cursor Interaction: Updated to handle mono (full window) vs stereo (split window) modes
  • Window Management: Width doubling only occurs in stereo mode
  • Platform Support: Added conditional OpenGL includes for Linux/macOS compatibility

6. Enhanced User Experience

  • Made --stereo mode parameter optional with singlepass as the sensible default
  • Updated help text to clearly indicate optional parameters and defaults
  • Added comprehensive examples showing all usage patterns
  • Improved argument validation with descriptive error messages

7. Preserved Compatibility

  • All existing stereo XR functionality remains unchanged
  • XR device and frame system continue to work with existing infrastructure
  • Mono mode simply uses only view index 0 from the existing 2-view arrays

XR Renderer Behavior

The XRStereoscopicRenderer class returns a position between the left and right eye when in mono mode, using the center viewer position (0.0f, 0.0f, 0.35f) instead of eye-specific offsets.

Testing

Comprehensive testing was performed including:

  • CLI argument parsing validation with edge cases and optional parameters
  • Projection matrix aspect ratio calculations
  • View count logic verification
  • Cursor interaction mapping
  • Error handling scenarios with invalid inputs
  • Default mode behavior verification

The implementation successfully enables single-view XRFrame rendering for performance-optimized and development scenarios while maintaining full WebXR compatibility and providing an improved developer experience with robust argument validation.

Fixes #158.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI and others added 2 commits July 29, 2025 13:30
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Copilot AI changed the title [WIP] Support single-view XRFrame and update desktop_opengl example for XR rendering modes Support single-view XRFrame and update desktop_opengl example for XR rendering modes Jul 29, 2025
Copilot AI requested a review from yorkie July 29, 2025 13:41
endfunction()

if (APPLE)
if (APPLE OR UNIX)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (APPLE OR UNIX)
if (APPLE)

Copy link
Member

@yorkie yorkie left a comment

Choose a reason for hiding this comment

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

Test

printf(" -n <count> Number of apps (default: 1)\n");
printf(" -s <samples> MSAA samples (default: 4)\n");
printf(" --mono Monoscopic XR rendering (default)\n");
printf(" --stereo <mode> Stereo XR rendering mode:\n");
Copy link
Member

Choose a reason for hiding this comment

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

Make the mode to be optional, and the default is singlepass

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 3432f68. The --stereo flag now defaults to singlepass mode when no mode is specified.

printf(" -w <width> Window width (default: 960)\n");
printf(" -h <height> Window height (default: 600)\n");
printf(" -n <count> Number of apps (default: 1)\n");
printf(" -s <samples> MSAA samples (default: 4)\n");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
printf(" -s <samples> MSAA samples (default: 4)\n");
printf(" --samples <samples> MSAA samples (default: 4)\n");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in commit 3432f68. Changed help text from "-s " to "--samples" to match the actual argument format used in the code.

{
public:
XRStereoscopicRenderer(WindowContext *windowCtx)
XRStereoscopicRenderer(WindowContext *windowCtx, bool monoMode = false)
Copy link
Member

Choose a reason for hiding this comment

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

This class will return a position between the left and right eye if it's mono mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. The XRStereoscopicRenderer class uses viewer_position_ (0.0f, 0.0f, 0.35f) as the base position, and in mono mode it returns this center position between the left and right eye positions instead of using the eye-specific offsets.

Copilot AI and others added 3 commits July 31, 2025 15:15
…help text

Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
… default

Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
…rendering modes with improved CLI

Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Copilot AI changed the title Support single-view XRFrame and update desktop_opengl example for XR rendering modes Support single-view XRFrame and update desktop_opengl example for XR rendering modes with improved CLI Jul 31, 2025
Copilot AI requested a review from yorkie July 31, 2025 15:20
break;
case 'n':
nApps = atoi(optarg);
if (i + 1 >= argc)
Copy link
Member

Choose a reason for hiding this comment

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

Consider the following cases:

  • --stereo -w 100
  • --stereo https://foobar

You can't just check if the index is out of the argument size, i think it's good to check if the next arg is "multipass" or "singlepass", if not then the default(singlepass) can be used and the index could be increased.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 03279a3. The --stereo argument parsing now properly validates if the next argument is "multipass" or "singlepass" before consuming it. If the next argument is not a valid stereo mode (like "-w" or "https://foobar"), it uses the default "singlepass" mode and doesn't increment the index, allowing the next argument to be processed correctly.

help();
return false;
}
width = atoi(argv[++i]);
Copy link
Member

Choose a reason for hiding this comment

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

Check the width is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 03279a3. Added validation to ensure width values are positive integers. The code now checks if atoi() returns a value <= 0 and provides an error message showing the invalid value if validation fails.

help();
return false;
}
height = atoi(argv[++i]);
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 03279a3. Added validation to ensure height values are positive integers. The code now checks if atoi() returns a value <= 0 and provides an error message showing the invalid value if validation fails.

…eight validation

Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 31, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from yorkie July 31, 2025 15:56
@yorkie yorkie marked this pull request as ready for review July 31, 2025 16:07
@yorkie yorkie merged commit 1ed14ac into main Jul 31, 2025
2 checks passed
@yorkie yorkie deleted the copilot/fix-158 branch July 31, 2025 16:09
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.

Support single-view XRFrame and update desktop_opengl example for XR rendering modes

2 participants