Skip to content

Port webxr hand demo to new folder structure and clean up code#1366

Merged
ldcWV merged 25 commits intofacebookresearch:masterfrom
ldcWV:ldchen/webxr_hand_demo_port
Jul 15, 2021
Merged

Port webxr hand demo to new folder structure and clean up code#1366
ldcWV merged 25 commits intofacebookresearch:masterfrom
ldcWV:ldchen/webxr_hand_demo_port

Conversation

@ldcWV
Copy link
Copy Markdown
Contributor

@ldcWV ldcWV commented Jul 1, 2021

Motivation and Context

123186570-462f3000-d44d-11eb-9441-e62be1818744.mov

This is a continuation of Eric's branch here: https://github.com/eundersander/habitat-sim/commits/eundersander/webxr_hand_demo

This PR ports the hand demo code to a new folder structure, completely separate from the Habitat-sim source code. It provides an example that users can refer to in order to build their own webapps that use the Habitat-sim API.

It also contains some code cleanup.

Known issue: the immersive VR mode does not work on Oculus Quest 2 due to a bug outside the scope of this PR. However, it is still viewable using the WebXR emulator extension (see the README for more details).

The webapp is temporarily testable via here.

How Has This Been Tested

I ran the webapp on my computer by hosting it locally as well as hosting it on a server. I also was able to get it working in an Oculus headset while USB debugging is enabled, although it still doesn't work when USB debugging is off.

I was able to spawn, pick up, and move around objects.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jul 1, 2021
@ldcWV ldcWV marked this pull request as ready for review July 1, 2021 00:42
@ldcWV ldcWV requested a review from eundersander July 1, 2021 00:43
Copy link
Copy Markdown
Contributor

@eundersander eundersander left a comment

Choose a reason for hiding this comment

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

I left a few comments for now. I'll take a closer look soon!

Comment thread build_and_install_habitat_sim_js.sh
Comment thread examples/web_apps/webxr_hand_demo/README.md Outdated
Comment thread examples/web_apps/webxr_hand_demo/README.md Outdated
Comment thread examples/web_apps/webxr_hand_demo/standalone.html
Copy link
Copy Markdown
Contributor

@eundersander eundersander left a comment

Choose a reason for hiding this comment

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

I found a number of cases that look like improper copy-paste from earlier projects. How about make a pass through and make sure you've cleaned all that up, then ping me for another review.

Comment thread examples/web_apps/webxr_hand_demo/index.html Outdated
Comment thread examples/web_apps/webxr_hand_demo/js/habitat_main.js Outdated
Comment thread examples/web_apps/webxr_hand_demo/js/habitat_main.js Outdated
@ldcWV ldcWV requested a review from eundersander July 14, 2021 01:45
Copy link
Copy Markdown
Contributor

@eundersander eundersander left a comment

Choose a reason for hiding this comment

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

I see you refactored preload in index.js (thanks!). This is used by the older web demos: bindings.html and viewer.html. I don't think they are tested by CI. Can you test them locally before merging this?

Aside from that, LGTM!

@ldcWV ldcWV merged commit b9a7d6a into facebookresearch:master Jul 15, 2021
@ldcWV ldcWV deleted the ldchen/webxr_hand_demo_port branch July 15, 2021 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants