Clock: Consider changing getElapsedTime behavior

[AlaricBaraou]

Description

Over the years, I came across multiple projects that were calling .getElapsedTime() and messing up their deltas instead of reading the .elapsedTime property.

The core problem is that .getElapsedTime() does more than just "get" a value - it has the side effect of updating the internal state by calling .getDelta(). As much as the doc is relatively clear about it, users would typically expect a getter method to be pure and not modify state.

Here is a potential solutions to address this issue.

Solution

Rename the methods to better reflect their behavior ( example below )

    // Rename to make it clear this updates state, a bit of a mouth full but you get the idea
    updateAndGetElapsedTime() {  // formerly getElapsedTime()
        this.updateDelta();      // formerly getDelta()
        return this.elapsedTime;
    }

    getElapsedTime() {
        console.warn('getElapsedTime is deprecated, read the .elapsedTime property')
        return this.elapsedTime;
    }

    getDelta() {
        console.warn('getDelta is deprecated, call updateDelta instead')
    }

    // Rename to make it clear this updates state
    updateDelta() {              // formerly getDelta()
        let diff = 0;
        if (this.autoStart && !this.running) {
            this.start();
            return 0;
        }
        if (this.running) {
            const newTime = now();
            diff = (newTime - this.oldTime) / 1000;
            this.oldTime = newTime;
            this.elapsedTime += diff;
        }
        return diff;
    }

Alternatives

Maybe make the doc clearer, but I've seen this error made by advanced three.js developer so I'm not sure it would help.

Additional context

The issue / misconception being present in community project like Drei and React three fiber and Tres made me feel like it was time to talk about this,

pmndrs/react-three-fiber#3455
pmndrs/drei#2341
https://github.com/Tresjs/tres/pull/928/files

This is also misused in the official examples.
Here each call will call getDelta while a single getDelta followed by reading .elapsedTime would be more appropriate

three.js/examples/misc_raycaster_helper.html

Lines 97 to 98 in eea50d8

	capsule.position.y = Math.sin( clock.getElapsedTime() * 0.5 + capsule.position.x );
	capsule.rotation.z = Math.sin( clock.getElapsedTime() * 0.5 ) * Math.PI * 1;

Same for these that while harmless because they are called immediately after, make unnecessary call to getDelta internally.

three.js/examples/webgl_buffergeometry_lines.html

Lines 120 to 121 in b8d79b8

	const delta = clock.getDelta();
	const time = clock.getElapsedTime();

three.js/examples/webgl_geometry_dynamic.html

Lines 122 to 123 in 6144493

	const delta = clock.getDelta();
	const time = clock.getElapsedTime() * 10;

three.js/examples/webgpu_tsl_vfx_linkedparticles.html

Lines 440 to 442 in 8a475a6

	colorOffset.value += clock.getDelta() * colorRotationSpeed.value * timeScale.value;
	const elapsedTime = clock.getElapsedTime();

Happy to eventually make a PR once the way to go is decided.

[Mugen87]

Would it be an option for you to migrate to THREE.Timer instead?

Doc page: https://threejs.org/docs/index.html#examples/en/misc/Timer

It has been added as a new alternative to THREE.Clock that avoids the issues you have described in your post.

I'm personally not in favor of changing the API of THREE.Clock at this point since this will lead to a lot of confusion and migration tasks. THREE.Timer has been added as a separate module so THREE.Clock can be used as before. That said, it might make sense to deprecate THREE.Clock at some point.

[AlaricBaraou]

This could be an option.

We'd have to extend it while deprecating the previous APIs to allow the usage of old methods like getElapsedTime while the users make the transition.

I guess my only issue is the mandatory _usePageVisibilityAPI. It should be possible to disable it with a parameter in the constructor to give the choice to use it or not in my opinion.
Even though I can imagine way to work around it.

[Mugen87]

It should be possible to disable it with a parameter in the constructor to give the choice to use it or not in my opinion.

Making the usage optional is okay, imo. I vote to let the usage of the API enabled by default but an additional ctor parameter sounds good to me.

[Mugen87]

As an additional note: In context of WebGPURenderer and TSL we use the TSL objects time and deltaTime now when we need time data in the shaders.

three.js/src/nodes/utils/Timer.js

Lines 4 to 18 in 2061c3e

	/**
	* Represents the elapsed time in seconds.
	*
	* @tsl
	* @type {UniformNode<float>}
	*/
	export const time = /*@__PURE__*/ uniform( 0 ).setGroup( renderGroup ).onRenderUpdate( ( frame ) => frame.time );
	/**
	* Represents the delta time in seconds.
	*
	* @tsl
	* @type {UniformNode<float>}
	*/
	export const deltaTime = /*@__PURE__*/ uniform( 0 ).setGroup( renderGroup ).onRenderUpdate( ( frame ) => frame.deltaTime );

There is also the elapsed time that is passed as a parameter to the animation loop. That often makes the usage of a separate timer obsolete. That said, it's still important to have something like THREE.Timer or THREE.Clock around.

[Mugen87]

As a next step should we move Timer into the core and deprecated Clock?

[Mugen87]

Closing, to avoid breaking changes in the Clock module, see #30497 (comment).

The API of Timer has evolved up to a point where it reaches a proper maturity level. IMO, it's ready to move the module into the core.