Skip to content
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

Augment reality to show tabletop scene #303

Merged
merged 24 commits into from
Jan 15, 2025

Conversation

TADraeseke
Copy link
Collaborator

Description

PR to add a new Kotlin sample Augment reality to show tablestop scene in Augmented reality category.

Links and Data

What To Review

  • Check AR core installs/works
  • Check that you can put an AR scene on a plane through your camera feed

How to Test

Run the sample with your device

@shubham7109 shubham7109 self-requested a review January 13, 2025 17:15
@shubham7109 shubham7109 added the New sample New Kotlin sample using ArcGIS Maps SDK label Jan 13, 2025
@shubham7109 shubham7109 changed the title Trev8939/augment reality to show tabletop scene Augment reality to show tabletop scene Jan 13, 2025
Copy link
Collaborator

@shubham7109 shubham7109 left a comment

Choose a reason for hiding this comment

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

Sample works well @TADraeseke 💯

Just a few suggestion changes and one error-handling comment.

TADraeseke and others added 7 commits January 13, 2025 20:05
…om/esri/arcgismaps/sample/augmentrealitytoshowtabletopscene/DownloadActivity.kt

Co-authored-by: Shubham Sharma <[email protected]>
…om/esri/arcgismaps/sample/augmentrealitytoshowtabletopscene/screens/AugmentRealityToShowTabletopSceneScreen.kt

Co-authored-by: Shubham Sharma <[email protected]>
…om/esri/arcgismaps/sample/augmentrealitytoshowtabletopscene/screens/AugmentRealityToShowTabletopSceneScreen.kt

Co-authored-by: Shubham Sharma <[email protected]>
Copy link
Collaborator

@shubham7109 shubham7109 left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Copy link
Collaborator

@hud10837 hud10837 left a comment

Choose a reason for hiding this comment

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

@TADraeseke Generally looks good. However, I notice that you don't use the onInitializationStatusChanged callback to display information to the user. Swift doesn't require this because they have access to a built-in coaching overlay from ARKit. However, we don't have such a thing in ARCore, so the microapp in the toolkit repo uses the onInitializationStatusChanged callback to tell the user to move their phone around until a plane is detected, then to tap on the plane to place a scene. Was this a deliberate decision not to include this workflow?

Note: TableTopSceneView also has another new piece of API requestCameraPermissionAutomatically. I don't know if this is worth mentioning in the sample README

@TADraeseke TADraeseke requested a review from hud10837 January 14, 2025 21:20
@TADraeseke
Copy link
Collaborator Author

Thanks @hud10837 -- added some instructions back in using the onInitializationStatusChanged callback. Ready for re-review!

Copy link
Collaborator

@hud10837 hud10837 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

TADraeseke and others added 2 commits January 15, 2025 20:10
…om/esri/arcgismaps/sample/augmentrealitytoshowtabletopscene/screens/AugmentRealityToShowTabletopSceneScreen.kt

Co-authored-by: hud10837 <[email protected]>
@TADraeseke
Copy link
Collaborator Author

Thanks @hud10837!

@TADraeseke TADraeseke merged commit d65622b into v.next Jan 15, 2025
1 check passed
@TADraeseke TADraeseke deleted the trev8939/augmentRealityToShowTabletopScene branch January 15, 2025 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New sample New Kotlin sample using ArcGIS Maps SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants