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

implementing NFT with threading support with emscripten #2

Open
wants to merge 22 commits into
base: fixing-nft
from

Conversation

@kalwalt
Copy link
Owner

commented Jul 22, 2019

In this PR i test the NFT with threading support. Basically i modified some functions in ARToolKitJS.cpp with the threading methods in trackingSub.c :

THREAD_HANDLE_T *trackingInitInit( KpmHandle *kpmHandle );
int trackingInitStart( THREAD_HANDLE_T *threadHandle, ARUint8 *imagePtrLuma );
int trackingInitGetResult( THREAD_HANDLE_T *threadHandle, float trans[3][4], int *page );
int trackingInitQuit( THREAD_HANDLE_T **threadHandle_p );

The project is not finished, they are some modifications to do... but even if it is not finished, i got 80-90 fps and 20-25 fps while detecting the NFT marker/image.
Note: not all the examples will works, i'm testing now only the nft_threejs.html example.
I'm developing for now only the minified version, i not added the Pthread support to the debug version due to this issue explained in this emscripten article:

Pthreads + memory growth (ALLOW_MEMORY_GROWTH) is especially tricky, see wasm design issue #1271. This currently causes JS accessing the wasm memory to be slow - but this will likely only be noticeable if the JS does heavy work (wasm runs at full speed, so moving work over can fix this). This also requires that your JS be aware that the HEAP* views may need to be updated - use the GROWABLE_HEAP_* helper functions which automatically handle that for you.

When it will be ready, will be merged in PR #1

to do list:

  • build the debug lib version with Pthreads support (if possible)
  • solve the asm.js issue: Invalid asm.js: Invalid member of stdlib
  • testing all the examples
  • removing not necessary code ( old code with webWorkers)
  • adding threading only to NFT
  • adding filtering to the trans Matrix
  • restoring the wasm LIb?
  • solving the issue with Firefox see #2 (comment)

Note:

If someone want to try and test the code you can go to https://kalwalt.github.io/kalwalt-interactivity-AR/

But Please read carefully this before:

You need to enable the SharedArrayBuffer option in your browser. This was disabled due to Spectre issue read this article !!!

@kalwalt kalwalt added the enhancement label Jul 22, 2019
@kalwalt kalwalt self-assigned this Jul 22, 2019
@kalwalt kalwalt referenced this pull request Jul 22, 2019
5 of 7 tasks complete
@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Jul 22, 2019

With the last commit i get this error:

three.min.js:359 THREE.Matrix3: .getInverse() can't invert matrix, determinant is 0

what does it means? the matrix is not filled in the right way? look:

int trackingInitGetResult( THREAD_HANDLE_T *threadHandle, float trans[3][4], int *page );

the trans is a 3 * 4 as you can see in the code:

float trans[3][4];
float trackingTrans[3][4];
kpmResultNum = trackingInitGetResult( arc->threadHandle, trackingTrans, &pageNo);
ARLOGi("kpmResultNum is: %d\n", kpmResultNum);
for( i = 0; i < kpmResultNum; i++ ) {
//if (kpmResult[i].pageNo == markerIndex && kpmResult[i].camPoseF == 0 ) {
if (pageNo == markerIndex ) {
// if( flag == -1 || err > kpmResult[i].error ) { // Take the first or best result.
if( flag == -1 ) { // Take the first or best result.
flag = i;
err = kpmResult[i].error;
ARLOGe("error in the tracking");
}
}
}
flag = kpmResultNum;
ARLOGi("flag is: %d\n", flag);
if (flag > -1) {
for (j = 0; j < 3; j++) {
for (k = 0; k < 4; k++) {
trans[j][k] = trackingTrans[j][k];
}
}

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Jul 22, 2019

from this stackoverflow article mrdoob comment:

Matrix3.getInverse(): can't invert matrix, determinant is 0 usually happens when either the scale.x, scale.y or scale.z are 0. Make sure you're not scaling the object to 0.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Jul 22, 2019

The NFT marker is detected, i see the scan in the console, but the sphere is not displayed. I will see this issue in the next days.
A real improvement in fps, at least on my Desktop machine!

@evaristoc

This comment has been minimized.

Copy link

commented Jul 23, 2019

If it is calculating a determinant, it is looking something like a scaling factor. If I am not wrong should be applicable only to square matrices. Is trackingTrans[3][4] the target matrix for the determinant operation? (Are matrices written as M[m][n] in C++?)

@evaristoc

This comment has been minimized.

Copy link

commented Jul 23, 2019

@kalwalt
This number : kpmResultNum : do you think is a determinant?

@evaristoc

This comment has been minimized.

Copy link

commented Jul 23, 2019

@kalwalt

I will try to check the code these days to see if I spot something. Apparently the trackingTrans matrix is the pose matrix.

I want to get and idea what the target of the threading is. At least one of the targeted functions appears to be trackingInitMain. The target (master?) data seems to be the descriptors (a struc) : trackingHandle.

If I understood correctly, I would say it makes sense to run a threading over that part.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Jul 23, 2019

If it is calculating a determinant, it is looking something like a scaling factor. If I am not wrong should be applicable only to square matrices. Is trackingTrans[3][4] the target matrix for the determinant operation? (Are matrices written as M[m][n] in C++?)

Unfortunately I'm not very skilled in math, biìut looking in the code it should be the pose matrix?

This number : kpmResultNum : do you think is a determinant?

I don't think, it was part of the previous code, i will delete it.

I want to get and idea what the target of the threading is. At least one of the targeted functions appears to be trackingInitMain. The target (master?) data seems to be the descriptors (a struc) : trackingHandle.

i think i have implemented as in mainLoop in the nftSimple example:

https://github.com/artoolkitx/artoolkit5/blob/a8baa4b95be381776af40d49b677bd5757f780c8/examples/nftSimple/nftSimple.c#L560-L598

but the data after this routine pass trough
the markersNFT struct from ARMarkerNFT.c (see in the nftSimple example few lines after L598) the trans matrix data pass trough the arFilterTransMat function; maybe it is a necessary step?

if (flag > -1) {

//ARLOGi("flag is: %d\n", arc->detectedPage);
if (pageNo >= 0 && pageNo == arc->detectedPage) {

This comment has been minimized.

Copy link
@kalwalt

kalwalt Jul 23, 2019

Author Owner

the problem is this we do not pass any data to the EM_ASM
if i change the variable in the conditional for example:
if (pageNo >= 0) we get data but with that Threejs error, and the sphere appear and disappears. MAybe we should use another condition or what else?

This comment has been minimized.

Copy link
@kalwalt

kalwalt Jul 23, 2019

Author Owner

don't remember why i put this:
if (pageNo >= 0 && pageNo == arc->detectedPage) {

This comment has been minimized.

Copy link
@kalwalt

kalwalt Jul 23, 2019

Author Owner

maybe it should be

if (arc->detectedPage >= -1) {

//// or better

if (arc->detectedPage > -1) {

This comment has been minimized.

Copy link
@evaristoc

evaristoc Jul 23, 2019

I would rather ask why appears and disappears. As you put it, it is a hack that might work for some but not all cases. I would prefer to understand why pageNo should be as condition.

However, this might not have sense: pageNo >= 0 && pageNo == arc->detectedPage. I can see some contradictory behaviour there.

If you leave it as arc->detectedPage > -1, does it mean that pageNo >= 0? For what I read from the code, detectedPage was a sort of flag for different states. What meant -1 status for detectedPage?

This comment has been minimized.

Copy link
@evaristoc

evaristoc Jul 23, 2019

What about:

if (pageNo >= 0 && arc->detectedPage SOMETHING -1) {

This comment has been minimized.

Copy link
@evaristoc

evaristoc Jul 23, 2019

This is the flagging I found for detectedPage:

int detectedPage  = -2;  // -2 Tracking not inited, -1 tracking inited OK, >= 0 tracking online on page.
@evaristoc

This comment has been minimized.

Copy link

commented Jul 23, 2019

@kalwalt
pose matrix
I guess but not sure the operations with the pose (transformation) matrix are being left at minimal: the actual shape is 4x4 but the last row default to [0,0,0,1]. It is possible that is a common practice to skip that row. Something to confirm? Also the vector that multiply it has 1 as last value ([x,y,z,1]T). Check https://en.wikipedia.org/wiki/Transformation_matrix for more details.

kpmResultNum
I will check this later?

markersNFT struct
Haven't been able to read the whole code yet. Those are apparently the chosen corners (the key points that will be listed for comparison). For what I saw in the code, the operation where markersNTF struc was used suggested a rectification of the position of the markers so they can be compared. I guess this is an homographic correction.

The code apparently never bother about detecting whether there was a variation of the values of the transformation matrix : it apparently just rectify at every frame, no matter the values. So if I understand correctly that part of the script always runs.

It is likely necessary: if I am not wrong the detector algorithm needs to rectify the test image (frame) in order to compare the found points in a similar orientation, position, etc than in the training image before declaring a good matching. Once the points have been detected, the homographic correction might be used to correct the volume projection over the target area in the test image. There is something similar happening with marker-based detection, @kalwalt, but the training marker is deployed differently and the algos might be easier.

However, this is NOT a step that might require threading. A for-loop will do. I think there are other parts that will be more suitable to, to mention:

  • the method devoted to the search of the corners
  • the one that create the descriptors
  • the one that compares (the matching one)
  • the part that choose good points and reduce the number of outliers (if there is one implemented)
@evaristoc

This comment has been minimized.

Copy link

commented Jul 23, 2019

@kalwalt

However now that I see it you might not have access to those methods though... hmmmm.... And they might be already optimized.

So probably the point comparison might be a good target, as well as the creation of the strucs when a painful for-loop is implemented.

Can you rather work on a simpler approach by for example making some for-loops more threaded instead? See that those for-loops are almost always there, for every frame.

Just a suggestion.

@evaristoc

This comment has been minimized.

Copy link

commented Jul 23, 2019

Contact you these days? Take care!

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Jul 23, 2019

@evaristoc thanks for the feedback! I have not time to answer to all the questions/topics but I want to make an observation: the kpmMatching and kpmGetResult functions (inside the threading function too) are responsable for the matching and retrieveing trans data and previously in the getNFTMarkerInfo used this two functions without threading, maybe i overcomplicated the code, But i have no time to test my idea now.... very late for me, i will try tomorrow if i will have time.

@kalwalt kalwalt force-pushed the nft-with-threads branch from 296fc2e to 218cb8f Jul 26, 2019
@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Jul 27, 2019

With the latest commits i got always the message Tracking lost. that means the ar2Tracking function receive some empty args, but it is not possible, for this reason i'm very confused....

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Jul 27, 2019

@evaristoc @nicolocarpignoli It seems that ar2Tracking was filled with wrong args:
it needs arc->videoFrame instead of arc->videoLuma , I tested with the Chrome Browser and the FPS are not so bad: 35fps while detecting the pinball image and more 50fps without.
Please test it and let me know what do you think!

But there are other refinement in the code to do, and other issues to solve...

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Jul 27, 2019

Unfortunately this not works with Firefox:

Current environment does not support SharedArrayBuffer, pthreads are not available! artoolkitNft.min.js:formatted:7495
ExitStatus: Program terminated with exit(-1)
@shawmakesmusic

This comment has been minimized.

Copy link

commented Jul 27, 2019

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Aug 2, 2019

@kalwalt yes, it could be an idea! Thanks for the effort so far

Also, I have sent you an email on github mail address but not sure if you got it. If not please let me know

I will check it!

kalwalt added 2 commits Aug 2, 2019
@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Aug 30, 2019

kalwalt added 2 commits Aug 30, 2019
@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 15, 2019

@evaristoc @nicolocarpignoli @ThorstenBux i added another example to test NFT with a gltf model. The problem is that the model is not showing onto the nft marker but only in the kanji pattern and on this last the model appear completely black and the animation not start. I tried to add a precision: 'mediump' in the THREE.WebGLRenderer without any effect.
And also for the NFT marker i tried the apply different scaling but the model still not appear. note that the NFT marker is detect because i receive the messages in the console.

@nicolocarpignoli

This comment has been minimized.

Copy link

commented Sep 16, 2019

@kalwalt have you tried with different model? More simple, like this => https://github.com/nicolocarpignoli/nicolocarpignoli.github.io/blob/master/ar-playground/models/CesiumMan.gltf

I saw on other threads on AR.js (also where you posted comments) that this may occur.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 16, 2019

@nicolocarpignoli i used the CesiumMan.gltf model. Yes i know the other issues. I have to try other options in the next days.

kalwalt added 2 commits Sep 18, 2019
@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 22, 2019

I have the same result with the Duck gltf model: i can see it with the kanji pattern but not with the NFT 'pinball' marker. The NFT marker is detected and regularly tracked, i have no idea why it can not be viewed.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 22, 2019

@nicolocarpignoli @evaristoc @ThorstenBux @shawmakesmusic now it works!! it was a problem of scaling the model and repositioning it !!
I will do the same for the other example.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 22, 2019

I should also test the binary version of the models, probably more efficient in loading time, and needs to test also with mobile devices...
I would know if there are some good news for the NFT marker creator @Carnaux

@nicolocarpignoli

This comment has been minimized.

Copy link

commented Sep 23, 2019

@kalwalt Great!

@Carnaux

This comment has been minimized.

Copy link

commented Sep 23, 2019

@kalwalt Unfortunately I didn't have time in August and September, but in October I will have a lot more time for projects. The last thing I was doing was trying to read the jpeg image because the current implementation of the marker creator is focused to local, not web, so I can't read some properties of the jpeg file using JavaScript. Maybe @ThorstenBux can help me to understand the data inside the iset and fset files.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 23, 2019

@Carnaux I imagined you were busy, but no problem! Unfortunately i have not so much time but I will try to move forward. regarding the problem for jpeg images I will begin to look at the problem, maybe I will open a specific issue in your repository.

@Carnaux

This comment has been minimized.

Copy link

commented Sep 23, 2019

@kalwalt I will open one and describe what is needed and what I was trying.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 23, 2019

@Carnaux just opened one right now!

@nicolocarpignoli

This comment has been minimized.

Copy link

commented Sep 24, 2019

@kalwalt are you able to host a version of it on Github.io? It would be a great way to test quickly with different mobile phones too. If you don't know how to to that just ask i can help you it's simple

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 24, 2019

@nicolocarpignoli I can do it. and this is a good idea. 😄

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 24, 2019

@nicolocarpignoli with Github.io you can only host the master or the gh-pages or docs folder, you can not host other branch apart them. I will host on my https://github.com/kalwalt/kalwalt-interactivity-AR. I will do tomorrow as now is too late and i am very tired.

@nicolocarpignoli

This comment has been minimized.

Copy link

commented Sep 25, 2019

yes no worries host it wherever you want :)

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Sep 25, 2019

@nicolocarpignoli and everybody here... i hosted one of the examples (CesiumMan) at my page https://kalwalt.github.io/kalwalt-interactivity-AR/nft_threads_cesium.html will host also the other examples when i will have time.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Oct 1, 2019

i added some other example to test to my https://kalwalt.github.io/kalwalt-interactivity-AR/ if someone want to try.

@kalwalt

This comment has been minimized.

Copy link
Owner Author

commented Oct 7, 2019

The asm.js issue: Invalid asm.js: Invalid member of stdlib seems disappeared, don't know why i would like to know what caused maybe the last changes?

Edit: but i tested on my https://kalwalt.github.io/kalwalt-interactivity-AR/
running things locally it not happens but instead i get that error on my webpage.

@kalwalt kalwalt referenced this pull request Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.