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

WIP - Feature threejs r103 #515

Open
wants to merge 24 commits into
base: dev
from

Conversation

Projects
None yet
2 participants
@kalwalt
Copy link

commented Apr 29, 2019

What kind of change does this PR introduce?
This PR upgrade the Three.js version (r103) in the examples and in other places. it upgrade some deprecated method ( note this is not directly related to Three.js):

- onResize()
+ onResizeElement()
- copySizeTo 
+ copyElementSizeTo()

and in some examples and in threex-armarkerhelper.js:

+ THREE.AxesHelper()
- THREE.AxisHelper()

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

How can we test it?

Tested in Ubuntu 18.04 and Android. It willl be nice if other developers can test it in other OS.
Summary

The Three.js version included is very old, this issue solve some issue related to Three.js version and make possible to benefit of the new features and bug fixes of the lib.
Does this PR introduce a breaking change?
I hope no.
Other information
Maybe i will add a new example with some new Three.js feature.

@kalwalt

This comment has been minimized.

Copy link
Author

commented Apr 29, 2019

The above commits are not connected to the Three.js library but I thought it useful to upgrade, Note that the dev example already uses these methods:

arToolkitSource.onResizeElement()

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 2, 2019

In the console i receive also this warning:

THREE.AxisHelper has been renamed to THREE.AxesHelper.

I think i colud make also these changes, renaming:

- THREE.AxisHelper 
+ THREE.AxesHelper.

it is located 29 times in 27 files, but it worth to change them.

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 9, 2019

i have this issue: after i built the libs with the makefile command, when i run the basic aframe example
(Ar.js/aframe/examples/basic.html) i get this message:

A-Frame Version: 0.6.1 (Date 29-07-2017, Commit #1bfc562) aframe.min.js:41:18046
three Version: ^0.86.0 aframe.min.js:41:18119
WebVR Polyfill Version: ^0.9.36 aframe.min.js:41:18175
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key minConfidence may not be recognized aframe.min.js:2:19179
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothCount may not be recognized aframe.min.js:2:19179
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothTolerance may not be recognized aframe.min.js:2:19179
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothThreshold may not be recognized aframe.min.js:2:19179
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key hit-testing-renderDebug may not be recognized aframe.min.js:2:19179
Live reload enabled. basic.html:60:4
Successfully compiled asm.js code (total compilation time 207ms) aframe-ar.js
THREE.WebGLRenderer 86 aframe.min.js:15:19494
AR.js 1.6.2 - trackingBackend: artoolkit aframe-ar.js:7139:2
Allocated videoFrameSize 1228800 aframe-ar.js:2:23001
Pattern detection mode set to 1. aframe-ar.js:2:23001
Pattern ratio size set to 0.500000. aframe-ar.js:2:23001
L’utilizzo di mozImageSmoothingEnabled è deprecato. Al suo posto utilizzare la proprietà senza prefisso imageSmoothingEnabled. aframe-ar.js:5435:2
ARjs.Anchor - changeMatrixMode: modelViewMatrix / markersAreaEnabled: true aframe-ar.js:6599:2

look the Three.js version printed in the console is 86 ! It should be 103...

Instead the Three.js example (Ar.js/three.js/examples/basic.html):

THREE.WebGLRenderer 103 three.min.js:184:454
Live reload enabled. basic.html:191:4
Successfully compiled asm.js code (total compilation time 242ms) ar.js
Allocated videoFrameSize 1228800 ar.js:2:23001
Pattern detection mode set to 1. ar.js:2:23001
Pattern ratio size set to 0.500000. ar.js:2:23001
L’utilizzo di mozImageSmoothingEnabled è deprecato. Al suo posto utilizzare la proprietà senza prefisso imageSmoothingEnabled. ar.js:5435:2

i get the actual version 103.
I have the suspect that the aframe in use needs r86? or there are other changes required in the code ?
@nicolocarpignoli @janpio what do you think?

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 9, 2019

using the aframe.js i arrived to this conclusion:
it point to this line:

// at line 26036
function WebGLRenderer( parameters ) {

		console.log( 'THREE.WebGLRenderer', REVISION 
);

and the constant is set as:

// at linline 5919
var REVISION = '86';

so aframe require at least revision 86? and maybe i should update aframe to a newer version of it.

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 9, 2019

My initial thought was that Ar.js embedd his own Three.js version but it is not right, aframe has already embedded his own Three.js version. So if i updated the Three.js version and the ThreeX source (see also the Three.axesHelper error in a previuos comment), i should use the latest version of aframe that support the r103. I will try this way.

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 9, 2019

with the latest from aframe:

[Deprecation] document.registerElement is deprecated and will be removed in M73, around March 2019. Please use window.customElements.define instead. See https://www.chromestatus.com/features/4642138092470272 for more details.
module.exports.registerElement @ aframe-v0.9.2.min.js:259
aframe-v0.9.2.min.js:353 A-Frame Version: 0.9.2 (Date 2019-05-06, Commit #f57a1fa)
aframe-v0.9.2.min.js:353 three Version (https://github.com/supermedium/three.js): ^0.102.2
aframe-v0.9.2.min.js:353 WebVR Polyfill Version: ^0.10.10
aframe-v0.9.2.min.js:95 THREE.WebGLRenderer 103dev
aframe-v0.9.2.min.js:23 extras:primitives:warn Mapping keys should be specified in lower case. The mapping key minConfidence may not be recognized 
aframe-v0.9.2.min.js:23 extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothCount may not be recognized 
aframe-v0.9.2.min.js:23 extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothTolerance may not be recognized 
aframe-v0.9.2.min.js:23 extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothThreshold may not be recognized 
aframe-v0.9.2.min.js:23 extras:primitives:warn Mapping keys should be specified in lower case. The mapping key hit-testing-renderDebug may not be recognized 
basic.html:60 Live reload enabled.
aframe-ar.js:7139 AR.js 1.6.2 - trackingBackend: artoolkit
aframe-ar.js:6599 ARjs.Anchor - changeMatrixMode: modelViewMatrix / markersAreaEnabled: true
aframe-ar.js:2 Allocated videoFrameSize 1228800
aframe-ar.js:2 Pattern detection mode set to 1.
aframe-ar.js:2 Pattern ratio size set to 0.500000.

Three.axesHelper error of course disappear see the Three.js version 0.102.2
i could downgrade the Three.js version to match them, I will think about it...

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 11, 2019

I notice more warnings in the console, this is what i get from master:

extras:primitives:warn Mapping keys should be specified in lower case. The mapping key minConfidence may not be recognized 
aframe.js:3217 
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key hit-testing-renderDebug may not be recognized 

and this is from the branch i'm working (synced with dev):

extras:primitives:warn Mapping keys should be specified in lower case. The mapping key minConfidence may not be recognized 
aframe-v0.9.2.js:2396 
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothCount may not be recognized 
aframe-v0.9.2.js:2396 
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothTolerance may not be recognized 
aframe-v0.9.2.js:2396 
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key smoothThreshold may not be recognized 
aframe-v0.9.2.js:2396 
extras:primitives:warn Mapping keys should be specified in lower case. The mapping key hit-testing-renderDebug may not be recognized

Have you an idea where they come from?

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 11, 2019

And also these Violation messages:

aframe-v0.9.2.js:75381 [Violation] Added non-passive event listener to a scroll-blocking 'touchmove' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952
aframe-v0.9.2.js:66763 [Violation] Added non-passive event listener to a scroll-blocking 'touchstart' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952
addEventListeners @ aframe-v0.9.2.js:66763
update @ aframe-v0.9.2.js:66695
initComponent @ aframe-v0.9.2.js:73768
updateProperties @ aframe-v0.9.2.js:73740
updateComponent @ aframe-v0.9.2.js:72422
updateComponents @ aframe-v0.9.2.js:72395
entityLoadCallback @ aframe-v0.9.2.js:72181
emitLoaded @ aframe-v0.9.2.js:73109
aframe-v0.9.2.js:66763 [Violation] Added non-passive event listener to a scroll-blocking 'touchstart' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952
addEventListeners @ aframe-v0.9.2.js:66763
play @ aframe-v0.9.2.js:66707
play @ aframe-v0.9.2.js:74225
initComponent @ aframe-v0.9.2.js:73772
updateProperties @ aframe-v0.9.2.js:73740
updateComponent @ aframe-v0.9.2.js:72422
updateComponents @ aframe-v0.9.2.js:72395
entityLoadCallback @ aframe-v0.9.2.js:72181
emitLoaded @ aframe-v0.9.2.js:73109
dev.html:93 Live reload enabled.
aframe-v0.9.2.js:66763 [Violation] Added non-passive event listener to a scroll-blocking 'touchstart' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952

complete gist

This was referenced May 16, 2019

kalwalt added some commits May 20, 2019

@nicolocarpignoli

This comment has been minimized.

Copy link
Collaborator

commented May 21, 2019

the 'lower-case' warning may be related to #528?

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 21, 2019

the 'lower-case' warning may be related to #528?

yes, @nicolocarpignoli it is related! it seems that is completely solved, but I did only a quick test, will test further today.

@kalwalt

This comment has been minimized.

Copy link
Author

commented May 21, 2019

@nicolocarpignoli one note: in the aframe folder https://github.com/jeromeetienne/AR.js/tree/master/aframe/examples/vendor/aframe/build there are different files (old) we should keep them? for example https://github.com/jeromeetienne/AR.js/blob/master/aframe/examples/vendor/aframe/build/aframe-v0.6.1-three-r86.js
and https://github.com/jeromeetienne/AR.js/blob/master/aframe/examples/vendor/aframe/build/aframe.js
seems the same, they output same version and commit:

A-Frame Version: 0.6.1 (Date 29-07-2017, Commit #1bfc562)
aframe.js:77533 three Version: ^0.86.0

In my opinion if there are no reasonable questions we could remove those old files.
I am also testing with Android in localhost, i willl report my result at the end of the day or at least tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.