AR-js-org / AR.js

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

Shaking fix for new-location-based components #483

Closed nwcourses closed 1 year ago

nwcourses 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?

This introduces the "reduce shaking" feature of the older location-based components into gps-new-camera, using exponential smoothing.

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

How can we test it?

Use the smoothing example from the new-location-based examples.

Summary

Shaking fix not available in new A-Frame location based components.

Does this PR introduce a breaking change?

No

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

Chrome latest on Pixel 3, Android 12

Other information

@kalwalt could you check this before I merge? I also have another couple of fixes I'm aiming to create PRs for, when all these are merged I will probably make a new release. Thanks.

nwcourses commented 1 year ago

Sorry! Opened this with my other github account. This is nickw1.

kalwalt commented 1 year ago

I think i will not have the time to test It during these days, maybe next week but i can not promise...

nickw1 commented 1 year ago

OK no worries!

nickw1 commented 1 year ago

@kalwalt if it helps, I now have a demo of the "shaking fix" online at

https://hikar.org/sf/aframe/examples/new-location-based/smoothing/

It requires a mobile device to work.

I also have a demo of #484 at

https://hikar.org/sf/aframe/examples/new-location-based/basic2/

(can test on desktop device)

If you don't have time to test these, are you happy if I merge them myself? I'd like if possible to make a new release (while I'm on holiday, back to work next week!) with these, #485 and one or two other things to increase the compatibility between new-location-based and the classic location-based components.

Thanks.

kalwalt commented 1 year ago

Since i have no time to test and if you think that this will help in some way the location based feature, you can merge It. 🙂

nickw1 commented 1 year ago

@kalwalt many thanks. Essentially this stops the objects "shaking" by smoothing the sensor readings. This fix was applied to the older location-based components a while ago, but with this PR it's now available in the new-location-based components.