Feature: AR-Viewer

[ffrank913]

1. Why is this change necessary?

2. What does this change do, exactly?

3. Describe each step to reproduce the issue or behaviour.

4. Please link to the relevant issues (if any).

5. Checklist

• I have rebased my changes to remove merge conflicts
• I have written tests and verified that they fail without my change
• I have written or adjusted the documentation according to my changes
• This change has comments for package types, values, functions, and non-obvious lines of code
• I have read the contribution requirements and fulfil them.

[dfrancos-hub]

@CodiumAI-Agent /review

[dfrancos-hub]

@CodiumAI-Agent /describe

[CodiumAI-Agent]

PR Reviewer Guide 🔍

(Review updated until commit 1f25019)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 5 🔵🔵🔵🔵🔵

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The method for launching AR does not handle errors gracefully. It uses console logs and rejects promises without providing error messages to the caller. Code Duplication There is significant code duplication in the AR launch methods for iOS and Android. Consider refactoring to a more generic approach that reduces redundancy. Complex Method The 'Launch' method in the WebXR module is overly complex and handles multiple responsibilities. Refactoring into smaller, more focused methods would improve maintainability. Long Method The 'Update' method in the WebXRController class is very long and complex. Consider breaking it into smaller, more manageable functions.

[CodiumAI-Agent]

Title

Feature: AR-Viewer

---

User description

1. Why is this change necessary?

2. What does this change do, exactly?

3. Describe each step to reproduce the issue or behaviour.

4. Please link to the relevant issues (if any).

5. Checklist

• I have rebased my changes to remove merge conflicts
• I have written tests and verified that they fail without my change
• I have written or adjusted the documentation according to my changes
• This change has comments for package types, values, functions, and non-obvious lines of code
• I have read the contribution requirements and fulfil them.

---

PR Type

enhancement, tests

---

Description

• Introduced AR functionality with support for both iOS and Android platforms, utilizing AR Quick Look and WebXR.
• Implemented comprehensive WebXR integration, including session management, rendering, and interaction controls.
• Added new classes and methods for handling AR and WebXR interactions, such as raycasting, object manipulation, and touchscreen controls.
• Enhanced the scene management to support XR features, including light estimation and scene initialization/disposal.
• Updated existing tests and added new ones to cover the newly introduced AR and WebXR functionalities.
• Refactored imports across multiple files to use the type keyword for improved type safety.
• Made miscellaneous improvements and cleanups in various parts of the codebase.

---

Changes walkthrough 📝

	Relevant files
Enhancement	26 files AR.ts Add AR functionality with system checks for iOS and Android src/ar/AR.ts Introduced DIVEAR class for AR functionality. Added methods to launch AR on iOS and Android. Implemented system checks for AR support. +67/-0    ARQuickLook.ts Implement AR Quick Look with USDZ export                                  src/ar/arquicklook/ARQuickLook.ts Implemented DIVEARQuickLook class for AR Quick Look. Added methods to export scene models to USDZ format. Integrated ARQuickLook launch functionality. +42/-0    WebXR.ts Integrate WebXR with session and rendering management        src/ar/webxr/WebXR.ts Added DIVEWebXR class for WebXR integration. Implemented session management and rendering loop. Added support for camera reset and overlay. +176/-0  WebXRController.ts Add WebXR controller for interaction and object manipulation src/ar/webxr/controller/WebXRController.ts Created DIVEWebXRController for handling WebXR interactions. Implemented object placement, grabbing, and movement. Added touchscreen controls for interaction. +330/-0  WebXRCrosshair.ts Implement crosshair for WebXR interactions                              src/ar/webxr/crosshair/WebXRCrosshair.ts Introduced DIVEWebXRCrosshair class. Added default mesh setup and pose update methods. +37/-0    WebXROrigin.ts Manage WebXR origin with hit test source                                  src/ar/webxr/origin/WebXROrigin.ts Added DIVEWebXROrigin class for origin management. Implemented hit test source and matrix decomposition. +191/-0  Overlay.ts Add overlay UI for WebXR sessions                                                src/ar/webxr/overlay/Overlay.ts Created Overlay class for WebXR session UI. Added close button functionality. +50/-0    WebXRRaycaster.ts Implement raycaster for AR and scene intersections              src/ar/webxr/raycaster/WebXRRaycaster.ts Implemented DIVEWebXRRaycaster for AR and scene intersections. Added event dispatching for hit detection. +131/-0  WebXRRaycasterAR.ts Implement AR raycaster for hit testing                                      src/ar/webxr/raycaster/ar/WebXRRaycasterAR.ts Added DIVEWebXRRaycasterAR for AR hit testing. Implemented transient input hit test source. +102/-0  WebXRRaycasterTHREE.ts Add THREE.js raycaster for scene intersections                      src/ar/webxr/raycaster/three/WebXRRaycasterTHREE.ts Created DIVEWebXRRaycasterTHREE for scene raycasting. Implemented intersection detection with scene objects. +49/-0    WebXRTouchscreenControls.ts Implement touchscreen controls for WebXR interactions        src/ar/webxr/touchscreencontrols/WebXRTouchscreenControls.ts Introduced DIVEWebXRTouchscreenControls for touch interactions. Added event handling for touch, rotate, and pinch gestures. +356/-0  AxisCamera.ts Update AxisCamera imports and scene logic                                src/axiscamera/AxisCamera.ts Updated imports to use type keyword. Modified scene attachment logic. +6/-6      Communication.ts Add AR launch functionality to communication module            src/com/Communication.ts Added AR launch functionality in DIVECommunication. Updated imports with type keyword. +17/-0    index.ts Add LAUNCH_AR action type                                                                src/com/actions/index.ts Added LAUNCH_AR action type. +2/-0      EventExecutor.ts Implement event executor for handling events                          src/events/EventExecutor.ts Introduced DIVEEventExecutor class for event handling. Added subscription and dispatch methods. +35/-0    findSceneRecursive.ts Update findSceneRecursive imports                                                src/helper/findSceneRecursive/findSceneRecursive.ts Updated imports to use type keyword. +2/-2      Info.ts Enhance WebXR support checks with detailed reasons              src/info/Info.ts Added WebXRUnsupportedReason enum. Implemented methods to check WebXR support and reasons. +37/-1    MediaCreator.ts Update MediaCreator imports                                                            src/mediacreator/MediaCreator.ts Updated imports to use type keyword. +4/-4      Renderer.ts Enhance renderer with callback type and parameters              src/renderer/Renderer.ts Added DIVERenderCallback type for render callbacks. Updated render loop to include time and frame parameters. +21/-11  Scene.ts Add XR root and management methods to scene                            src/scene/Scene.ts Added DIVEXRRoot for XR scene management. Implemented methods for initializing and disposing XR. +35/-12  XRRoot.ts Implement XR root for scene management and light estimation src/scene/xrroot/XRRoot.ts Created DIVEXRRoot class for XR scene management. Added methods for light estimation initialization and disposal. +60/-0    XRLightRoot.ts Add XR light root with estimation handling                              src/scene/xrroot/xrlightroot/XRLightRoot.ts Added DIVEXRLightRoot class for XR light management. Implemented light estimation event handling. +80/-0    BaseTool.ts Update BaseTool imports                                                                    src/toolbox/BaseTool.ts Updated imports to use type keyword. +9/-3      Toolbox.ts Update Toolbox imports                                                                      src/toolbox/Toolbox.ts Updated imports to use type keyword. +1/-1      SelectTool.ts Update SelectTool imports                                                                src/toolbox/select/SelectTool.ts Updated imports to use type keyword. +1/-1      TransformTool.ts Update TransformTool imports and scene logic                          src/toolbox/transform/TransformTool.ts Updated imports to use type keyword. Modified scene attachment logic for gizmo. +6/-6
Tests	9 files AxisCamera.test.ts Update AxisCamera tests with new scene structure                  src/axiscamera/test/AxisCamera.test.ts Added mock for HelperRoot in scene. Updated tests to reflect scene changes. +4/-0      Communication.test.ts Update Communication test imports                                                src/com/test/Communication.test.ts Updated imports with type keyword. +1/-1      Info.test.ts Add tests for WebXR support and reasons                                    src/info/test/Info.test.ts Added tests for WebXR unsupported reasons. Updated existing tests for WebXR support. +45/-5    MediaCreator.test.ts Update MediaCreator tests with scene mock                                src/mediacreator/test/MediaCreator.test.ts Added mock for Root in DIVEScene. +7/-2      Renderer.test.ts Update Renderer test imports                                                          src/renderer/test/Renderer.test.ts Updated imports to use type keyword. +1/-1      Scene.test.ts Update Scene tests with XRRoot and Renderer mocks                src/scene/test/Scene.test.ts Added mocks for XRRoot and Renderer. Updated tests for new scene structure. +39/-5    Toolbox.test.ts Update Toolbox test imports                                                            src/toolbox/test/Toolbox.test.ts Updated imports to use type keyword. +1/-1      SelectTool.test.ts Update SelectTool test mocks                                                          src/toolbox/select/test/SelectTool.test.ts Updated mock for TransformControls. +1/-1      TransformTool.test.ts Update TransformTool test mocks and structure                        src/toolbox/transform/test/TransformTool.test.ts Updated mock for TransformControls. Adjusted tests for new scene structure. +2/-4
Miscellaneous	1 files dive.ts Clean up unused imports in dive.ts                                              src/dive.ts Removed unused import. +0/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[dfrancos-hub]

@CodiumAI-Agent /improve

[CodiumAI-Agent]

PR Code Suggestions ✨

Latest suggestions up to 1f25019

Category	Suggestion	Score
Possible bug	Add null checks to prevent runtime errors when accessing undefined properties Consider checking for null or undefined values after the GetWebXRUnsupportedReason call to prevent potential runtime errors when accessing properties of WebXRUnsupportedReason. src/ar/AR.ts [46-47] -WebXRUnsupportedReason[DIVEInfo.GetWebXRUnsupportedReason()!] +const reasonCode = DIVEInfo.GetWebXRUnsupportedReason(); +if (reasonCode === undefined) { + console.error('Failed to get WebXR unsupported reason'); + return Promise.reject(new Error('Failed to get WebXR unsupported reason')); +} +const reason = WebXRUnsupportedReason[reasonCode]; Suggestion importance[1-10]: 8 Why: This suggestion correctly identifies a potential runtime error due to accessing properties of an undefined object, which is a critical issue that could cause application crashes.	8
	Handle errors in XR session requests to improve robustness Add error handling for the navigator.xr.requestSession call to manage cases where the session request fails, which is not currently handled. src/ar/webxr/WebXR.ts [75-78] -const session = await navigator.xr.requestSession('immersive-ar', this._options); +let session; +try { + session = await navigator.xr.requestSession('immersive-ar', this._options); +} catch (error) { + console.error('Failed to start XR session:', error); + return Promise.reject(new Error('Failed to start XR session')); +} Suggestion importance[1-10]: 8 Why: Properly handling errors during session requests is crucial for the stability of the XR feature, especially to manage user expectations and system responses when XR is not available or fails to start.	8
	Add null checks to ensure HelperRoot is initialized before use Ensure that the HelperRoot object is properly initialized before attempting to add or remove the DIVEAxisCamera instance to avoid potential runtime errors. src/axiscamera/AxisCamera.ts [75-100] -this._scene.Root.HelperRoot.add(this); -... -this._scene.Root.HelperRoot.remove(this); +if (this._scene && this._scene.Root && this._scene.Root.HelperRoot) { + this._scene.Root.HelperRoot.add(this); + ... + this._scene.Root.HelperRoot.remove(this); +} Suggestion importance[1-10]: 7 Why: Adding null checks before using HelperRoot prevents potential runtime errors if HelperRoot is not initialized, enhancing the robustness of the code.	7
Enhancement	Improve error handling by providing descriptive error messages Ensure that the Promise.reject() calls in the Launch method of DIVEAR class are replaced with more descriptive error messages or error objects to provide clearer debugging information. src/ar/AR.ts [30] -return Promise.reject(); +return Promise.reject(new Error('ARQuickLook not supported')); Suggestion importance[1-10]: 7 Why: Adding descriptive error messages enhances debugging and error tracking, improving the robustness of error handling in the application.	7
	Replace the generic any type with a more specific type definition Use a more specific type than any for the LAUNCH_AR action in the Actions interface to improve type safety and maintainability. src/com/Communication.ts [56] -LAUNCH_AR: any; +LAUNCH_AR: { type: string; payload?: any }; Suggestion importance[1-10]: 6 Why: Using a more specific type instead of any for the LAUNCH_AR action improves type safety and code maintainability.	6
	Make the frame parameter optional in DIVERenderCallback to handle non-XR scenarios Ensure that the XRFrame parameter is optional in the DIVERenderCallback type to accommodate scenarios where an XR frame is not available. src/renderer/Renderer.ts [32-35] export type DIVERenderCallback = ( time: DOMHighResTimeStamp, - frame: XRFrame, + frame?: XRFrame, ) => void; Suggestion importance[1-10]: 6 Why: Making the frame parameter optional in the DIVERenderCallback type definition allows the callback to be used in contexts where an XR frame might not be available, increasing the flexibility of the renderer's callback system.	6
Best practice	Improve maintainability and encapsulation by using methods for DOM manipulations Replace direct DOM manipulation in the End method with a more structured approach to modifying styles, such as using a dedicated method or class manipulation. src/ar/webxr/WebXR.ts [87] -DIVEWebXR._overlay.Element.style.display = ''; +DIVEWebXR._overlay.show(); Suggestion importance[1-10]: 6 Why: Using methods for DOM manipulations instead of direct style changes enhances code maintainability and encapsulation, although it's a moderate improvement and not critical for functionality.	6
Typo	Fix the typo in the enum value for better clarity and correctness Correct the typo in the enum value UNKNWON_ERROR to UNKNOWN_ERROR to avoid potential confusion and bugs in error handling. src/info/Info.ts [1] -'UNKNWON_ERROR' = 0, +'UNKNOWN_ERROR' = 0, Suggestion importance[1-10]: 5 Why: Correcting the typo from UNKNWON_ERROR to UNKNOWN_ERROR improves the readability and correctness of the enum definition.	5

---

Previous suggestions

Suggestions up to commit 1f25019

Category	Suggestion	Score
Best practice	Add error handling for asynchronous operations to improve reliability Add error handling for the await DIVEWebXR.Launch(...) call to manage potential rejections and prevent unhandled promise rejections. src/ar/AR.ts [55] -await DIVEWebXR.Launch(this._renderer, this._scene, this._controller); +try { + await DIVEWebXR.Launch(this._renderer, this._scene, this._controller); +} catch (error) { + console.error('Failed to launch WebXR:', error); + return Promise.reject(error); +} Suggestion importance[1-10]: 9 Why: Adding error handling for the asynchronous DIVEWebXR.Launch method prevents unhandled promise rejections and improves the robustness and reliability of the application.	9
	Use console.error for error logging to enhance log clarity Use console.error instead of console.log for error messages to differentiate them from regular log messages. src/ar/AR.ts [29] -console.log('ARQuickLook not supported'); +console.error('ARQuickLook not supported'); Suggestion importance[1-10]: 6 Why: Using console.error instead of console.log for error messages helps in distinguishing them from regular logs, improving the readability and maintainability of the logs.	6
Enhancement	Improve error handling by providing detailed error messages Replace Promise.reject() with Promise.reject(new Error('appropriate error message')) to provide more context about the error. src/ar/AR.ts [30] -return Promise.reject(); +return Promise.reject(new Error('ARQuickLook not supported')); Suggestion importance[1-10]: 8 Why: Adding specific error messages enhances the debugging process and provides clearer context for failures, which is crucial for error handling in production environments.	8
Maintainability	Remove unnecessary Promise.resolve() to clean up the code Ensure that Promise.resolve() is replaced with a more meaningful operation or removed if not necessary, as it currently does not contribute to the function's logic. src/ar/AR.ts [37] -return Promise.resolve(); +// Assuming the function does not need to return anything specific +return; Suggestion importance[1-10]: 5 Why: Removing unnecessary Promise.resolve() calls simplifies the code and removes redundant operations, enhancing code clarity and performance.	5

Suggestions up to commit 1f25019

Category	Suggestion	Score
Error handling	Add error handling around setting the XR session to improve robustness Add error handling for the await renderer.xr.setSession(session); call in the Launch method to catch and handle any potential errors that might occur during the session setup. src/ar/webxr/WebXR.ts [85] -await renderer.xr.setSession(session); +try { + await renderer.xr.setSession(session); +} catch (error) { + console.error('Failed to set XR session:', error); + return Promise.reject(error); +} Suggestion importance[1-10]: 8 Why: Proper error handling for session setup is crucial for the stability and robustness of the XR feature.	8
	Improve error handling in the initialization of the XR controller Consider checking the return value of this._xrController.Init() for errors and handle them appropriately, instead of just catching and calling this.End(). src/ar/webxr/WebXR.ts [128-129] -await this._xrController.Init().catch(() => { +try { + await this._xrController.Init(); +} catch (error) { + console.error('Failed to initialize XR controller:', error); this.End(); -}); + return Promise.reject(error); +} Suggestion importance[1-10]: 8 Why: Enhancing error handling during the XR controller initialization can prevent runtime errors and improve the application's stability.	8
Enhancement	Improve error handling by providing detailed error messages with Promise.reject() Ensure that the Promise.reject() calls in the Launch method of DIVEAR class are provided with an error message or an error object to improve error handling and debugging. src/ar/AR.ts [30] -return Promise.reject(); +return Promise.reject(new Error('Appropriate error message here')); Suggestion importance[1-10]: 7 Why: Providing specific error messages will significantly improve debugging and error tracking capabilities.	7
Best practice	Enhance logging by using a more sophisticated logging framework Replace the console.log statements with a more robust logging framework that supports different levels of logging and better integrates with the system's logging infrastructure. src/ar/AR.ts [33] -console.log('Launching AR on iOS'); +logger.info('Launching AR on iOS'); Suggestion importance[1-10]: 4 Why: While enhancing logging is beneficial for maintenance and debugging, the impact is moderate as it does not directly affect functionality.	4

[dfrancos-hub]

Persistent review updated to latest commit 1f25019