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

Rollup: Added bubleCleanup. #19934

Merged
merged 2 commits into from
Jul 27, 2020
Merged

Rollup: Added bubleCleanup. #19934

merged 2 commits into from
Jul 27, 2020

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Jul 27, 2020

Ended up adding another transform function to clean up's buble output.

Before:

var BoxGeometry = /*@__PURE__*/(function (Geometry) {
	function BoxGeometry( width, height, depth, widthSegments, heightSegments, depthSegments ) {

		Geometry.call(this);

		this.type = 'BoxGeometry';

		this.parameters = {
			width: width,
			height: height,
			depth: depth,
			widthSegments: widthSegments,
			heightSegments: heightSegments,
			depthSegments: depthSegments
		};

		this.fromBufferGeometry( new BoxBufferGeometry( width, height, depth, widthSegments, heightSegments, depthSegments ) );
		this.mergeVertices();

	}

	if ( Geometry ) BoxGeometry.__proto__ = Geometry;
	BoxGeometry.prototype = Object.create( Geometry && Geometry.prototype );
	BoxGeometry.prototype.constructor = BoxGeometry;

	return BoxGeometry;
}(Geometry));

After:

function BoxGeometry( width, height, depth, widthSegments, heightSegments, depthSegments ) {

	Geometry.call(this);

	this.type = 'BoxGeometry';

	this.parameters = {
		width: width,
		height: height,
		depth: depth,
		widthSegments: widthSegments,
		heightSegments: heightSegments,
		depthSegments: depthSegments
	};

	this.fromBufferGeometry( new BoxBufferGeometry( width, height, depth, widthSegments, heightSegments, depthSegments ) );
	this.mergeVertices();

}

BoxGeometry.prototype = Object.create( Geometry.prototype );
BoxGeometry.prototype.constructor = BoxGeometry;

@mrdoob
Copy link
Owner Author

mrdoob commented Jul 27, 2020

@Mugen87 I think we should be able to proceed converting src to ES6 classes?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 27, 2020

Basically yes. However, the problem is that certain examples in the js directory can't be converted to classes although it would be necessary. E.g. all loaders would have to be classes when THREE.Loader becomes a class (since the jsm modules are build from js). Same for everything that inherits from Object3D, BufferGeometry and so on. Hence, the last time this topic was discussed it was suggested to wait with the class transition until examples/js is gone (end of the year).

@mrdoob
Copy link
Owner Author

mrdoob commented Jul 27, 2020

I see. But we should be able to do some folders? src/math, src/geometries, ...?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 27, 2020

src/math should be safe since nothings inherits from it. src/geometries has some derivatives in examples/js though (e.g. in ParametricGeometries).

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 27, 2020

Wait not true. GLTFLoader creates a custom interpolant from THREE.Interpolant. So even src/math can't be completely done.

@mrdoob
Copy link
Owner Author

mrdoob commented Jul 27, 2020

How about converting as many files as we can and add a comment at the top of the ones that we can't with the reason?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 27, 2020

That's of course an option. I just wonder if we should focus ES6 module migration now if we can't complete it anyway. At the end of the year, it can be done in one go.

May I ask why you want to move forward with it now? 😇

@mrdoob
Copy link
Owner Author

mrdoob commented Jul 27, 2020

Tree-shaking.

@mrdoob
Copy link
Owner Author

mrdoob commented Jul 27, 2020

I did not want to move forward before because the output of buble was adding an uneeded extra function to every class in three.js and three.min.js. Now that the output it's cleaner I feel better about it.

@mrdoob
Copy link
Owner Author

mrdoob commented Jul 27, 2020

Basically yes. However, the problem is that certain examples in the js directory can't be converted to classes although it would be necessary. E.g. all loaders would have to be classes when THREE.Loader becomes a class (since the jsm modules are build from js). Same for everything that inherits from Object3D, BufferGeometry and so on. Hence, the last time this topic was discussed it was suggested to wait with the class transition until examples/js is gone (end of the year).

I think I'll have to give it a go, because I don't understand where the problem is coming from. And if there is a problem that the only solution is to wait until the end of the year.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 27, 2020

see #17425 (comment)

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 27, 2020

However, if you want to improve Tree-shaking capability now, it's of course okay to migrate what's possible.

@DefinitelyMaybe
Copy link
Contributor

@mrdoob previously I parsed through src/ and examples/ and made a list of the different file dependencies (comment)

I did not want to move forward before because the output of buble was adding an uneeded extra function to every class in three.js and three.min.js. Now that the output it's cleaner I feel better about it.

Ah, makes sense, wish I had of asked about the why earlier and then I could've had a look into that myself

@mrdoob
Copy link
Owner Author

mrdoob commented Jul 28, 2020

@DefinitelyMaybe Very useful! Although, rather than "used in examples", what we want to know is "extended in examples", no?

@DefinitelyMaybe
Copy link
Contributor

ah, true. Though not bad as some cautionary first steps? I can refine it later based on whats been done.

I've also noted this conversation within the #11552 as the folks over there will like the news.

@ianpurvis
Copy link
Contributor

ianpurvis commented Jul 31, 2020

@mrdoob @DefinitelyMaybe I think I've found an issue where end1 is not matching and results in breakage like this:

[ROLLUP] bundles src/Three.js → build/three.js...
[ROLLUP] (!) Error when using sourcemap for reporting an error: Can't resolve original location of error.
[ROLLUP] src/audio/AudioListener.js: (137:1)
[ROLLUP] [!] Error: 'return' outside of function
[ROLLUP] src/audio/AudioListener.js (137:1)
[ROLLUP] 135:   };
[ROLLUP] 136: 
[ROLLUP] 137:   return AudioListener;
[ROLLUP]        ^
[ROLLUP] 138: }(Object3D));
[ROLLUP] Error: 'return' outside of function

This was while converting src/audio/AudioListener.js to an ES6 class for #11552. I dumped the transform before/after.

Before:

import { Vector3 } from '../math/Vector3.js';
import { Quaternion } from '../math/Quaternion.js';
import { Clock } from '../core/Clock.js';
import { Object3D } from '../core/Object3D.js';
import { AudioContext } from './AudioContext.js';

var _position = new Vector3();
var _quaternion = new Quaternion();
var _scale = new Vector3();
var _orientation = new Vector3();

var AudioListener = /*@__PURE__*/(function (Object3D) {
	function AudioListener() {

		Object3D.call(this);

		this.type = 'AudioListener';

		this.context = AudioContext.getContext();

		this.gain = this.context.createGain();
		this.gain.connect( this.context.destination );

		this.filter = null;

		this.timeDelta = 0;

		// private

		this._clock = new Clock();

	}

	if ( Object3D ) AudioListener.__proto__ = Object3D;
	AudioListener.prototype = Object.create( Object3D && Object3D.prototype );
	AudioListener.prototype.constructor = AudioListener;

	AudioListener.prototype.getInput = function getInput () {

		return this.gain;

	};

	AudioListener.prototype.removeFilter = function removeFilter () {

		if ( this.filter !== null ) {

			this.gain.disconnect( this.filter );
			this.filter.disconnect( this.context.destination );
			this.gain.connect( this.context.destination );
			this.filter = null;

		}

		return this;

	};

	AudioListener.prototype.getFilter = function getFilter () {

		return this.filter;

	};

	AudioListener.prototype.setFilter = function setFilter ( value ) {

		if ( this.filter !== null ) {

			this.gain.disconnect( this.filter );
			this.filter.disconnect( this.context.destination );

		} else {

			this.gain.disconnect( this.context.destination );

		}

		this.filter = value;
		this.gain.connect( this.filter );
		this.filter.connect( this.context.destination );

		return this;

	};

	AudioListener.prototype.getMasterVolume = function getMasterVolume () {

		return this.gain.gain.value;

	};

	AudioListener.prototype.setMasterVolume = function setMasterVolume ( value ) {

		this.gain.gain.setTargetAtTime( value, this.context.currentTime, 0.01 );

		return this;

	};

	AudioListener.prototype.updateMatrixWorld = function updateMatrixWorld ( force ) {

		Object3D.prototype.updateMatrixWorld.call( this, force );

		var listener = this.context.listener;
		var up = this.up;

		this.timeDelta = this._clock.getDelta();

		this.matrixWorld.decompose( _position, _quaternion, _scale );

		_orientation.set( 0, 0, - 1 ).applyQuaternion( _quaternion );

		if ( listener.positionX ) {

			// code path for Chrome (see #14393)

			var endTi�e = this.context.currentTime + this.timeDelta;

			listener.positionX.linearRampToValueAtTime( _position.x, endTime );
			listener.positionY.linearRampToValueAtTime( _position.y, endTime );
			listener.positionZ.linearRampToValueAtTime( _position.z, endTime );
			listener.forwardX.linearRampToValueAtTime( _orientation.x, endTime );
			listener.forwardY.linearRampToValueAtTime( _orientation.y, endTime );
			listener.forwardZ.linearRampToValueAtTime( _orientation.z, endTime );
			listener.upX.linearRampToValueAtTime( up.x, endTime );
			listener.upY.linearRampToValueAtTime( up.y, endTime );
			listener.upZ.linearRampToValueAtTime( up.z, endTime );

		} else {

			listener.setPosition( _position.x, _position.y, _position.z );
			listener.setOrientation( _orientation.x, _orientation.y, _orientation.z, up.x, up.y, up.z );

		}

	};

	return AudioListener;
}(Object3D));

export { AudioListener };

After:

import { Vector3 } from '../math/Vector3.js';
import { Quaternion } from '../math/Quaternion.js';
import { Clock } from '../core/Clock.js';
import { Object3D } from '../core/Object3D.js';
import { AudioContext } from './AudioContext.js';

var _position = new Vector3();
var _quaternion = new Quaternion();
var _scale = new Vector3();
var _orientation = new Vector3();

	function AudioListener() {

		Object3D.call(this);

		this.type = 'AudioListener';

		this.context = AudioContext.getContext();

		this.gain = this.context.createGain();
		this.gain.connect( this.context.destination );

		this.filter = null;

		this.timeDelta = 0;

		// private

		this._clock = new Clock();

	}

	if ( Object3D ) AudioListener.__proto__ = Object3D;
	AudioListener.prototype = Object.create( Object3D && Object3D.prototype );
	AudioListener.prototype.constructor = AudioListener;

	AudioListener.prototype.getInput = function getInput () {

		return this.gain;

	};

	AudioListener.prototype.removeFilter = function removeFilter () {

		if ( this.filter !== null ) {

			this.gain.disconnect( this.filter );
			this.filter.disconnect( this.context.destination );
			this.gain.connect( this.context.destination );
			this.filter = null;

		}

		return this;

	};

	AudioListener.prototype.getFilter = function getFilter () {

		return this.filter;

	};

	AudioListener.prototype.setFilter = function setFilter ( value ) {

		if ( this.filter !== null ) {

			this.gain.disconnect( this.filter );
			this.filter.disconnect( this.context.destination );

		} else {

			this.gain.disconnect( this.context.destination );

		}

		this.filter = value;
		this.gain.connect( this.filter );
		this.filter.connect( this.context.destination );

		return this;

	};

	AudioListener.prototype.getMasterVolume = function getMasterVolume () {

		return this.gain.gain.value;

	};

	AudioListener.prototype.setMasterVolume = function setMasterVolume ( value ) {

		this.gain.gain.setTargetAtTime( value, this.context.currentTime, 0.01 );

		return this;

	};

	AudioListener.prototype.updateMatrixWorld = function updateMatrixWorld ( force ) {

		Object3D.prototype.updateMatrixWorld.call( this, force );

		var listener = this.context.listener;
		var up = this.up;

		this.timeDelta = this._clock.getDelta();

		this.matrixWorld.decompose( _position, _quaternion, _scale );

		_orientation.set( 0, 0, - 1 ).applyQuaternion( _quaternion );

		if ( listener.positionX ) {

			// code path for Chrome (see #14393)

			var endTime = this.context.currentTime + this.timeDelta;

			listener.positionX.linearRampToValueAtTime( _position.x, endTime );
			listener.positionY.linearRampToValueAtTime( _position.y, endTime );
			listener.positionZ.linearRampToValueAtTime( _position.z, endTime );
			listener.forwardX.linearRampToValueAtTime( _orientation.x, endTime );
			listener.forwardY.linearRampToValueAtTime( _orientation.y, endTime );
			listener.forwardZ.linearRampToValueAtTime( _orientation.z, endTime );
			listener.upX.linearRampToValueAtTime( up.x, endTime );
			listener.upY.linearRampToValueAtTime( up.y, endTime );
			listener.upZ.linearRampToValueAtTime( up.z, endTime );

		} else {

			listener.setPosition( _position.x, _position.y, _position.z );
			listener.setOrientation( _orientation.x, _orientation.y, _orientation.z, up.x, up.y, up.z );

		}

	};

	return AudioListener;
}(Object3D));

export { AudioListener };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants