Upgrade three.js and aframe

[kalwalt]

⚠️ 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 PR will upgrade Three.js and Aframe dependencies.
Can it be referenced to an Issue? If so what is the issue # ?
See issue #611 #504 #563 #535
How can we test it?

Test all the examples
Summary

Newer Three.js lib need to imported as a module so probably we need to create .module.js version for each build libs. Doing this probably we solve also issue #504
Does this PR introduce a breaking change?
I hope no but this is a draft PR.
Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser

Other information

[kalwalt]

I should have upgraded most of the three and aframe location-based examples, The only issue that i found is related to Aframe i got this warning:
renderer.js:39 THREE.WebGLRenderer: The property .useLegacyLights has been deprecated. Migrate your lighting according to the following guide: https://discourse.threejs.org/t/updates-to-lighting-in-three-js-r155/53733.
but i think there is not so much that we can do for now.

[kalwalt]

@nickw1 now all the examples should use aframe 1.6.0 and three 164, i applied all the changes you suggested in issue #611, i would merge it before 7 or 8 december, of course if you have nothing against. I haven't found any particular issue but let's do some more testing with the examples...

[nickw1]

@kalwalt great! Will test the location-based examples and let you know if there's any problem but other than that, all should be good!

[kalwalt]

@kalwalt great! Will test the location-based examples and let you know if there's any problem but other than that, all should be good!

Thank you @nickw1! I have the same idea, probably there shouldn't be any issue. Just a quick question; do you have an idea because exists this file: https://github.com/AR-js-org/AR.js/blob/master/aframe/dist/main.js ?
I have made some fixes for the multiMarkers option in artoolkit5-js, not all the required methods are implemented yet but maybe after this PR i may concentrate to improve it.

[nickw1]

@kalwalt I don't think that main.js is needed, because the bundles are placed in build. At a guess it was something which was committed accidentally.

[nickw1]

@kalwalt have just tested the location-based examples.

Some comments:

• some of the old location-based examples (in location-based) do not work, but the older versions of the location-based API are not really being maintained anymore and they don't work on master either. Can we just remove this directory? It will ease maintenance effort for one thing, only the new-location-based components have seen any updates for several years.

• the new-location-based examples mostly work with this PR the same as they do on master. avoid-shaking doesn't seem to work on either. However the osm-ways example is non-functional with this PR, while it works fine on master. Let me see if I can investigate this.

[nickw1]

Update on the above: I can get osm-ways to work if the a-camera is placed after the custom component, i.e.

<a-entity osm></a-entity> <!-- this does not work if placed AFTER the a-camera. -->
 <a-camera gps-new-camera='gpsMinDistance: 5; simulateLatitude: 51.049; simulateLongitude: -0.723' position='0 20 0' wasd-controls='acceleration: 1300'></a-camera>

Not sure if this is related to an internal change in A-Frame?

[kalwalt]

Update on the above: I can get osm-ways to work if the a-camera is placed after the custom component, i.e.

<a-entity osm></a-entity> <!-- this does not work if placed AFTER the a-camera. -->
 <a-camera gps-new-camera='gpsMinDistance: 5; simulateLatitude: 51.049; simulateLongitude: -0.723' position='0 20 0' wasd-controls='acceleration: 1300'></a-camera>

Not sure if this is related to an internal change in A-Frame?

Yes, It could be an internal change in A-frame. I will test your suggested fix when i can.
In regards of the other location examples, maybe it's better to focus with one version/type. Anyway let me think few days about, as I haven't so much time this week end.

Post edit: briefly think... If the old location examples as you said doesn't work, neither in master, better to remove them, please can you do this if you have time?

[kalwalt]

For the A-frame examples I haven't developed a module version as I did for the three.jsexamples. simply because for the vanilla J's examples wasn't required, but maybe it potentially could solve some issues loading as a module in React for example?

[nwcourses]

(Sorry this is @nickw1, I logged in on a different account)

Some of the location examples do work, but they aren't using the new-location-based components so I'm thinking they are maybe misleading. So are you still good for me to remove them?

I will do so in this branch if it's OK, as then we don't get master diverging.

Might be worth developing a module version for A-Frame too.

[nickw1]

The problem in osm-ways is odd because my Hikar webapp (using AR.js) works fine with aframe 1.6.0 and AR.js with this PR, the only issue is a PlaneBufferGeometry vs PlaneGeometry error in one of its dependencies (my own aframe-osm-3d lib which I can quickly fix). And in the Hikar webapp the camera is before the entity.

For now I think I'll rewrite the osm-ways example using the same template as some of the (working) examples, there is probably some difference I haven't spotted just yet.

With the reduce-shaking examples I think they need a real mobile device, come to think of it. Let me try on one.

[nickw1]

@kalwalt a few updates:

• reduce-shaking works fine on a real mobile device.
• osm-ways also works fine on a real mobile device.

Related to the osm-ways issue, I have discovered an issue in which simulateLatitude and simulateLongitude do not generate the gps-camera-update-position event correctly if hard-coded in the HTML, though they do work when they are updated dynamically. This seems to be the case in master too, so maybe something which slipped in during a recent update.

However I am unlikely to have much time to look into this soon, so maybe it's best raised as a separate issue. I'm more likely to be focusing on locar.js in any case.

For the "old" location-based examples these seem to work fine:

avoid-shaking
click-places
max-min-distance
peakfinder-2d

The others appear to be using old coding standards.

What I propose in that case is to change the new-location-based example directory to location-based, move the four examples above to a classic-components subdirectory, and delete the rest. Does this sound good? Let me know if I can go ahead and do this.

It looks like there is nothing in this PR which stops the examples working, though.

[kalwalt]

@kalwalt a few updates:

• reduce-shaking works fine on a real mobile device.
• osm-ways also works fine on a real mobile device.

Related to the osm-ways issue, I have discovered an issue in which simulateLatitude and simulateLongitude do not generate the gps-camera-update-position event correctly if hard-coded in the HTML, though they do work when they are updated dynamically. This seems to be the case in master too, so maybe something which slipped in during a recent update.

However I am unlikely to have much time to look into this soon, so maybe it's best raised as a separate issue. I'm more likely to be focusing on locar.js in any case.

For the "old" location-based examples these seem to work fine:

avoid-shaking click-places max-min-distance peakfinder-2d

The others appear to be using old coding standards.

What I propose in that case is to change the new-location-based example directory to location-based, move the four examples above to a classic-components subdirectory, and delete the rest. Does this sound good? Let me know if I can go ahead and do this.

It looks like there is nothing in this PR which stops the examples working, though.

Those examples are not necessary, so much the better. Proceed as you suggested. As you finish I will implement the module feature for aframe as discussed.
In regards the issue you catch it for the gps-camera-update-position event, better to open a specific issue for kepp track of it.

[kalwalt]

I will do so in this branch if it's OK, as then we don't get master diverging.

Hadn't the time to answer you, yes of course, work on this branch!

[nickw1]

OK - done.

[kalwalt]

OK - done.

thank you @nickw1 if i have time i will work on this sunday evening, we are close to finish this task.

[kalwalt]

I'm testing an example in this gist but it's not loading properly, i will investigate it.

[kalwalt]

I'm not sure that the module aframe versions dist libs will be helpful. Let me explain, the AFRAME arjs system is heavily based on THREE dependencies (i'm not referring to the internal aframe super-three.js code) from AR.js internal API, this cause the multiple three.js instances error for example and a bigger file size, it' shouldn't be necessary but this was designed from the beginning (more or less), so what we can do now? we should redesign the whole code base, not for sure in this PR...

[nickw1]

Maybe we should just stick with the "multiple three.js instances" warning now and try and re-design the code base as you say.

[kalwalt]

Maybe we should just stick with the "multiple three.js instances" warning now and try and re-design the code base as you say.

@nickw1 yeah maybe, i am a bit sad because i would release an updated AR.js, the redesign process will be a bit awkard, but maybe i'am wrong. I am convinced that for aframe-ar and all similar libraries, three.js is not necessary at all. For all calculations with matrices and maths, you can use a library like glm.js or gl-matrix.js. I think it would be a good idea to create a package AR.js-base or AR.js-core to include all the basic classes that may serve as root for AR.js and AR.js-threejs. Briefly i can start to convert this basilar classes:

1. Source
2. Profile
3. Context
4. ArBaseControls
5. MarkerControls

I think i can start with these, probably it's needed to add some more, but it's enough to start with.
I will redesign with the ES6 statndard in mind and maybe something more advanced standard. That is the Source class instead use this signature:

Source = function (parameters) {
// the constructor
}

// the init prototype method
Source.prototype.init = function (onReady, onError) {
// init code...
}

a more modern approach:

class Source {
  constructor() {
  }

  init() {
  }

  //private method setParameters
  #setParameters()
}

I will start soon to create a new repository, and to work with the basis. We will think how to proceed with this PR.

[nickw1]

@kalwalt your design sounds good. I think it makes sense to release AR.js with updated versions of A-Frame and three as soon as possible, then work on the redesign.

So presumably aframe-ar (NFT/marker based) just uses three.js for mathematical operations? I agree that it should not be necessary for this, as three.js is intended to be a rendering library rather than a general mathematical library.

[kalwalt]

@kalwalt your design sounds good. I think it makes sense to release AR.js with updated versions of A-Frame and three as soon as possible, then work on the redesign.

yes, the re-design process will take some time for sure. I think i will merge this PR in the next days.

So presumably aframe-ar (NFT/marker based) just uses three.js for mathematical operations?

The answer is not easy and fast but briefly i summarize what happens under the hood:

For the pattern/barcode and NFT markers we have the artoolkit5-js code that when detect one of them it dispatch an event with the matrixGL_RH (Right Hand OpenGL matrix). From artoolkit5-js we get also the camera Projection matrix. Inside AR.js we need them to pass the matrixGL_RH to the object3D (the root of the scene) and camera projection Matrix to the three.js PerspectiveCamera. But, for example, inside Ar.js we smooth the matricies with three.js, this operation can be done with other matrix libs as well or in Aframe starting from the this.el.object3D.

I just started to develop a Ar.js-core testing library, as soon as i provide a reliable code i will push on it.

I'm thinking to drastically cut and simplify the aframe-ar.js code, it will help me in the code development to focus on the strictly necessary.
I would also improve and add more dcoumentations and infos on this topic when i will push the code in the repository, because i haven't touched all the aspect of the problem, in my description above.

[nickw1]

@kalwalt thanks for the detail. I agree that it's a good idea to simplify the library: that was basically why I created locar.js, to focus on a small and tightly-focused location-based library which would be easier to work on and debug.

[kalwalt]

@nickw1 I will merge this PR tomorrow in the late afternoon, after making some final tests. i will wait few days for the release so if we find some bugs maybe we can fix them.