AR hit test #4892
AR hit test #4892
Conversation
|
Awesome thanks! Once we merge #4890 I look into this. |
|
I merged #4890. I think this needs a rebase. |
|
We should replace the existing ar-hit-test component of the model viewer example with the new one |
|
Note to self: test how well the reticle auto-sizing works with a really off centre model. As expected it broke, this has now been fixed |
|
Note to self: Add anchor support |
|
Note to self: See if the hit-test bbox calculations can be leveraged to reposition the directional light in the helper |
|
The reticle texture is now automatically generated from the model being moved it only renders the texture when the model is changed Here is a video of it in action: https://twitter.com/AdaRoseCannon/status/1422986340178747404 Needs:
|
|
Anchors are working well, I have found an issue with the Spinosaurus Demo where the rotation of the model is not preserved so the feet of the dinosaur don't match footprints going to identify next |
|
The issue was that the camera was rotated by 90 degrees which wasn't visible for my other model because it had 90 degrees rotational symmetry! The issue is fixed now: https://twitter.com/AdaRoseCannon/status/1425051509679661058 |
|
I just updated the model-viewer example it works well. |
|
Because of the nested objects to support the rotation it wasn't generating the model footprints well so I set it to use the image version instead. |
|
It might need rebase. I think I see commits from previous PR |
…era since it does not work
tweak events
update docs
AdaRoseCannon
force-pushed
the
ar-hit-test
branch
from
3b1e144
to
a2823cd
Compare
|
Rebased. |
|
|
||
| [hit-test]: https://immersive-web.github.io/hit-test/ | ||
|
|
||
| This component uses the WebXR hit-test API to position virtual objects in the real world. Remember to request the `hit-test` optional feature to allow it work. |
There was a problem hiding this comment.
Can we perhaps automate this? The ar-hit-test component adds hit-test to WebXR/optionalFeatures?
There was a problem hiding this comment.
Yes I was thinking that but no other components do it.
If we do this then I think also:
webxrshould addDOM-Overlayif requiredbackgroundshould add lighting-estimation- this should also add
anchorsas well ashit-test
is that okay to do in this PR?
There was a problem hiding this comment.
Yeah I think it should work. the init method of ar-hit-test handles the setup
There was a problem hiding this comment.
What is the best way to do this?
var features = new Set(this.el.systems.webxr.data.optionalFeatures);
features.add('hit-test');
features.add('anchors');
this.el.setAttribute('webxr', 'optionalFeatures', Array.from())
Doesn't work, I think Systems don't handle the 3 argument setAttribute like components do.
IS it okay to update this.el.systems.webxr.data.optionalFeatures directly?
There was a problem hiding this comment.
I don't love much the API we have know (array typed properties are rare) but you can do.
var optionalFeaturesArray = sceneEl.getAttribute('webxr').optionalFeatures;
optionalFeaturesArray.push('hit-test');
sceneEl.setAttribute('webxr', {optionalFeatures: optionalFeaturesArray});
There was a problem hiding this comment.
And that won't set the Overlay element and required features to null right?
There was a problem hiding this comment.
It was setting the OverlayElement to null so I did the following to ensure all the settings were preserved.
// Update WebXR to support light-estimation
var webxrData = this.el.getAttribute('webxr');
var optionalFeaturesArray = webxrData.optionalFeatures;
if (!optionalFeaturesArray.includes('light-estimation')) {
optionalFeaturesArray.push('light-estimation');
this.el.setAttribute('webxr', webxrData);
}
src/components/scene/ar-hit-test.js
Outdated
| if (hitTest.createAnchor) { | ||
| hitTest.createAnchor() | ||
| .then(function (anchor) { | ||
| anchorToObject3D.set(anchor, options); |
There was a problem hiding this comment.
Is better to use a map vs. a regular object here? object3D has an id that could be used as object key. I don't have much experience working with Maps so I could be talking non sense
There was a problem hiding this comment.
We're already using Maps and they make more semantic sense in this case since it is how they are supposed to be used.
src/components/scene/ar-hit-test.js
Outdated
| tempVec3.copy(offset); | ||
| tempQuaternion.copy(pose.transform.orientation); | ||
| tempVec3.applyQuaternion(tempQuaternion); | ||
| object3D.position.sub(tempVec3); |
There was a problem hiding this comment.
Is sub more clear than add in this context? I usually associate offset with addition but don't fully understand yet the full context here.
There was a problem hiding this comment.
Ah i'm not adding an additional offset.
At the beginning I calculate the offset the object's position versus the center of the calculated reticle.
I subtract this offset when the user places the object so that it lines up optically with the reticle
|
Looking good 👍 Left a few comments. Thanks so much |
|
Thanks for the feedback, i'll fix most of those and left comments for the ones I think should stay. I am on holiday for the next few days so i'll update the PR with fixes on Monday. |
|
Addressed all other comments, just need feedback on this one regarding the best way to update the webxr system. |
|
WebXR system now updates OptionalDependencies automatically which is a lot more convenient to users and something which has caught me out many times before. |
… get discarded automatically
|
@AdaRoseCannon Thanks for the patience. Code looks good. I don't have an Android / AR phone to test at the moment. Is this ready to go? |
|
I think so, I am currently writing a more advanced demo using it thoroughly which will hopefully unearth anything weird but I think it's fine |
|
Found some issues regarding dynamically changing |
|
It doesn't handle the case very well where the object you are trying to position is inside another object which has scale. I'll look into fixing it but it might be too flaky and might be easier to tell people to just not do that. |
|
Thanks so much! This is awesome. Love the work and patience with me 😄 |
Description:
Add support for hit-test for Augmented Reality headsets and handsets. I haven't been able to test on real headset hardware so please test there if you can, it should work but I've only tested in an emulator.
This is dependent on #4890 to be merged first here is a link to the comparison between the branches:
https://github.com/AdaRoseCannon/aframe/compare/background-lighting-estimation...AdaRoseCannon:ar-hit-test?expand=1
Changes proposed:
Additionally, if you want, I could add support for letting it do hit-testing in VR by doing raytracing against the scene to allow developers to build a uniform AR/VR experience but I am not sure how you'd feel about that.