artoolkitx / jsartoolkit5

Javascript ARToolKit v5.x
https://artoolkitx.github.io/jsartoolkit5/examples/
Other
292 stars 69 forks source link

Enhancement in the tools/makem.js script and other fixes #96

Open kalwalt opened 4 years ago

kalwalt commented 4 years ago

This PR add these features:

description of the new script feature: to build the whole libs run the build command with:

npm run build-local

with Docker ( !! recommended !! ):

docker exec emscripten npm run build-local

after this libar.bc is build and if you have done some changes in the code, but not in artoolkit5, you don't need to build libar.bc again so run:

npm run build-local-no-libar

with Docker ( !! recommended !! ):

docker exec emscripten npm run build-local-no-libar

In this way the build process will take about in half the time.

Related PR https://github.com/kalwalt/jsartoolkit5/pull/45 issue https://github.com/kalwalt/jsartoolkit5/issues/44

kalwalt commented 4 years ago

@nicolocarpignoli I would add also the ALLOW_MEMORY_GROWTH=1 as you said that is needed.

kalwalt commented 4 years ago

I simply tested adding FLAGS += ' -s ALLOW_MEMORY_GROWTH=1'; and rebuilding the libs with Docker, Did you make other modifies @nicolocarpignoli?

I think we should also improve the nft examples @ThorstenBux, should we present only the WASM version as you did in your https://github.com/ThorstenBux/ar-magazine/blob/master/src/threejs_wasm_worker.js ? We could only leave two examples with wasm. The example of the sphere is the one with the model gltf. i would like to create a script that can be reused multiple times, example:

if (navigator.mediaDevices && navigator.mediaDevices.getUserMedia) {
        var hint = {
          audio: false,
          video: true
        };

        if (window.innerWidth < 800) {
          hint = {
            audio: false,
            video: {
                width: { ideal: 480 },
                height: { ideal: 640 },
                facingMode:
                    { exact:
                        "environment"
                    }
                },
          };
        }

        navigator.mediaDevices.getUserMedia(hint).then(function(stream) {
          video.srcObject = stream;
          video.addEventListener("loadedmetadata", function() {
            video.play();

            start(
              container,
              markers["pinball"],
              video,
              video.videoWidth,
              video.videoHeight,
              canvas,
              function() {
                statsMain.update();
              },
              function() {
                statsWorker.update();
              },
              null
            );
          });
        });
      }

became an external script and ypu need only:

// inside threejs_main_worker.html before end of </script></body>

<script src="nftLoader.js"></script>

nftLoader(markers, 640, 480, canvas, video, statsMain, statsWorker);
kalwalt commented 4 years ago

Testing code in my other repository here https://github.com/kalwalt/kalwalt-interactivity-AR/blob/master/nft/nftLoader.js used in this html example, basically you can simply:

<body>
// other html code here
    <script>
      /**
       * APP / ELEMENTS
       */
      var container = document.getElementById("app");
      var video = document.getElementById("video");
      var canvas = document.getElementById("canvas");

      nftLoader(container, video, 480, 640, canvas, "pinball", true);

    </script>
  </body>
kalwalt commented 4 years ago

@ThorstenBux in some machine you can experience "array buffer out of memory" without -s ALLOW_MEMORY_GROWTH=1 what do you think? Does It is better to add in your opinion?

ThorstenBux commented 4 years ago

I would add it yes. I recall on my iPhone 6s that it breaks without -s ALLOW_MEMORY_GROTH=1

kalwalt commented 4 years ago

I would add it yes. I recall on my iPhone 6s that it breaks without -s ALLOW_MEMORY_GROTH=1

i never experienced this, probably because i have tested only on Android, @nicolocarpignoli instead experienced this and we added in the version embedded for AR.js. I will add for sure.

kalwalt commented 4 years ago

Have you experienced this issue while trying one of my examples here as you said in this https://github.com/kalwalt/jsartoolkit5/pull/42#issuecomment-605893572 ?

ThorstenBux commented 4 years ago

No I haven't however I'm not sure with which device I tested that. I recall that inside artoolkitX.js I have allowMemoryGrowth enabled and never had any issues with it which is why I would vote for having it :) .

kalwalt commented 4 years ago

@ThorstenBux i added -s ALLOW_MEMORY_GROTH=1 and i improved the NFT examples following your code in ar-magazine:

ThorstenBux commented 4 years ago

Thank you. I'll check why the build isn't working and merge then.

kalwalt commented 4 years ago

Thank you. I'll check why the build isn't working and merge then.

yes, bitrise is failing for some time, I forgot to tell you.

kalwalt commented 4 years ago

@ThorstenBux bitrise failing because the github-release don't find some of the input variables:

Issue with input: failed to parse config:
- APIToken: required variable is not present
- Username: required variable is not present
- Tag: required variable is not present
ThorstenBux commented 4 years ago

Some bug there. A PR should never push a release. I'm on it