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

asyncCallback sporadically fails to report events #129

Open
hiattp opened this Issue Jun 7, 2014 · 2 comments

Comments

Projects
None yet
2 participants
@hiattp

hiattp commented Jun 7, 2014

I haven't delved into this too deeply beyond observing the effect, but the timeout strategy behind the event callback is dropping events (maybe 1 out of 10 times or so). Given the nature of the issue it is relatively hard to reproduce, but try to observe "start" events (e.g. "drag start") across a series of drags and every once in a while it won't appear when you start dragging.

My guess is that events are stacking up and clearing previous events' timeouts (e.g. an "apply" coming on the back of a "drag start" will clear the "drag start" timeout so that it never calls the callback).

@jwarren1980

This comment has been minimized.

Show comment
Hide comment
@jwarren1980

jwarren1980 Aug 13, 2014

Your guess is correct, and I had to fight over a similar issue before.

The current code for FreeTransform will call your callback event and send status of any recent events that occurred. But if two events occur close to one another, the second event will cancel out the call of the first event before it has had a chance to be fired. The reason is because each event triggers a call to your callback function through a setTimeout method, but each event will first clear any existing timeouts triggered from before. As browsers and CPUs get faster and support more multi-threading, it is possible to have two events triggered within 1ms of each other which makes the current code fail on tracking all events in order.

The solution I used, which works for handling all events as they are called, in order, is to replace the function asyncCallback with the one below:

/**
 * Call callback asynchronously for better performance
 */
function asyncCallback(e)
{
     if (ft.callback)
     {
          // Remove empty values
          var events = [];

          e.map(function (e, i) { if (e) { events.push(e); } });

          //clearTimeout(timeout);

          //timeout = setTimeout(function () { if (ft.callback) { ft.callback(ft, events); } }, 1);

          if (ft.callback) { ft.callback(ft, events); }
     }
}

jwarren1980 commented Aug 13, 2014

Your guess is correct, and I had to fight over a similar issue before.

The current code for FreeTransform will call your callback event and send status of any recent events that occurred. But if two events occur close to one another, the second event will cancel out the call of the first event before it has had a chance to be fired. The reason is because each event triggers a call to your callback function through a setTimeout method, but each event will first clear any existing timeouts triggered from before. As browsers and CPUs get faster and support more multi-threading, it is possible to have two events triggered within 1ms of each other which makes the current code fail on tracking all events in order.

The solution I used, which works for handling all events as they are called, in order, is to replace the function asyncCallback with the one below:

/**
 * Call callback asynchronously for better performance
 */
function asyncCallback(e)
{
     if (ft.callback)
     {
          // Remove empty values
          var events = [];

          e.map(function (e, i) { if (e) { events.push(e); } });

          //clearTimeout(timeout);

          //timeout = setTimeout(function () { if (ft.callback) { ft.callback(ft, events); } }, 1);

          if (ft.callback) { ft.callback(ft, events); }
     }
}
@hiattp

This comment has been minimized.

Show comment
Hide comment
@hiattp

hiattp Aug 13, 2014

Yeah that looks like it would work, thanks!

hiattp commented Aug 13, 2014

Yeah that looks like it would work, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment