WebGLAnimation: Convert to es6 class

[ianpurvis]

Supports #19986

[ianpurvis]

Looks like some e2e tests failed... I'll check it out.

[mrdoob]

Moving internal vars to the module scope eliminates binding issues, but I am not sure they are safe there...

What happens if there are two instances of WebGLAnimation?
Wouldn't context, isAnimating, ... be overwritten?

[ianpurvis]

Moving internal vars to the module scope eliminates binding issues, but I am not sure they are safe there...

What happens if there are two instances of WebGLAnimation?
Wouldn't context, isAnimating, ... be overwritten?

Thanks... in my heart I knew it was wrong 😆

I found a safe solution, we can do a one-time bind to this in the constructor:

constructor() {

	this._context = null;
	this._isAnimating = false;
	this._animationLoop = null;
	this._requestId = null;

	// Re-bind the prototype function so that `this` can be used with rAF:
	this._onAnimationFrame = this._onAnimationFrame.bind( this );

}

[mrdoob]

Now that I know how Symbols work, I think this is the approach I find less weird...

const _context = Symbol( '_context' );
const _isAnimating = Symbol( '_isAnimating' );
const _animationLoop = Symbol( '_animationLoop' );
const _requestId = Symbol( '_requestId' );
const _onAnimationFrame = Symbol( '_onAnimationFrame' );

class WebGLAnimation {

	constructor() {

		this[ _context ] = null;
		this[ _isAnimating ] = false;
		this[ _animationLoop ] = null;
		this[ _requestId ] = null;

		const scope = this;

		this[ _onAnimationFrame ] = function ( time, frame ) {

			scope[ _animationLoop ]( time, frame );

			scope[ _requestId ] = scope[ _context ].requestAnimationFrame( scope[ _onAnimationFrame ] );

		};

	}

	start() {

		if ( this[ _isAnimating ] === true ) return;
		if ( this[ _animationLoop ] === null ) return;

		this[ _requestId ] = this[ _context ].requestAnimationFrame( this[ _onAnimationFrame ] );

		this[ _isAnimating ] = true;

	}

	stop() {

		this[ _context ].cancelAnimationFrame( this[ _requestId ] );

		this[ _isAnimating ] = false;

	}

	setAnimationLoop( callback ) {

		this[ _animationLoop ] = callback;

	}

	setContext( value ) {

		this[ _context ] = value;

	}

}

export { WebGLAnimation };

IE11 doesn't support Symbol though. But, I guess we could turn Symbol( '_context' ) to '_context' at build time and pollute the API in three.js and three.min.js 🤷‍♂️

And, in a distant future... going from this[ _context ] to this.#context would be fairly straightforward.

[mrdoob]

I do not know what are the performance implications of using Symbol though.

[ianpurvis]

👍 👍 I think that should work, I'll glitch a benchmark so that we can start looking at performance.

[ianpurvis]

Alright, hopefully this is a decent starting point! https://button-knotty-tugboat.glitch.me

I was able to get some coverage on macOS and iOS... Looks like Chrome and iOS Safari handle all the es6 techniques pretty evenly. Interestingly, desktop Safari actually favors the symbol and string techniques. But Firefox penalizes both of those techniques heavily.

Chrome 84.0.4147.125 on OS X 10.15.6 64-bit
Running 4 tests in random order...
🏋️ incrementScopedVar x 162,183,036 ops/sec ±0.24% (66 runs sampled)
🏋️ incrementPropViaSymbol x 473,961,157 ops/sec ±1.05% (64 runs sampled)
🏋️ incrementPropViaString x 476,167,672 ops/sec ±0.25% (63 runs sampled)
🏋️ incrementPropDirectly x 477,688,995 ops/sec ±0.24% (63 runs sampled)
💪 Fastest is incrementPropDirectly, incrementPropViaString.

Safari 13.1.2 on OS X 10.15.6
Running 4 tests in random order...
🏋️ incrementScopedVar x 508,801,361 ops/sec ±2.10% (56 runs sampled)
🏋️ incrementPropDirectly x 598,270,143 ops/sec ±0.50% (65 runs sampled)
🏋️ incrementPropViaString x 666,520,642 ops/sec ±0.31% (62 runs sampled)
🏋️ incrementPropViaSymbol x 668,056,577 ops/sec ±0.30% (64 runs sampled)
💪 Fastest is incrementPropViaSymbol, incrementPropViaString.

Firefox 79.0 on OS X 10.15
Running 4 tests in random order...
🏋️ incrementScopedVar x 405,513,980 ops/sec ±1.30% (64 runs sampled)
🏋️ incrementPropViaString x 125,742,749 ops/sec ±0.27% (66 runs sampled)
🏋️ incrementPropDirectly x 600,459,804 ops/sec ±3.99% (61 runs sampled)
🏋️ incrementPropViaSymbol x 124,670,980 ops/sec ±0.24% (67 runs sampled)
💪 Fastest is incrementPropDirectly.

Safari 14.0 on Apple iPhone (iOS 14.0)
Running 4 tests in random order...
🏋️ incrementScopedVar x 217,876,111 ops/sec ±2.29% (59 runs sampled)
🏋️ incrementPropDirectly x 216,502,987 ops/sec ±0.98% (60 runs sampled)
🏋️ incrementPropViaString x 212,062,590 ops/sec ±0.45% (60 runs sampled)
🏋️ incrementPropViaSymbol x 212,841,235 ops/sec ±0.38% (62 runs sampled)
💪 Fastest is incrementPropDirectly.

[ianpurvis]

Quick heads-up, I moved that benchmark to here and added the WeakMap technique and also the constructor-scoped var technique for completeness. I also started a different benchmark for testing performance when calling_onAnimationFrame with rAF. Feel free to remix as needed or let me know if you want access to edit at the source

[Mugen87]

@mrdoob According to the discussion in #21235, the PR looks good.

[Mugen87]

Closing. We've agreed in #19986 to not change/refactor the code in src/renderers/webgl.