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.

Sign up for GitHub

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

Jump to bottom

Can't load multiple NFT markers #95

Open
ametis70 opened this issue Mar 9, 2020 · 58 comments
Open

Can't load multiple NFT markers #95

ametis70 opened this issue Mar 9, 2020 · 58 comments

Comments

@ametis70
Copy link

ametis70 commented Mar 9, 2020

I'm trying to load multiple NFT markers using artoolkit.addNFTMarker(markerURI, (id) => { /*...*/}) and its throwing an exception (with a number that is different every time it runs) as soon as the second image set starts to load. I tried recompiling jsartoolkit to enable exception catching, but I didn't help a lot.

image

Did anyone had success with this? I'm using a slightly modified version of one of the Misdake's examples (with jsartoolkit code running on a Worker)
@ThorstenBux
Copy link

How many would you want to load?

@ametis70
Copy link
Author

ametis70 commented Mar 9, 2020

Now I'm just trying to load two, but maybe 5 or 6 in the future. I found that if I create another ARController I can load one marker in each controller. If wonder if it is meant to be used that way? Or should I be able to load multiple markers in the same ARController object?

@ThorstenBux
Copy link

@nicolocarpignoli had played with it. It is certainly not meant to be by loading ARController multiple times. But I thought @nicolocarpignoli had success with 2.

@kalwalt
Copy link

kalwalt commented Mar 9, 2020 •
edited
Loading

Hi @ianmethyst thanks for reporting this. What does it meams Uncaught in the log error? Can you post the entire log in specific the Uncaught part? I think that could be a memory issue....

@nicolocarpignoli
Copy link

I was able to make it work using AR.js version. Honestly we have to check on that and see differences. I'm sure we can make it work with lean jsartoolkit5 (it should be easier, actually).

@ametis70
Copy link
Author

ametis70 commented Mar 9, 2020

I'll upload the complete log with an example project to replicate the error later. I'm not at home right now

@kalwalt
Copy link

kalwalt commented Mar 9, 2020

@ianmethyst Perfect!

@ametis70
Copy link
Author

Sorry for the delay. Here is the log (which is way too big compared to what is shown in the Chrome DevTools), and a zip with a git repo that contains the example I used when I found this problem. I removed all the files from the jsartoolkit repo that are not related (but kept the directory structure) and added a npm script so you can do npm install && npm run serve to launch a server with the example.

With git diff 417b1ff HEAD js/artoolkit.worker.js you can check the changes introduced that make this error happen. I just copied and pasted the block that initializes the NFT marker, but this time I used the same marker twice (I tried using different markers but got the same result).

localhost-1583890591748.log
nft-error.zip

@ametis70
Copy link
Author

Is there any way I can help with this? I believe there is missing some documentation and some guidance on how to contribute, because I gladly help but I find myself quite lost when I try to navigate the repo. For instance, I don't fully understand what is built with emscripten and what is plain javascript that ends in the final code

@ThorstenBux
Copy link

We highly appreciate any help. So yes you can certainly help. What is your experience what would you like to contribute?

@kalwalt
Copy link

kalwalt commented Mar 13, 2020

Sorry @ianmethyst @ThorstenBux i haven't had the time to look Into this. I will try today.

@kalwalt
Copy link

kalwalt commented Mar 13, 2020

@ianmethyst i will provide an example soon with a different approach, though i would investigate deeply on that error in a near future.

@kalwalt
Copy link

kalwalt commented Mar 13, 2020

I tested your code @ianmethyst rebuilding the libs with -s ALLOW_MEMORY_GROWTH=1 and i get again that error. We should try if this error also happens outside the WebWorker and understand well why this happens.

@ametis70
Copy link
Author

ametis70 commented Mar 14, 2020 •
edited
Loading

We highly appreciate any help. So yes you can certainly help. What is your experience what would you like to contribute?

I have some experience with JavaScript. I use it for frontend and backend (with Node.js) development. I've also been using three.js for a while.

I'd like to start helping fix this issue as it is something we are aware of, but I'd mostly like to contribute with the documentation, because I believe it's the most important part of any FOSS software. I'm gonna open another issue for that, though.

I tested your code @ianmethyst rebuilding the libs with -s ALLOW_MEMORY_GROWTH=1 and i get again that error. We should try if this error also happens outside the WebWorker and understand well why this happens.

Alright, thanks for trying that @kalwalt. I'll remove the Worker from the repo from the example I uploaded and see if it stills throws the exception

@kalwalt
Copy link

kalwalt commented Mar 14, 2020

@ianmethyst i also tested without the WebWorker and happens the same error. I think we should create a multi handler for Multi NFT, starting from the C++ code. We will go in detail in a dedicated issue.

I'd like to start helping fix this issue as it is something we are aware of, but I'd mostly like to contribute with the documentation, because I believe it's the most important part of any FOSS software. I'm gonna open other issue for that, though.

yes this is fundamental. I improved a bit the docs inside the api but should be enhanced. Actually you can create the docs running npm run create-doc read in the readme.md.

@Memleak1
Copy link

Memleak1 commented Apr 1, 2020 •
edited
Loading

any update on this? or any workaround? thank you for your time, im using artoolkit.min.js and I have same exception. Seem to be blocking in the second call to :

var id = Module._addNFTMarker(arId, prefix);
if (callback) callback(id);.

First marker being added succesfully.

@Memleak1
Copy link

@ThorstenBux ThorstenBux @ianmethyst nicolocarpignoli
any update? Thank you for your time.

@kalwalt
Copy link

kalwalt commented Apr 14, 2020

@Memleak1 No any updates from this point, i was focused to solve other part of the code. To handle correctly and efficently a series of NFT markers is required some changes in the C++ code, I think i know how to do but never had the time to implement. 🙂

@ametis70
Copy link
Author

ametis70 commented Apr 25, 2020 •
edited
Loading

Sorry, I was really busy lately and couldn't look into this this before. I just checked the code in the C++ ARToolKit5 repo and found a few things. I don't know C++, so correct me if I'm wrong, but the ARMarkerNFT class inherits from ARMarker that has a load() method and a ARPattern array (patterns). Then, in the .cpp file of ARMarkerNFT, for what I can see, load() is implemented so that the pattern passed as an argument is saved into the first index of the patterns array instead of being loaded into the next index. It's just a guess, but I believe that could be the cause of the exception being thrown.

I'll leave the links to the relevant files:
ARPattern.h
ARPattern.cpp
ARMarker.h
ARMarker.cpp
ARMarkerNFT.h
ARMarkerNFT.cpp

@kalwalt
Copy link

kalwalt commented Apr 25, 2020

@ianmethyst you looked in the right place, we should implement a sort of iterator as it exist in ARMarkerNFT, but without the threaded part of course.

@Memleak1
Copy link

Memleak1 commented Apr 27, 2020 •
edited
Loading

@kalwalt @ianmethyst, thank you for coming back to me, I tried to run the code to take a look and seem like I cannot build it using VS 2017. Did you manage to make it run? Seems I have issues with the KPM library

@ametis70
Copy link
Author

@Memleak1 You can use (and you should, to avoid unexpected errors due to OS differences) Docker to build jsartoolkit. It takes the C++ and turns it into JS using Emscripten. If you're trying to build C++ artoolkit5, then you can check this section of its readme.

@Memleak1
Copy link

@ianmethyst yes thats what we do to create an js version, but are you suggesting to make the change on the c++ side and test on the js side? I didnt have any luck building the C++ vs project so far 2017 & 2013.

@kalwalt
Copy link

kalwalt commented Apr 29, 2020

@Memleak1 please open another issue and post some logs, with detailed informations as much as you can. You can file an issue here or at my repository issue tracker Thank you 🙂

@hiukim
Copy link

hiukim commented May 2, 2020 •
edited
Loading

I can confirm this error. I have been digging through the codebase including the underlying artoolkit5 the past couple of days. It was a crazy journey :)

You cannot call addNFTMarker multiple times on a single AR controller. Once the marker dataset is set, calling this again will throw error. I believe there are a couple of logical flaws in the code. I wonder does anyone able to run multiple markers before.

I fixed a couple of things and finally managed to get something working. If you want, you can use my build here: https://github.com/hiukim/jsartoolkit5/tree/fixnft/build You still cannot call loadNFTMarker multiple times here, but I have extended a new method ar.loadNFTMarkers that accept multiple urls.

ar.loadNFTMarkers([markerURI_1, markerURI_2], function (markerIds) {
// console.log(markerIds[0], markerIds[1]);
});

If you want to build yourself, you need to use this artoolkit5 branch https://github.com/hiukim/artoolkit5/tree/nftfix There is a small fix here as well.

@kalwalt
Copy link

kalwalt commented May 2, 2020

@hiukim Nice! What about performances? How many NFT markers is possible to load with your mod?
and why do you modify the artoolkit5 source code? Too many questions but i will try your code.... 😄

@Memleak1
Copy link

Memleak1 commented May 3, 2020

@hiukim amazing! how did you manage to build your visual studio project? I'm having a hard time building it. Regarding the multimarker, seem like so people managed to get it working in A-Frame, I didnt look yet a their solution. I wonder what they made differently... kalwalt#40

@hiukim
Copy link

hiukim commented May 3, 2020

@kalwalt @Memleak1
Just to make sure everyone's on the same page and save some time for all the fellows who come along next.

The whole ar process consists of two parts: The detection part and the tracking part. The detection part is trying to find a first match in the loaded markers. After first match, the tracking part comes in and keep track of the match. There are actually two different algorithms behind.

The way it is (or supposed to be) working now is that: At the beginner, ONLY the detection part is running. It try to find a match in all the markers loaded (e.g. through the addNFTMarker. When there is a match, the detection part will stop, and the tracking part comes in. Now the process only keep track of the matched marker, and will not try to detect other markers. When the tracking is lost, the ball is passed back to the detection.

The ar controller as is now cannot track multiple markers. It is supposed be able to accept loading multiple markers for detection though. My fix is for the second part. It's important to point out that all I'm saying is for one AR controller. That's the behaviour of an AR controller. But you can spawn multiple AR controller to track multiple markers.

I hope I explained that clear enough. Now I can come back to the questions :)

Regarding performance, my changes won't hurt the performance because in the tracking part, it is still only tracking one marker. The performance of the detection part will depend on the number of loaded markers and their resolutions.

Regarding why I need to change the artoolkit5 source code. because I think I figured a bug there. If there is anyone here familiar with the source code, it would be great to if he/she can also take a look. I mean, I'm not totally confident too. If anyone interested in, it's good to have another eye to double check. The changes is here: hiukim/artoolkit5@ac59580

Regarding tracking multiple markers in another project of aframe. I have also dig into that too. If I understand the code correctly, each anchor spawns their own web workers, and each worker itself spawns an an AR controller. In that case, I'm not surprised if there is a working example that can "track" multiple markers using multiple AR controllers. I'm curious, however, is anyone able to load multiple markers in one AR controller though.

@Memleak1 Regarding building the project. I'm not sure what you mean by building visual studio project. I just follow the README and build with docker.

@nicolocarpignoli
Copy link

Hi everybody,
I just wanted to add that on AR.js (but should be the same here on jsartoolkitx) I was able to recognise multiple NFT Markers.
I have pushed now the branch. There are two images to scan: pinball image and alterra.

Branch and direct link to the example is this one:

https://github.com/AR-js-org/AR.js/blob/multiple-nft-markers/aframe/examples/image-tracking/nft/multi.html

To try it out, you should clone the repo, switch on the branch 'multiple-nft-markers' and run the html file above under an https server.

I have not checked this example in a while, so don't know much about performances. I'm sure it works though. Let me know

@kalwalt
Copy link

kalwalt commented May 3, 2020

I can confirm what you said about thé algorithme @hiukim. If your changes to artoolkit5 are necessary maybe @ThorstenBux may integrate them on the main repository. But it should be tested for all the platforms (Linux, Windows, Mac..), maybe Will be better to create a emscripten branch, and enclose that code between defines. So to prevent for other platform to be compiled. But mine Is only a supposition, i have only took a quick look to your code...
@nicolocarpignoli yes i remember that code...🙂

@kalwalt
Copy link

kalwalt commented May 3, 2020

@hiukim the problem with AR.js is that we need to create two instances of ArController: one in the Worker and the other 'outside' in Ar.js, so if you run another nft, there will be 3 ArController. For a performant device maybe is not a problem, but make a difference for another less performant...
I tested to run in a worker a nft Marker plus a Pattern Marker, but the result wasn't succesfull, probably is required a deeper investigation on this subject. If you are interested we can discuss on the AR.js issue tracker. Said this, we need for sure to improve jsartoolkit5, because is the base for many projects...

@ThorstenBux
Copy link

ThorstenBux commented May 3, 2020 via email

@kalwalt
Copy link

kalwalt commented May 3, 2020 •
edited
Loading

Walter, if you could let me know why you need two controllers that would be great. Cause it was never designed to have two running simultaneously. Is that the same that you did in webarstudio?

yes because in the worker you can not share an instanced object, so you are obliged to create two ARController, we haven't found another solution yet. this is valid for AR.js and webartsudio too.
But t would be great to found another solution...

@hiukim
Copy link

hiukim commented May 3, 2020

@ThorstenBux @kalwalt Yes, I figured artoolkit5 is somewhat unmaintained. Actually, I'm kind of confused with artoolkit5 and artoolkitx. Is the artoolkitx the next version of artoolkit5?

In that case, shouldn't we use artoolkitx as the backend? But that would make it jsartoolkitx. haha

@ThorstenBux
Copy link

ThorstenBux commented May 3, 2020 via email

@kalwalt
Copy link

kalwalt commented May 3, 2020 •
edited
Loading

@ThorstenBux @kalwalt Yes, I figured artoolkit5 is somewhat unmaintained. Actually, I'm kind of confused with artoolkit5 and artoolkitx. Is the artoolkitx the next version of artoolkit5?

In that case, shouldn't we use artoolkitx as the backend? But that would make it jsartoolkitx. haha

Yes ArtoolkitX Is the next version of Artoolkit5, but the issue is that with Artoolkit5 is easier to develop for emscripten the JavaScript version of the lib with NFT... That sound strange but is the reality. Anyway @ThorstenBux already did the JavaScript ArtoolkitX version.

@ThorstenBux
Copy link

ThorstenBux commented May 3, 2020 via email

@ametis70
Copy link
Author

ametis70 commented May 3, 2020

In AR.js implementation, each markers will spawn a new ar controller. and each controller try to detect and track their corresponding marker. At start, there are two detection process running (which already hurts). Another obvious issue is even one marker is being detected, the detection algorithm for another marker keeps running.

I can confirm this hurts performance badly. Having two ARControllers in a Xiaomi Redmi Note 7 (it's not a high end device, but most people have devices with worse hardware than this) made a huge drop from 60fps to 15fps even when a marker is found, because the other controller keeps searching for its marker.

This got a lot worse when I tried my code on an iPhone 7, because it drops to 3fps, stays there for a while and then crashes

I fixed a couple of things and finally managed to get something working. If you want, you can use my build here: https://github.com/hiukim/jsartoolkit5/tree/fixnft/build You still cannot call loadNFTMarker multiple times here, but I have extended a new method ar.loadNFTMarkers that accept multiple urls.

@hiukim I'll try it later! Thanks for your efforts

@ThorstenBux
Copy link

Hi @hiukim @Memleak1 @ianmethyst

@kalwalt and me decided to collect everything we worked on in the past year or two (which is a lot) in a new org called webarkit we will use this a base from now on to move forward.
Please have a look https://github.com/webarkit

We both would like to invite you to become members of the org. Just let me know if you are interested.

Our big vision is to unify opensource webAR and to be the best opensource SDK for webAR available.

@hiukim
Copy link

hiukim commented May 4, 2020

@ThorstenBux Nice. It's good to have a new base to move forward. I'm happy to contribute.

@kalwalt
Copy link

kalwalt commented May 4, 2020

@hiukim you are welcome!

@kalwalt
Copy link

kalwalt commented May 4, 2020

As @ThorstenBux said if anyone is interested just send a message to Thorsten or to me 😄

@Memleak1
Copy link

Memleak1 commented May 4, 2020

@ThorstenBux I'll be happy to join!

@albjeremias
Copy link

what does this mean? that webarkit will be based on artoolkit5 ? what's the difference between ar.js and webarkit? anyway i will keep on following this discussions, very interesting. thank you for sharing

@nicolocarpignoli
Copy link

nicolocarpignoli commented May 4, 2020 •
edited
Loading

@kalwalt @ThorstenBux great news. I have only one advice: it should be clear to everyone what is the relationships between AR.js, jsartoolkit5, jsartoolkitx, and any other project that is rising/has rised with the '*rtoolkit' name :)

Maybe on each repository a brief description may be useful and should be enough
Just my thought.

@kalwalt
Copy link

kalwalt commented May 5, 2020

@nicolocarpignoli Yes i will do an introductory presentation of the project and the idea behind of the organization. A quick clarification: we don't want to replace Ar.js at all, the core of WebArkit will be WebARStudio; improvements that we will do to jsartoolkit5 or artoolkit5 we will be happy to contribute to Ar.js .

This was referenced May 7, 2020
Closed
Open
@kalwalt kalwalt mentioned this issue Jul 14, 2020
Closed
@ifananomali
Copy link

ifananomali commented Aug 6, 2020 •
edited
Loading

I can confirm this error. I have been digging through the codebase including the underlying artoolkit5 the past couple of days. It was a crazy journey :)

You cannot call addNFTMarker multiple times on a single AR controller. Once the marker dataset is set, calling this again will throw error. I believe there are a couple of logical flaws in the code. I wonder does anyone able to run multiple markers before.

I fixed a couple of things and finally managed to get something working. If you want, you can use my build here: https://github.com/hiukim/jsartoolkit5/tree/fixnft/build You still cannot call loadNFTMarker multiple times here, but I have extended a new method ar.loadNFTMarkers that accept multiple urls.

ar.loadNFTMarkers([markerURI_1, markerURI_2], function (markerIds) {
// console.log(markerIds[0], markerIds[1]);
});

If you want to build yourself, you need to use this artoolkit5 branch https://github.com/hiukim/artoolkit5/tree/nftfix There is a small fix here as well.

Thanks @hiukim for your great efforts. I just tested it and it works. The current problem I'm facing is the amount of markers that can be loaded. I tested it with 10 markers but unfortunately it failed to load (Exceed maximum pages). But, if I decrease it to 9 markers, it works again. Can you tell me how to increase the amount of markers to be loaded? Please help me, because I need to load 10 markers. Thank you.

@hiukim
Copy link

hiukim commented Aug 6, 2020

@ifananomali I think there's probably a config settings in artoolkit that controls the limit. could be this one, but I'm not suree https://github.com/artoolkitx/artoolkit5/blob/master/include/ARWrapper/ARController.h#L76

You might have to change it and rebuilt it and try.

You can also try to reduce the dpi in your marker images when compile.

@ifananomali
Copy link

@ifananomali I think there's probably a config settings in artoolkit that controls the limit. could be this one, but I'm not suree https://github.com/artoolkitx/artoolkit5/blob/master/include/ARWrapper/ARController.h#L76

You might have to change it and rebuilt it and try.

You can also try to reduce the dpi in your marker images when compile.

I already tried to reduce all markers dpi but didn't work. Could you please tell me how to rebuild the project? I want to change it from your artoolkit (forked) and then rebuild it. Thank you.

@hiukim
Copy link

hiukim commented Aug 7, 2020 •
edited
Loading

The build instruction is on the README - section Build the project

Step2: You can pull my artoolkit5 repo (should be the nftfix branch) instead of the existing one.

Everything else stays the same

@ifananomali
Copy link

ifananomali commented Aug 7, 2020 •
edited
Loading

The build instruction is on the README - section Build the project

Step2: You can pull my artoolkit5 repo (should be the nftfix branch) instead of the existing one.

Everything else stays the same

Thank you @hiukim for your help. I've succeeded to increase the number of markers in jsartoolkit5 after rebuilding the ARToolKitJS.cpp with changes on PAGES_MAX.

https://github.com/hiukim/jsartoolkit5/blob/2c20bc069b6c4a960c9ec667e47f7c7f06ca451f/emscripten/ARToolKitJS.cpp#L29

@hiukim
Copy link

hiukim commented Aug 7, 2020

The build instruction is on the README - section Build the project
Step2: You can pull my artoolkit5 repo (should be the nftfix branch) instead of the existing one.
Everything else stays the same

Thank you @hiukim for your help. I've succeeded to increase the number of markers in jsartoolkit5 after rebuilding the ARToolKitJS.cpp with changes on PAGES_MAX.

https://github.com/hiukim/jsartoolkit5/blob/2c20bc069b6c4a960c9ec667e47f7c7f06ca451f/emscripten/ARToolKitJS.cpp#L29

nice!

@DLGL99GL
Copy link

DLGL99GL commented Nov 9, 2021

hello everybody，how's it going? has the function of multiple NFT markers been updated in project in jsartoolkit5？ Or is there any other example by three.js? thanks

@kalwalt
Copy link

kalwalt commented Nov 9, 2021

Hi @DLGL99GL we implemented multi NFT in our webarkit/jsartoolkit5 and if you want also our ARnft has multi NFT support.

@kalwalt kalwalt mentioned this issue Nov 9, 2021
Open
@DLGL99GL
Copy link

Hi @DLGL99GL we implemented multi NFT in our webarkit/jsartoolkit5 and if you want also our ARnft has multi NFT support.

yeah, that's exactly what I wanted. Thank you so much

@danielpetho danielpetho mentioned this issue Mar 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@hiukim @kalwalt @Memleak1 @ThorstenBux @ametis70 @nicolocarpignoli @albjeremias @ifananomali @DLGL99GL