processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.34k stars 3.26k forks source link

Mouse/touch event handling bug #4223

Open dhowe opened 4 years ago

dhowe commented 4 years ago

I'm not positive that this is a bug, but it sure is confusing that the code below results in 3 very different behaviors in the browser, depending on whether devtools is open and on the device selected.

To recreate:

Three behaviors (with mouse-events):

  1. With no js console closed, sketch works as expected
  2. with console opened but no device emulator, rect and mouse are out of sync
  3. with a touch-device in device emulator, mousePressed is never called and the rect is undraggable

With touch-events (comment mouse-events and uncomment touch-events below), the only difference is that in case C, we get behavior B (rect & mouse are out of sync)

  let x = 100, y = 100, active = false;

  function setup() {
    createCanvas(400, 400);
  }

  function draw() {
    background(0);
    rect(x, y, 50, 50);
  }

  //function touchEnded() {
  function mouseReleased() {
    //console.log('mouseReleased');
    active = false;
  }

  //function touchStarted() {
  function mousePressed() {
    //console.log('mousePressed', mouseX-pmouseX);
    active = (mouseX > x && mouseX < x + 50 && mouseY > y && mouseY < y + 50);
  }

  //function touchMoved() {
  function mouseDragged() {
    //console.log('mouseDragged:',mouseX-pmouseX);
    if (active) {
      x += mouseX - pmouseX;
      y += mouseY - pmouseY;
    }
  }
limzykenneth commented 4 years ago

I first thought this is a window resize problem but it still behaves weird when I pop the inspector window out on its own.

It is also exclusive to Chrome when inspector is opened and Safari without anything else opened, Firefox doesn't seem to exhibit any weird behaviour.

I'm not sure what caused this problem but it may be the framerate and event loop getting out of sync or something like that.

dhowe commented 4 years ago

Thanks for taking a look @limzykenneth ... any thoughts on next steps? Meanwhile, as you've confirmed, I'll label as a bug...

limzykenneth commented 4 years ago

I'll need to do some more tests, I'll report back if I find any insights.

limzykenneth commented 4 years ago

It is pretty much an issue of framerate and event loop not matching. Specifically, _updateNextMouseCoords is called by the event handler while _updateMouseCoords is called every frame and such there is the possibility of mouseX/Y having been updated with new values but pmouseX/Y haven't yet and so the difference accumulates.

To test, if _updateMouseCoords is called in _updateNextMouseCoords just before setting mouseX/Y the sketch above will work as expected but

line(mouseX, mouseY, pmouseX, pmouseY);

will no longer work as expected (see #1471 for some info).

I have no idea why the behaviour is so weird on Chrome, probably a Chrome bug. I also have no idea how to reconcile the two. We probably will prefer to update these values per frame but _updateNextMouseCoords rely on having an event object in order to update values so it needs to get that event object somehow, while at the same time using the event handler for events is more idiomatic. Without fundamentally changing the way these values are updated, I'm drawing a blank on how to solve this. Maybe some one else can have a crack at this.

dhowe commented 4 years ago

@limzykenneth thanks for the research -- this is clearly part of the problem, but in my testing mousePressed() doesn't even fire when on a touch-device and dragging (case 3 above)...

zenzoa commented 4 years ago

I am seeing the same thing with a game I'm working on in p5js. I put console.log's in mousePressed() and mouseReleased(), and on Safari on my iPad Pro, it only prints the mouseReleased one. I was able to get touchStarted() to work though, so for now I'm duplicating the mousePressed() code in there.

ykabusalah commented 4 years ago

Has anyone been assigned to this? Could I tackle this?

limzykenneth commented 4 years ago

@ykabusalah No assignment has been made yet as we are still figuring out where the problem is and where any potential solution is. You can have a look at the problem and let us know here what you think. Thanks.

trych commented 3 years ago

I have a sketch, where pmouseX does not work at all, but has a certain constant – but seemingly random – value on each frame (like 107 or 366, no idea where these numbers are coming from). This value then stays with pmouseX, no matter how much I move the mouse. Is this the same issue, or should I open a new issue for that?

Oh and btw. this issue happens on all three browsers for me: Chrome, Firefox and Safari.

limzykenneth commented 3 years ago

@trych You will have to include a minimal sketch demonstrating the issue as I can't determine from your description alone whether it's a related issue or not. Thanks.

Vishal2002 commented 6 months ago

@limzykenneth Sir I tried to find the root cause for this but failed . I think there might be some issue with Canvas and the predefined CSS with it.

limzykenneth commented 2 months ago

Coming back to this after a long time, I'll summarize my suggested action for the first two behaviours noted in the original post. The problem is caused by the event loop (mouseDragged()) executing more often than the animation loop (draw()), the animation loop is always determined by the browser, usually the screen refresh rate, but the event loop does not necessarily execute at the same rate (on Firefox it seems to be capped to be the same as the animation loop rate but on Chrome, when the inspector is opened, the event fires more often than the animation loop).

Another piece that completes the problem here is that mouseX/Y are updated in the event (which can execute more often than the animation loop) while pmouseX/Y are updated in the animation loop. This is desired behaviour as pmouseX/Y only has meaning when it is updated in the animation loop since it stands for the mouse X/Y value in the previous frame. At the same time mouseX/Y should be updated in the event loop to make a no loop mouse event only sketch possible.

My suggestion for fixing the example sketch will be to not use mouseDragged() but to update the x and y values only in draw():

let x = 100, y = 100, active = false;

function setup() {
    createCanvas(windowWidth, windowHeight);
    // noLoop();
}

function draw() {
    background(0);
    // console.log("frame");
    if (active) {
        x += mouseX - pmouseX;
        y += mouseY - pmouseY;
    }
    rect(x, y, 50, 50);
}

//function touchEnded() {
function mouseReleased() {
    //console.log('mouseReleased');
    active = false;
}

//function touchStarted() {
function mousePressed() {
    //console.log('mousePressed', mouseX-pmouseX);
    active = (mouseX > x && mouseX < x + 50 && mouseY > y && mouseY < y + 50);
}

//function touchMoved() {
// function mouseDragged() {
//  //console.log('mouseDragged:',mouseX-pmouseX);
//  if (active) {
//      x += mouseX - pmouseX;
//      y += mouseY - pmouseY;
//  }
// }

This points to a larger suggestion which is to not use pmouseX/Y in event handlers. This can be noted in the documentation and possibly try to catch with FES but I'll help the pro5 grantee to explore this possibility.


The third issue I still need to look into.

isohedral commented 2 weeks ago

I found this report after experiencing similarly perplexing behaviour in my code (dragging objects moved them too fast when the Javascript console was open). I'm quite surprised that pmouseX and pmouseY are updated only once per frame. The documentation certainly doesn't reflect that, and it seems to me that changing one's code to update based on, say, mouseX-pmouseX only in draw() is clunky and overly conflates the view and controller aspects of a program. There's also the somewhat embarrassing fact that I've been teaching students to implement direct manipulation this way for about ten years (good thing I'm revising the course).

I suppose that for now I'll try rolling my own (creating local variables to track previous and current mouse positions and updating them as events are received). But I'll be interested to learn about the ultimate resolution of this issue.

limzykenneth commented 2 weeks ago

@isohedral A very common teaching sketch using pmouseX and pmouseY is as follow:

function setup() {
  createCanvas(400, 400);
}

function draw() {
  line(mouseX, mouseY, pmouseX, pmouseY);
}

If pmouseX and pmouseY were to update per event instead of per frame and the event rate is faster than the frame rate, it will break the sketch entirely (expected output is a rudimentary drawing behavior but actual outcome will be a series of dots instead). This is one of the main reason why pmouseX and pmouseY is updated per frame.

It can be possible to introduce another pair of variables that does keep track of previous values per event but just to point out there are use cases where per frame update is required.

isohedral commented 2 weeks ago

Thanks, that makes sense. And the good news is that I solved my proximal problem in the way I suggested above—I created my own pmouseX and pmouseY variables that I update in my event handlers. (This still doesn't explain the weird fact that the behaviour is different depending on whether Chrome's Javascript console is open, but I won't worry about that for now.)

I think you're right that there isn't an obvious one-size-fits-all solution that covers the use of these variables in both draw() and event handlers. Introducing new per-event variables might cause too much confusion for novices like my undergraduate students. And maybe this is a sign that I'm pushing P5.js too hard in my own work, asking it to scale up to a level of complexity it isn't designed for.

For the record, my canonical direct manipulation demo is basically identical to @dhowe's post at the top of this thread (but with a circle instead of a rectangle). I may continue to teach it this way in intro CS, but include a small note pointing out that it's not quite correct. Oh well.

ksen0 commented 2 weeks ago

Hello, this is a really interesting issue and I'd like to try working on it if no one else is. I was able to also reproduce the behavior in Safari (with and without developer tooling/logging), which makes sense given @limzykenneth's findings above - that the behavior is due to browser-specific differences in event and animation loop execution rates.

I'm not sure whether I am capable of solving this, but if no one is working on it, I'd like to try. Specifically, I'd start with understanding the src/events/mouse.js and the logic of pmouseX. The goal for the expected behavior that is browser-agnostic, and one way is to detect event rate difference and adjust for them, so I'd look into how possible (and how potentially messy?) that would be.

It would be my first attempt to contribute to p5.js, please let me know if I could work on it, and if so, if the above approach seems alright / what else I should be aware of. I think this is potentially a really cool problem to solve in terms of accessibility; maybe there are other niche places where loop event rate mismatch causes difficult-to-explain problems?

limzykenneth commented 1 week ago

@ksen0 In terms of detecting event rate difference, once we know that how can we adjust for them? For the most part I don't think there is a clear way to resolve this as pmouseX/Y updating at either the event rate or the animation rate are two different and valid cases with their own use cases, which can be seen from the examples @isohedral, @dhowe and I shared.

If you can work out a bit more details here in terms of what the proposed implementation/fix is that would be great. Thanks.