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.)
-
\$\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– ggorlen2022年10月14日 14:40:37 +00:00Commented 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– gaspar2022年10月15日 06:24:35 +00:00Commented Oct 15, 2022 at 6:24
1 Answer 1
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
andRTOLoop
outside the object's (DT) declaration.Good to see
Object.seal
though my preference is to useObject.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 berequestAnimationFrame(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
andduration
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||
. Egperformance = 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
-
\$\begingroup\$ Many thanks! Just one small question: I'd guess it hardly makes any difference, but why do you use
var
rather thanlet
? \$\endgroup\$gaspar– gaspar2022年10月16日 06:48:56 +00:00Commented 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– Blindman672022年10月17日 10:52:09 +00:00Commented Oct 17, 2022 at 10:52
Explore related questions
See similar questions with these tags.