Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing improved artoolkit5 js #359

Merged
merged 16 commits into from
Mar 3, 2022
Merged

Testing improved artoolkit5 js #359

merged 16 commits into from
Mar 3, 2022

Conversation

kalwalt
Copy link
Member

@kalwalt kalwalt commented Nov 19, 2021

⚠️ 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?

Try to solve a issue that occurs testing marker examples.

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

How can we test it?

Try only the three.js/examples/minimal_ES6.html and aframe/examples/marker-based/minimal_ES6.html.
Summary

Meshes onto the marker appears skewed or not aligned to the marker.

Does this PR introduce a breaking change?
No, it shouldn't but it's a WIP.

Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser
Oppo a72 android device.
Other information
See to do list/tasks to do before merging:

To do list:

@kalwalt kalwalt added bug Something isn't working enhancement New feature or request pattern marker about pattern markers pattern barcode about barcode markers ES6 module AR.js as a ES6 module labels Nov 19, 2021
@kalwalt kalwalt self-assigned this Nov 19, 2021
@kalwalt kalwalt marked this pull request as draft November 19, 2021 14:50
@kalwalt
Copy link
Member Author

kalwalt commented Dec 16, 2021

@nickw1 Haven't had the time to continue on this... but maybe if i have a bit of time i can make in another PR the webpack changes for the three.js location based as we discussed in another issue. So we could release another npm beta package.

@nickw1
Copy link
Collaborator

nickw1 commented Dec 17, 2021

@kalwalt sounds good.

@kalwalt
Copy link
Member Author

kalwalt commented Feb 9, 2022

I think i'm very close to solve this PR. But i need to make some tests on Mobile devices.

@kalwalt
Copy link
Member Author

kalwalt commented Feb 9, 2022

I made a simple test with an Android Device and finally this seems to fix the diplay issue. We can start to think to merge into dev and after in the master branch.
There are few things to fix, i will create a list of things to do before merging.

@kalwalt kalwalt marked this pull request as ready for review February 9, 2022 17:23
- fix labelingMode for White Markers
- build production libs
@kalwalt
Copy link
Member Author

kalwalt commented Feb 13, 2022

i should check if latest commits, about arjs-session, may affect location-based, after that i think i can merge this PR.

@kalwalt
Copy link
Member Author

kalwalt commented Feb 14, 2022

i should check if latest commits, about arjs-session, may affect location-based, after that i think i can merge this PR.

Aframe location-based shouldn't be touched by these changes, but anyway i will test them.

- upgrading other packages
@kalwalt
Copy link
Member Author

kalwalt commented Feb 15, 2022

I tested the aframe location-based examples and i didn't find any issue with the new changes but i found some cors issue @nickw1 with the hikar resources (?).
Not related but sometimes i also get this error, for example with the threejs/default.html example:

default.html:94 Uncaught TypeError: Cannot read properties of undefined (reading 'arController')
    at onResize (default.html:94:25)
    at default.html:89:4
onResize @ default.html:94
(anonymous) @ default.html:89
default.html:88 

but the example can track the Hiro marker, i think this happens because onResize happens before arController is ready.

@kalwalt
Copy link
Member Author

kalwalt commented Feb 15, 2022

The example is the peakfinder-2d and i receive a cors error about failing to download some resources. I f i get again this error i will open an issue.
I applied some changes that seems to solve the TypeError that happens onResize. See the commit after.

@kalwalt
Copy link
Member Author

kalwalt commented Feb 17, 2022

@nickw1 @nicolocarpignoli if you have nothing against it i will merge this PR soon.

@nickw1
Copy link
Collaborator

nickw1 commented Mar 2, 2022

@kalwalt Nothing in here that should affect any of my code AFAIK.

Which example was giving a CORS error?

@kalwalt
Copy link
Member Author

kalwalt commented Mar 2, 2022

@kalwalt Nothing in here that should affect any of my code AFAIK.

yes i think so too.

Which example was giving a CORS error?

If i remember well the peakfinder-2d example but i pulled in this branch your changes and i will test again. Note i'm not outside Europe. If i have this issue again i will report to you. 🙂

If my tests are ok i will merge this PR on dev and i will publish a new beta npm package.

@kalwalt
Copy link
Member Author

kalwalt commented Mar 2, 2022

@nick1 i can confirm that issue on the peakfinder-2d example, but maybe happens only testing on localhost? see the log:

Access to fetch at 'https://www.hikar.org/fm/ws/bsvr.php?bbox=-0.82,50.949999999999996,-0.62,51.15&outProj=4326&format=json&poi=natural' from origin 'http://localhost:5500' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.
index.js:15          GET https://www.hikar.org/fm/ws/bsvr.php?bbox=-0.82,50.949999999999996,-0.62,51.15&outProj=4326&format=json&poi=natural net::ERR_FAILED 404
init @ index.js:15
initComponent @ component.js:320
updateProperties @ component.js:302
module.exports.Component @ component.js:78
i @ component.js:662
value @ a-entity.js:332
value @ a-entity.js:495
value @ a-entity.js:463
(anonimo) @ a-entity.js:249
(anonimo) @ a-node.js:127
Promise.then (asinc)
value @ a-node.js:125
value @ a-entity.js:245
value @ a-entity.js:88
value @ a-register-element.js:160
m @ document-register-element.js:2
l @ document-register-element.js:2
(anonimo) @ document-register-element.js:2
r @ document-register-element.js:2
(anonimo) @ document-register-element.js:2
index.js:15                  Uncaught (in promise) TypeError: Failed to fetch
    at init (index.js:15:9)
    at initComponent (component.js:320:10)
    at i.updateProperties (component.js:302:12)
    at i.module.exports.Component (component.js:78:8)
    at new i (component.js:662:15)
    at HTMLElement.value (a-entity.js:332:19)
    at HTMLElement.value (a-entity.js:495:12)
    at HTMLElement.value (a-entity.js:463:14)
    at a-entity.js:249:14
    at a-node.js:127:21

@kalwalt
Copy link
Member Author

kalwalt commented Mar 2, 2022

tested the three.js location based example: Only the red cube is showed and is fixed in the middle of the device screen. I'm sure that example worked before, but i can't understand what is causing. Note i have enabled GPS position on my device (I double checked this). The current changes are only 837906d I will try a previous commit to see to be sure.

@kalwalt
Copy link
Member Author

kalwalt commented Mar 3, 2022

tested the three.js location based example: Only the red cube is showed and is fixed in the middle of the device screen. I'm sure that example worked before, but i can't understand what is causing. Note i have enabled GPS position on my device (I double checked this). The current changes are only 837906d I will try a previous commit to see to be sure.

If i force with:

let orientationControls;
get.m = 1

i can see all the four cubes in the N W E S positions, it seems that testing in localhost the code

const get = { m : 0 };
const parts = window.location.href.split('?');
if(parts.length==2) {
if(parts[1].endsWith('#')) {
parts[1] = parts[1].slice(0, -1);
}
const params = parts[1].split('&');
for(let i=0; i<params.length; i++) {
const param = params[i].split('=');
get[param[0]] = param[1];
}
}
on Mobile device don't work as expected to be.
Anyway this is not the argument of this PR. We can do these fix/improvements in another PR.

@kalwalt
Copy link
Member Author

kalwalt commented Mar 3, 2022

@nick1 i can confirm that issue on the peakfinder-2d example, but maybe happens only testing on localhost? see the log:

Access to fetch at 'https://www.hikar.org/fm/ws/bsvr.php?bbox=-0.82,50.949999999999996,-0.62,51.15&outProj=4326&format=json&poi=natural' from origin 'http://localhost:5500' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.
index.js:15          GET https://www.hikar.org/fm/ws/bsvr.php?bbox=-0.82,50.949999999999996,-0.62,51.15&outProj=4326&format=json&poi=natural net::ERR_FAILED 404
init @ index.js:15
initComponent @ component.js:320
updateProperties @ component.js:302
module.exports.Component @ component.js:78
i @ component.js:662
value @ a-entity.js:332
value @ a-entity.js:495
value @ a-entity.js:463
(anonimo) @ a-entity.js:249
(anonimo) @ a-node.js:127
Promise.then (asinc)
value @ a-node.js:125
value @ a-entity.js:245
value @ a-entity.js:88
value @ a-register-element.js:160
m @ document-register-element.js:2
l @ document-register-element.js:2
(anonimo) @ document-register-element.js:2
r @ document-register-element.js:2
(anonimo) @ document-register-element.js:2
index.js:15                  Uncaught (in promise) TypeError: Failed to fetch
    at init (index.js:15:9)
    at initComponent (component.js:320:10)
    at i.updateProperties (component.js:302:12)
    at i.module.exports.Component (component.js:78:8)
    at new i (component.js:662:15)
    at HTMLElement.value (a-entity.js:332:19)
    at HTMLElement.value (a-entity.js:495:12)
    at HTMLElement.value (a-entity.js:463:14)
    at a-entity.js:249:14
    at a-node.js:127:21

i have this issue also in dev branch and in master (for other examples that require hikar resosurces). As i said i don't tink this is related to this PR so i will merge it, but i will host some example on my https://github.com/kalwalt/kalwalt-interactivity-AR repository for testing.

@kalwalt kalwalt merged commit d6943f8 into dev Mar 3, 2022
@nickw1
Copy link
Collaborator

nickw1 commented Mar 5, 2022

@kalwalt Sorry! That URL is out of date, the server on hikar.org is now Node-based and has a different URL. Forgot I'd included it in the peakfinder! Will update.

Not sure about the problem with the three location based example, but it sounds like the orientation controls are not working. Is this on a mobile device? The box will be fixed in the middle of the screen on a desktop as it relies on the device sensors to orientate.

@kalwalt
Copy link
Member Author

kalwalt commented Mar 5, 2022

@kalwalt Sorry! That URL is out of date, the server on hikar.org is now Node-based and has a different URL. Forgot I'd included it in the peakfinder! Will update.

Ok, no problem. If you can give me the correct url i will fix it in the next days. I have some free time starting from Monday and so i can continue on this an if it is all ok, make the Big merge on master branch.

Not sure about the problem with the three location based example, but it sounds like the orientation controls are not working. Is this on a mobile device? The box will be fixed in the middle of the screen on a desktop as it relies on the device sensors to orientate.

Yes this happens while testing on a Mobile device, if i can i will try to fix also this.

@nickw1
Copy link
Collaborator

nickw1 commented Mar 5, 2022

@kalwalt I've just fixed it myself and committed to dev.

I will also need to fix the osm-ways example which has an incorrect URL.

On another matter I just tried building the latest dev commit (npx webpack) and had some errors reported relating to jsartoolkit and NFT, as follows:

 ERROR in ./three.js/src/threex/arjs-context-nft.js
    Module not found: Error: Can't resolve 'jsartoolkit' in '/home/nick/src/AR.js/three.js/src/threex'
     @ ./three.js/src/threex/arjs-context-nft.js 3:0-38 4:25-36
     @ ./three.js/src/index-arjs-nft.js
    
    ERROR in ./three.js/src/threex/arjs-markercontrols-nft.js
    Module not found: Error: Can't resolve 'jsartoolkit' in '/home/nick/src/AR.js/three.js/src/threex'
     @ ./three.js/src/threex/arjs-markercontrols-nft.js 4:0-38 5:22-33
     @ ./three.js/src/new-api/arjs-anchor.js
     @ ./three.js/src/index-arjs-nft.js
    
    ERROR in ./three.js/src/threex/arjs-context.js
    Module not found: Error: Can't resolve 'jsartoolkit' in '/home/nick/src/AR.js/three.js/src/threex'
     @ ./three.js/src/threex/arjs-context.js 3:0-38 4:25-36
     @ ./three.js/src/new-api/arjs-debugui.js
     @ ./three.js/src/index-arjs-nft.js
    
    ERROR in ./three.js/src/threex/arjs-markercontrols.js
    Module not found: Error: Can't resolve 'jsartoolkit' in '/home/nick/src/AR.js/three.js/src/threex'
     @ ./three.js/src/threex/arjs-markercontrols.js 3:0-38 4:22-33
     @ ./three.js/src/markers-area/arjs-markersareacontrols.js
     @ ./three.js/src/new-api/arjs-anchor.js
     @ ./three.js/src/index-arjs-nft.js
    
    ERROR in ./three.js/src/threex/arjs-markercontrols-nft.js
    Module not found: Error: Can't resolve 'worker-loader' in '/home/nick/src/AR.js'
     @ ./three.js/src/threex/arjs-markercontrols-nft.js 3:0-57 281:25-31
     @ ./three.js/src/new-api/arjs-anchor.js
     @ ./three.js/src/index-arjs-nft.js
    
    ERROR in chunk main [entry]
    ar-nft.js
    /home/nick/src/AR.js/three.js/src/index-arjs-nft.js c3951b5c9c9fcd0735e16a3e17649091
    Unexpected token (283:25)
    | 
    |     function handleNFT(descriptorsUrl, arController) {
    |         var worker = new !(function webpackMissingModule() { var e = new Error("Cannot find module './arjs-markercontrols-nft.worker.js'"); e.code = 'MODULE_NOT_FOUND'; throw e; }())();
    | 
    |         window.addEventListener('arjs-video-loaded', function (ev) {

Is this expected? e.g. I'm building the wrong way?
I don't need to use NFT but just giving you the heads up in case there is an issue.

@kalwalt
Copy link
Member Author

kalwalt commented Mar 5, 2022

Thank you for the fix!
In regards of the building error It could be a npm or node issue. I installed node 14 with nvm, that is nvm install 14. I suggest to you to delete the node_modules folder, run npm install and then npm run build. Tell me how It goes.🙂😉

@nickw1
Copy link
Collaborator

nickw1 commented Mar 6, 2022

OK thanks. I am using Node 14 but will try re-installing the modules.

I have just done another commit to fix the URL in the osm-ways example. In doing this I re-factored my server-side code so the URL in the peakfinder example has changed again.

@nickw1
Copy link
Collaborator

nickw1 commented Mar 6, 2022

Re-installing the modules seemed to fix the issue :-)

BTW I have now opened a PR #396 for the three.js location based, to allow event handling for GPS updates. It's pretty simple but @kalwalt feel free to check it.

As I've promised before I am also proposing to re-write the A-Frame location-based components to use the pure three code under the hood.

(What I think we really need are people with iDevices to make changes and test, as at the moment, this code is only known to work on the Android/Chrome combination)

@nickw1
Copy link
Collaborator

nickw1 commented Mar 6, 2022

... also have now completed a simple three.js location-based tutorial on the docs, see AR-js-org/AR.js-Docs#17

This assumes the changes made in #396.

@kalwalt
Copy link
Member Author

kalwalt commented Mar 6, 2022

Re-installing the modules seemed to fix the issue :-)

Ok, perfect. Probably was caused by the package-lock.json file. I added because depnadbot require it. I think it's useful for the project.

BTW I have now opened a PR #396 for the three.js location based, to allow event handling for GPS updates. It's pretty simple but @kalwalt feel free to check it.

Yes, thank you i will do it!

As I've promised before I am also proposing to re-write the A-Frame location-based components to use the pure three code under the hood.

I think we can do this after merging the ES6 feature (now in dev) in master. So i hope very soon...

(What I think we really need are people with iDevices to make changes and test, as at the moment, this code is only known to work on the Android/Chrome combination)

I agree with you, anyway i think i will buy a reconditioned/cheap IPhone for testing pourpose. I have the same problem with webarkit/ARnft and webarkit/jsartoolkitNFT where IDevice users asking to solve variuos issues, but i can't do so much without a device...

@kalwalt
Copy link
Member Author

kalwalt commented Mar 6, 2022

.. also have now completed a simple three.js location-based tutorial on the docs, see AR-js-org/AR.js-Docs#17

This assumes the changes made in #396.

That's great!

@kalwalt kalwalt deleted the testing-artoolkit5-js-imp branch December 22, 2024 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request ES6 module AR.js as a ES6 module pattern barcode about barcode markers pattern marker about pattern markers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants