Removing old jsartoolkit5 version from AR.js

[kalwalt]

Do you want to request a feature or report a bug?
This a feature request and also an enhancement of AR.js

What is the current behavior?
Actually we have two types of AR.js libs as described in the AR-js-org docs

1. Image Tracking and Location Based >> (ar-nft.js and aframe-ar-nft.js)
2. the old style lib for Pattern Marker; Barcode Marker... >> (ar.js and aframe-ar.js)

The first one use the newest jsartoolkit5 lib, the second use the old jsartoolkit5 lib with .min and .api bundled together.

Why this? artoolkit.api.js is required otherwise you will get this issue on Mobile device (portrait mode) see #60 (comment) and also pay attention to all the discussion in the PR #60
We need to remove the old lib because we can not add new features to jsartoolkit5.

If the current behavior is a bug, please provide the steps to reproduce.
remove artoolkit.api.js from the compilation (Makefile) ,

- cat vendor/jsartoolkit5/build/artoolkit.min.js \
- vendor/jsartoolkit5/js/artoolkit.api.js >> build/ar.js

+ cat vendor/jsartoolkit5/build/artoolkit.min.js >> build/ar.js

run the script, test the basic example or any other example on portrait mode (Mobile)

Please mention other relevant information such as the browser version, Operating System and Device Name
The bug i mentioned occurs only in portrait mode (Mobile)
What is the expected behavior?
Eliminating artoolkit.api.js should behave in the same way.
If this is a feature request, what is motivation or use case for changing the behavior?
Upgrading AR.js to the new jsartoolkit5 ( not only for nft).

[kalwalt]

I solved the problem, but i think we have to find a better solution. I will explain now all the aspect of the issue. As mentioned above, the problem arises when you are in 'portrait' mode (on Mobile) without the artooolkit.api.js.

Why this?

In the old and also in the new (with NFT) artoolkit.api.js at the beginning, when a new ARController is instantiated there is:

AR.js/three.js/vendor/jsartoolkit5/js/artoolkit.api.js

Line 31 in da5ea83

this.orientation = 'landscape';

this is the default setting. When you run a jsartoolkit5 code and you are in portrait mode this.orientation becomes 'portrait' because in ARController.getUserMediaARController

AR.js/three.js/vendor/jsartoolkit5/js/artoolkit.api.js

Lines 1292 to 1296 in da5ea83

	if (video.videoWidth < video.videoHeight) {
	arController.orientation = 'portrait';
	arController.videoWidth = video.videoHeight;
	arController.videoHeight = video.videoWidth;
	} else {

if the video width is less than height this.orientation = 'portrait', but because getUserMediaARController is never used inside AR.js ( treex-arcontroller, threex-artoolkitsource and so on...) this.orientation is always landscape !! see my comment here #60 (comment)

So what is the solution?

This issue was already in the old Ar.js lib see this changes in this PR in artoolkit.api.js about the portait mode inside the _copyImageToHeap
I tested adding that changes to the new artoolkit.api.js in this way:

  if( (image.nodeName === 'IMG' && image.width > image.height ) ||
	      		(image.nodeName === 'VIDEO' && image.videoWidth > image.videoHeight) ){
			        // if landscape
	                        this.ctx.drawImage(image, 0, 0, this.canvas.width, this.canvas.height); // draw video
		        }else{
			        // if portrait
			        this.ctx.clearRect(0, 0, this.canvas.width, this.canvas.height);
		        	var scale = this.canvas.height / this.canvas.width;
			        var scaledHeight = this.canvas.width*scale;
		        	var scaledWidth = this.canvas.height*scale;
			        var marginLeft = ( this.canvas.width - scaledWidth)/2;
			        this.ctx.drawImage(image, marginLeft, 0, scaledWidth, scaledHeight); // draw video
		         }

instead of:

               if (this.orientation === 'portrait') {
			this.ctx.translate(this.canvas.width, 0);
			this.ctx.rotate(Math.PI/2);
			this.ctx.drawImage(image, 0, 0, this.canvas.height, this.canvas.width); // draw video
		} else {
			this.ctx.drawImage(image, 0, 0, this.canvas.width, this.canvas.height); // draw video
		}

Applying these changes to the code and rebuilding the jsartoolkit5 libs solve this issue. Anyway i will prefer to not apply these changes to the jsartoolkit5 and find another solution.
More to come...

EDIT: related to these last words we can't add these changes to the artoolkitx/jsartoolkit5 because will give weird result on portrait mode...

[kalwalt]

If someone want to test the modified jsartoolkit5 version, as i explained in the previous comment, may grab the artoolkit.min.js from this branch https://github.com/kalwalt/jsartoolkit5/tree/fix-copyImageToHeap, and modify the Makefile as i explained in the comment at the top. After test the threejs basic example. other examples require to delete the artoolkit.api.js script import.

[kalwalt]

i have a testing AR.js branch on my repository, tests are welcome!!

[hatsumatsu]

getUserMediaARController is never used inside AR.js

@kalwalt Just curious, what's the reason to not use it in AR.js?

[kalwalt]

I suppose, because i didn't the first implementation of the lib, that the reason is Aframe, probably require a particular set-up, but i'm not a great expert with Aframe. Or maybe a deliberate code design choice?