Skip to content
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

mouseButton contains incorrect value after pressing both mouse buttons and releasing one of them #6847

Open
1 of 17 tasks
servobot opened this issue Mar 12, 2024 · 11 comments · May be fixed by #6943
Open
1 of 17 tasks

Comments

@servobot
Copy link

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

p5.js version

1.9.0

Web browser and version

122.0.6261.95 (Official Build) (64-bit) (cohort: Stable)

Operating system

Windows 10 Home Edition

Steps to reproduce this

Steps:

  1. Press and hold left mouse button
  2. Press and hold right mouse button
  3. Release left mouse button
    • Expected: console logs left
    • Actual: console logs right

Snippet:

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

function mouseReleased() {
  console.log(mouseButton);
}

function draw() {
  background(220);
}

Additional Context

I discovered this issue when implementing the game of Minesweeper. In that game, pressing and releasing both mouse buttons at the same time is the way to uncover all unmarked neighbors of a cell, and I found that I could not detect this condition reliably using mouseButton. I believe this is due to mouseButton being updated in mousePressed but not mouseReleased.

https://github.com/processing/p5.js/blob/main/src/events/mouse.js#L657
https://github.com/processing/p5.js/blob/main/src/events/mouse.js#L733

I have a pull request (with associated unit test) that appears to resolve the issue. This bug was previously reported in #5733, but the submitter resolved that issue as they found a workaround.

@servobot servobot added the Bug label Mar 12, 2024
Copy link

welcome bot commented Mar 12, 2024

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

@Renatolo
Copy link

Renatolo commented Apr 4, 2024

Hey, I would like to work on this bug, please!

Renatolo added a commit to Renatolo/p5.js that referenced this issue Apr 5, 2024
…s released

- Modified src/events/mouse.js to correctly update the mouseButton value upon releasing one mouse button.
- Updated documentation in src/events/mouse.js accordingly.
- Added unit test in test/unit/events/mouse.js to cover scenario described in the issue.
@limzykenneth
Copy link
Member

This is more or less intended behavior documented and I'm not keen to change the behavior. There is probably room to have better support for simultaneous mouse button presses as currently having the context menu being brought up by the right mouse button can confuse things often.

There is a larger proposal here for better mouse event handling that I think can be explored in p5.js 2.0.

@diyaayay
Copy link
Contributor

diyaayay commented Nov 27, 2024

In the PR: #7378, setting mouseButton on _onmousedown() works well but I noticed pointer events work differently than mouse events meaning _setMouseButton() is only called when a single button is pressed as onpointerdown() is not invoked multiple times on pressing simultaneous mouse buttons. Instead _onpointermove() is triggered when multiple mouse buttons are clicked in pointer events: https://stackoverflow.com/questions/75030295/pointer-events-dont-support-simultaneous-mouse-buttons. So, I attempted to handle mouse buttons by checking which buttons are pressed currently, using the buttons property, which continuously changes as pointerdown, pointermove, and pointerup events occur. This approach clearly shows which buttons are currently pressed during simultaneous button presses. However, this would require updating the description of mouseButton accordingly. @limzykenneth @davepagurek

fn._setMouseButton = function(e) {
    // Using e.buttons bitmask to track which buttons are pressed
    this.mouseButton.left = (e.buttons & 1) !== 0;   // Left button
    this.mouseButton.center = (e.buttons & 4) !== 0;  // Middle button
    this.mouseButton.right = (e.buttons & 2) !== 0;   // Right button
};

@davepagurek
Copy link
Contributor

I'm in favour of replacing the single variable with an object with left, right, and center properties like you're proposing, this seems like a useful breaking change to make.

Do those pointer move events that update the buttons also come attached to a specific pointer ID? I wonder if, in addition to a single global mouseButton property for use in single-pointer sketches, we could also store this same info per pointer in our touches array (or under whatever name we end up giving that.) So, touches[i].mouseButton.left would represent whether the left button is down for a specific pointer, while mouseButton.left would represent whether any pointer has its left button down.

@limzykenneth
Copy link
Member

@diyaayay Would the proposed approach means that if there are only pointerdown event but not pointermove, pressing down on left mouse button followed by right mouse button without releasing the left will cause only left mouse button to be registered?

I haven't quite use this part of things directly very much before but perhaps the element.releasePointerCapture described here may point to a potential solution? If you have a test sketch, do share and I can test it out locally as well.

@davepagurek Pointer events do come with pointer ID so we can identify individual pointer.

@diyaayay
Copy link
Contributor

diyaayay commented Nov 27, 2024

Would the proposed approach means that if there are only pointerdown event but not pointermove, pressing down on left mouse button followed by right mouse button without releasing the left will cause only left mouse button to be registered?

@limzykenneth Yes, that is what I've been seeing. So, for multiple mouse clicks only mouseup and mousedown sets the button that we have been using. You can see the values on multiple button clicks here in an mdn playground here. But, the same thing does not work for pointerup and pointerdown as seen in the pointer example.
But subsequent mouse presses are recognised as pointermove event that match the mouse buttons Working pointer example. I've tested the same thing in sketches also.
I'm not too sure though. I may be missing something. But mouseButton being an object seemed like something that gives more info to the user about pressed and released state of each button.
Some pointer devices, such as mouse or pen, support multiple buttons. In the [UIEVENTS] Mouse Event model, each button press produces a mousedown and mouseup event. To better abstract this hardware difference and simplify cross-device input authoring, Pointer Events do not fire overlapping pointerdown and pointerup events for chorded button presses (depressing an additional button while another button on the pointer device is already depressed).

Instead, chorded button presses can be detected by inspecting changes to the button and buttons properties. The button and buttons properties are inherited from the MouseEvent interface, but with a change in semantics and values.
Ref:

@diyaayay
Copy link
Contributor

Untitled.design.7.mp4

For the sketch:

function mousePressed() {
  console.log(mouseButton);
}

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

function draw() {
  background(220);
}

This is how mouseButton is displayed, but we can change it to not keep it as an object in order to maintain the current behavior.
@davepagurek @limzykenneth

@diyaayay
Copy link
Contributor

I haven't quite use this part of things directly very much before but perhaps the element.releasePointerCapture described here may point to a potential solution? If you have a test sketch, do share and I can test it out locally as well.

I don't think so. setPointerCapture() and releasePointerCapture() take pointerId as a parameter to continue receiving and stop receiving events for a particular pointer id. But multiple mouse button presses would have different ids. What we want is to keep receiving multiple pointer Ids on a particular event i.e. onpointerdown.

@davepagurek
Copy link
Contributor

This is how mouseButton is displayed, but we can change it to not keep it as an object in order to maintain the current behavior.

An object sounds good to me, it lets us indicate when more than one button is pressed, which is something the current API isn't expressive enough for.

@diyaayay
Copy link
Contributor

diyaayay commented Dec 1, 2024

https://editor.p5js.org/diya.solanki.31/sketches/KbLubzBoV

Here is a sketch that shows the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Proposal
Development

Successfully merging a pull request may close this issue.

5 participants