-
Notifications
You must be signed in to change notification settings - Fork 5
Visual Improvements to Maps3D demo #5
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the visual presentation and interaction of the Maps3D demo in both VR and AR modes.
- Updated XRSharp package version.
- Introduced an info panel with VR instructions and proper XR event handling in Maps3D.
- Adjusted camera options and UI layouts for room visibility improvements.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
XRSharpSamplesGallery.csproj | Updated XRSharp package reference version. |
Samples/Maps3D/Maps3D.xaml.cs | Added info panel initialization and XR event subscriptions. |
Samples/Maps3D/Maps3D.xaml | Introduced new info panel and adjusted Maps3D control placement. |
Menu/MenuItems.cs | Updated camera options to enhance demo visualization. |
Menu/For AR and VR/Menu3D.xaml | Modified layout parameters in the 3D menu for improved display. |
MainPage.xaml.cs | Revised AR/VR mode handling and added debug outputs. |
Comments suppressed due to low confidence (2)
src/XRSharpSamplesGallery/XRSharpSamplesGallery/Samples/Maps3D/Maps3D.xaml:20
- Duplicate control name 'map' is used; consider using unique names to avoid potential conflicts.
<xr:Maps3D Name="map" ViewportCenter="36.8065, 10.1815" xr:Canvas3D.Z="-2" ServerURL="https://userwareapiproxy.azurewebsites.net/tiles/3dtiles/root.json"/>
src/XRSharpSamplesGallery/XRSharpSamplesGallery/MainPage.xaml.cs:41
- [nitpick] Consider removing or refining debug log statements from production code if they are not required for ongoing diagnostics.
Console.WriteLine("AR " + Root3DInstance.IsInARMode);
src/XRSharpSamplesGallery/XRSharpSamplesGallery/Samples/Maps3D/Maps3D.xaml
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
private void OnUnloaded(object sender, EventArgs e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OnUnloaded event handler is defined but not wired to the control's Unloaded event; consider subscribing to it to ensure proper cleanup of event handlers.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@medbensalah could you check if it can be removed, or better to subscribe to Unloaded event
Co-authored-by: Copilot <[email protected]>
Changed globe initial size

added instructions for usage in VR
Room environment no longer appears in AR mode or if it is set to not be visible in the menu item