0
\$\begingroup\$

I created a function using requestAnimationFrame loop that works pretty much like the plain JS setTimeout function (except that a custom starting time can be optionally given for the duration, and it returns the finish time). I wonder whether this is an ideal way of writing it. In particular, I had some issues with self-references while defining the main object, so I had to define them separately. (Maybe it would make more sense to create a class, but I'd like to keep it as simple as possible.)

const DT = {
    // initiate RTOloop ("setTimeout" via RAF loop)
    // actually defined below, because of self-reference
    setRTO: undefined,
    // RTOloop actually defined below, because of self-reference
    RTOloop: undefined,
    // start time for setRTOLoop 
    RTOstart: undefined,
    // duration for setRTOLoop 
    RTOduration: undefined,
    // callback function for setRTOLoop 
    RTOcallback: undefined,
    // now: gets the appropriate version of "performance.now()"
    // normally available in all modern browsers, but it can resort to the still precise Date()
    // see https://developer.mozilla.org/en-US/docs/Web/API/Performance/now
    now: function() {
        let performance = window.performance || {};
        performance.now = (function() {
            return (
                performance.now ||
                performance.webkitNow ||
                performance.msNow ||
                performance.oNow ||
                performance.mozNow ||
                function() {
                    return new Date().getTime();
                }
            );
        })();
        return performance.now();
    }
};

// add the RAF Time-Out (RTO) initiation function definition
DT.setRTO = function(callback, duration, start = DT.now()) {
    DT.RTOstart = start;
    DT.RTOduration = duration;
    DT.RTOcallback = callback;
    DT.RTOloop(start);
};

// add the RAF setRTO function definition
DT.RTOloop = function(timestamp) {
    if (timestamp - DT.RTOstart < DT.RTOduration) {
        requestAnimationFrame((RAFtimestamp) => DT.RTOloop(RAFtimestamp));
    } else {
        DT.RTOcallback(timestamp, DT.RTOstart);
        DT.RTOcallback = undefined;
        DT.RTOstart = undefined;
        DT.RTOduration = undefined;
    }
};

Object.seal(DT);

// example use
DT.setRTO((stamp, start) => console.log(stamp - start), 500);

(Edit: This function is intended to be called from within an initial RAF callback, hence the timing is expected to be in sync with repaints.)

\$\endgroup\$
2
  • \$\begingroup\$ What's the point of this code? What problem is it solving for an actual app that can't be solved in a better way? \$\endgroup\$
    – ggorlen
    Commented Oct 14, 2022 at 14:40
  • \$\begingroup\$ It is to ensure that the display timing is synchronized with the screen repaint times; using RAF has been repeatedly shown to outperform setTimout (or setInterval) in providing more precise durations (desired number of frames) and timing measurements (via timestamps) of element displays (see e.g. here). I'm simply just trying to implement a nice function for this general purpose. \$\endgroup\$
    – gaspar
    Commented Oct 15, 2022 at 6:24

1 Answer 1

2
\$\begingroup\$

I must point out that I can see no usefulness in this code at all. Its acts like a timer with a randomized 16.66...ms error and provides no frame sync at all.

requestAnimationFrame does not sync with display frames, rather it tells the compositor to hold visual changes made in the rAF callback (stored in back buf) until safe to move to display buffer (during VSync).

Most browsers these days also do this with setTimeout and setInterval

Back to the review.

General points

  • Better to implement DT via a factory rather than flat code (see rewrite)

  • Not sure why you define setRTO and RTOLoop outside the object's (DT) declaration.

  • Good to see Object.seal though my preference is to use Object.freeze. This forces the exposed object to act more like an interface, allowing only getters and setters to modify state and ensure object integrity.

  • Avoid indirect calls as they are just a waste of source code characters and CPU cycles. Eg requestAnimationFrame((RAFtimestamp) => DT.RTOloop(RAFtimestamp)); can be requestAnimationFrame(DT.RTOloop);

  • You are not guarding your object's state. callback is not vetted as a function and thus can throw in your code. start and duration are not checked to be numbers and within a valid range.

    Always ensure your object has a valid state.

Undefined unwanted behaviour

There is nothing preventing a new call to setRTO starting a new rAF loop while an existing loop is running. This could cause a serious resource drain as more and more loops wait to terminate.

You need to ensure existing loops will terminate when expected, or prevent new loops starting before the current loop has ended. The rewrite (below) uses the semaphore running within DT's closure to prevent more than one loop running at a time.

Use modern JS.

At first I thought you were aiming for legacy browser support, however you have modern syntax (arrow function in DT.RTOLoop and default parameter in DT.setRTO) which negates all of the old style code, and I question the need to define performance.now, if the browser supports arrow function, performance.now is a given.

Modern JS points.

  • More arrow functions.
  • Object function shorthand declaration. { now(){} } rather than { now: function(){} }
  • ?? rather than ||. Eg performance = window.performance ?? {}

Rewrite

  • Rewrite creates a single instance of DT via a IIFE using a factory pattern.
  • DT is simplified to one property setRTO
  • The behaviour is vetted against invalid parameters and will do nothing if not given good data.
  • Will not start a new timer until existing one (if any) has completed.

const DT = (() => {
    const isNum = val => !isNaN(val) && val < Infinity; //  Special case Infinity is not a number
    var start, duration, callback, running;
    function onFrame(time) {
        time - start < duration ? 
            requestAnimationFrame(onFrame) : 
            (running = false, callback(time, start));
    }
    return Object.freeze({
        setRTO(cb, duration_, start_ = performance.now()) {
            if (cb instanceof Function && isNum(duration_) && isNum(start_) && !running) {
                callback = cb;
                start = start_;
                duration = duration_;
                running = true;
                onFrame(start);
            } else {
                // todo Fail code here
            }
        }
    });
})();


DT.setRTO((stamp, start) => console.log(stamp - start), 500);
DT.setRTO((stamp, start) => console.log(stamp - start), 100);  // this timer will not start

\$\endgroup\$
2
  • \$\begingroup\$ Many thanks! Just one small question: I'd guess it hardly makes any difference, but why do you use var rather than let? \$\endgroup\$
    – gaspar
    Commented Oct 16, 2022 at 6:48
  • 1
    \$\begingroup\$ @gaspar To make it clear that my intention is that the variables are function scoped \$\endgroup\$
    – Blindman67
    Commented Oct 17, 2022 at 10:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.