search

AR-js-org / AR.js

Image tracking, Location Based AR, Marker tracking. All on the Web.
MIT License
5.41k stars 924 forks source link

Add option to use initial location as world origin #508

Closed nickw1 closed 1 year ago

nickw1 commented 1 year ago

⚠️ All PRs have to be done versus 'dev' branch, so be aware of that, or we'll close your issue ⚠️

What kind of change does this PR introduce?

Adds the facility to use the initial GPS location as the WebGL/three.js/A-Frame world origin, as discussed in issue #499.

Can it be referenced to an Issue? If so what is the issue # ?

499

How can we test it?

A new example initial-location-as-origin has been added to the A-Frame new-location-based examples. Try this, either on mobile or on desktop (with the latter, you will have to manually enter a latitude and longitude if your location cannot be determined). Click on any of the four visible boxes. These will show each box's world coordinates and either the x or z should have a value less than 200. Furthermore the camera position should be reported as x=0, y=1.6, z=0.

Summary

This is done via a new initialPositionAsOrigin property of gps-new-camera. It's false by default, so as not to break existing applications, but recommended to be set to true for new code.

The rationale for this is discussed in issue #499, namely that very large absolute values for world coordinates can cause precision problems. By setting the world origin to the initial GPS position (as was the case in the older location based components, as it happens) this problem is lessened.

Does this PR introduce a breaking change?

No - original behaviour is maintained if the property is not set.

Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser

Tested with Firefox on desktop (Linux) and Android 12/Chrome latest (Pixel 3), the latter using a real GPS location.

Other information

My proposal is to make this the default at version 3.5, but not yet to avoid breaking changes. Also at 3.5 the floating origin feature of #499 will be introduced, I think this will require a more significant rewrite.

kalwalt commented 1 year ago

Hi @nickw1 i will look into it very soon! thank you for this PR!

nickw1 commented 1 year ago

Hi @nickw1 i will look into it very soon! thank you for this PR!

@kalwalt thanks, there's also #507 too, I'd like to add these two to the next bugfix release if possible.

kalwalt commented 1 year ago

@nickw1 I hope in the next days to find the time to test this and #507...!

kalwalt commented 1 year ago

@nickw1 i tested your code with my Android device, and i can confirm that the boxes appear when you click on the "go" panel. I noticed a small issues, when you click on the colored box and the alert window is shown, i click the ok button to close it but it's closed and reappear again, and now if you push the ok button the window finally is closed.

kalwalt commented 1 year ago

as said in #507 see this comment i got this error:

aframe-ar.js:1 Uncaught ReferenceError: msg is not defined
    at i._displayError (aframe-ar.js:1:1638714)
    at Object.gpserror (aframe-ar.js:1:1636796)
    at _watchPositionId._watchPositionId.navigator.geolocation.watchPosition.enableHighAccuracy (aframe-ar.js:1:783100)
nickw1 commented 1 year ago

Hello @kalwalt thanks for testing. The issue you describe is something to do with the click event, though I am not sure what without investigating further. However, it is unrelated to this PR, it is only related to the demo. As long as you get world coordinates with low x and z values, the PR should be working.

kalwalt commented 1 year ago

Hello @kalwalt thanks for testing. The issue you describe is something to do with the click event, though I am not sure what without investigating further. However, it is unrelated to this PR, it is only related to the demo. As long as you get world coordinates with low x and z values, the PR should be working.

Agree with you, I will open an issue for that error i got, so we can keep track of it.

kalwalt commented 1 year ago

so if receive this message clicking on the red box:

Box clicked! Position is: 0 0 -117.0774072....(values omitted)

does it is correct in your opinion?

nickw1 commented 1 year ago

Yes, looks fine.

nickw1 commented 1 year ago

OK done. @kalwalt Once I've merged this shall I make a new release?

kalwalt commented 1 year ago

OK done. @kalwalt Once I've merged this shall I make a new release?

yes, can you update also artoolkit5-js package to the new version before the new release?

nickw1 commented 1 year ago

So 0.3.0 ?

kalwalt commented 1 year ago

So 0.3.0 ?

yes exactly this is the version where i added the type definitions, it shouldn't create any issue. i have already tested in my local branch. thank you! 🙂

nickw1 commented 1 year ago

So 0.3.0 ?

yes exactly this is the version where i added the type definitions, it shouldn't create any issue. i have already tested in my local branch. thank you! slightly_smiling_face

OK great, will try and make a release tonight.