search

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

Fix math utils issue #532

Closed kalwalt closed 1 year ago

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

continue from PR #523

Can it be referenced to an Issue? If so what is the issue # ? Yes see PR #523 and issue #515 How can we test it?

Test the Aframe and Threejs examples included in the repository, in the console it should print Aframe version 1.3.0 and no errors about MathUtils... Summary

For more infos refer to the aforementioned issue and PR. 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 Tested all the aframe examples. Other information

Todo list:

kalwalt commented 1 year ago

https://github.com/AR-js-org/AR.js/pull/532/commits/f364a57e630312d9da70503e77240a61ee027e37 fixed issue https://github.com/AR-js-org/AR.js/issues/530

kalwalt commented 1 year ago

I upgraded three.js npm package to 150 and the three.js/examples/vendor/three.js/build/three.min.js and three.js/examples/vendor/three.js/build/three.js to the same version (the prreviuos was 132). I didn't find any issue upgrading three to this version, Aframe shouldn'tbe affected because use his internal version. I got some warnings about getInverse() deprecation, in the test-runner.html and arjs-session.html examples, those calls was inside three.js/src/markers-area/arjs-markersareacontrols.js and three.js/src/markers-area/arjs-markersarealearning.js. I will left some comments in the code for clarity.

nickw1 commented 1 year ago

Looks fine - happy to merge if all working as expected.

kalwalt commented 1 year ago

Looks fine - happy to merge if all working as expected.

Yes, i will do i have done only tests on desktop, i will do on Mobile soon, if all goes fine i will merge it.

kalwalt commented 1 year ago

updated GLTFLoader.js to r147 and partially fixed an issue (caused by changes in three.js r150). We can't use the newer because it require to import AR.js in a <script type="module">, but we don't have this feature yet. It's not a big deal to provide a jsm version of the lib but definetetly not the goal of this PR. Note that in the three.js/nft example you will receive this warning: three.min.js:7 THREE.PropertyBinding: Trying to update node for track: mesh_0.morphTargetInfluences but it wasn't found I think it's not crucial we can skip for now.

kalwalt commented 1 year ago

@nickw1 i will make a new release as i think this is an awaited fix.

nickw1 commented 1 year ago

@kalwalt I agree - sorry only just seen your update, I see you;ve released anyway which is great.