[Bug]: Inconsistency of events when more buttons are used

sushuier commented 4 months ago

CheckList

[X] I agree to follow this project's Code of Conduct
[X] I have read and followed the Contributing Guide
[X] I have read and followed the Issue Tracker Guide
[X] I have searched and referenced existing issues and discussions
[X] I am filing a BUG report.
[X] I have managed to reproduce the bug after upgrading to the latest version
[X] I have created an accurate and minimal reproduction

Version

5.3.0

In What environments are you experiencing the problem?

Chrome

Node Version (if applicable)

14.20.0

Link To Reproduction

https://codesandbox.io/p/devbox/fabric-vanillajs-sandbox-forked-e5r48u?file=%2Fsrc%2Findex.ts

Steps To Reproduce

中文：在画布上绘制任意一个图形，点击鼠标左键选中后，点击鼠标右键，松开鼠标左键和右键；图形会跟随鼠标移动； chatgpt translate：Draw any shape on the canvas. After selecting it by clicking the left mouse button, click the right mouse button and release both the left and right mouse buttons; the shape will follow the mouse movement.

Expected Behavior

中文：点击鼠标右键，不要影响到左键的正常拖拽操作 chatgpt translate: Clicking the right mouse button should not affect the normal drag operation of the left mouse button.

Actual Behavior

code： test video：

https://github.com/user-attachments/assets/d6589354-a98d-4112-9590-62703a908c98

Error Message & Stack Trace

No response

zhe-he commented 3 months ago

嗯。。。鼠标右键的点击也会触发你设置的mouse:down, mouse:move 和 mouse:up。你可以通过opt.e.button判断。它的值可以是以下之一： 0：主鼠标按钮（通常是左键） 1：中间鼠标按钮（通常是滚轮） 2：次鼠标按钮（通常是右键）

// translate Well... A right-click of the mouse will also trigger the mouse:down, mouse:move, and mouse:up events you set. You can determine this by checking opt.e.button. Its value can be one of the following: 0: Primary mouse button (usually the left button) 1: Middle mouse button (usually the scroll wheel) 2: Secondary mouse button (usually the right button)

asturur commented 3 months ago

This is something to fix, but is not trivial, all the interactions are built around the left mouse button and that is very restrictive for mixed events

asturur commented 3 months ago

There are also other issues on the same topic

sushuier commented 3 months ago

@zhe-he 试过，无效

zhe-he commented 3 months ago

@sushuier 我照着你的图片写了份，你试试这种方法是不是可以解决你的问题。 I wrote a version based on your picture. You can try this method to see if it solves your problem.

    const down = (e: TPointerEventInfo<MouseEvent>) => {
      if (e.e.button != 0) return;
      let lastPosX = e.e.clientX;
      let lastPosY = e.e.clientY;
      const move = (ev: MouseEvent) => {
        const vpt = canvas.viewportTransform.slice(0) as TMat2D;
        vpt[4] += ev.clientX - lastPosX;
        vpt[5] += ev.clientY - lastPosY;
        canvas.setViewportTransform(vpt);
        canvas.requestRenderAll();
        lastPosX = ev.clientX;
        lastPosY = ev.clientY;
      };

      document.addEventListener("mousemove", move, { passive: true });
      document.addEventListener(
        "mouseup",
        (_) => {
          document.removeEventListener("mousemove", move);
        },
        { once: true }
      );
    };
    canvas.on("mouse:down", down);

demo

sushuier commented 2 months ago

@sushuier 我照着你的图片写了份，你试试这种方法是不是可以解决你的问题。

    const down = (e: TPointerEventInfo<MouseEvent>) => {
      if (e.e.button != 0) return;
      let lastPosX = e.e.clientX;
      let lastPosY = e.e.clientY;
      const move = (ev: MouseEvent) => {
        const vpt = canvas.viewportTransform.slice(0) as TMat2D;
        vpt[4] += ev.clientX - lastPosX;
        vpt[5] += ev.clientY - lastPosY;
        canvas.setViewportTransform(vpt);
        canvas.requestRenderAll();
        lastPosX = ev.clientX;
        lastPosY = ev.clientY;
      };

      document.addEventListener("mousemove", move, { passive: true });
      document.addEventListener(
        "mouseup",
        (_) => {
          document.removeEventListener("mousemove", move);
        },
        { once: true }
      );
    };
    canvas.on("mouse:down", down);

demo

已测试，无效；这个bug有操作步骤的；你确定有理解我提的bug吗，操作步骤：点击鼠标左键，选中元素，然后点击右键（此时左键不松开），然后同时松开左键右键就会发现元素跟着鼠标跑了

zhe-he commented 2 months ago

@sushuier 已解决 Fixed fix demo

sushuier commented 2 months ago

@sushuier 已解决 fix demo

很棒，果然ok了；可否请教，你是如何找出这个解决方案呢： canvas._currentTransform = null; canvas['_target'] = undefined; canvas['_groupSelector'] = null;

zhe-he commented 2 months ago

Shouldn't we add if (button != 0) return; below line 997 and line 787? Waiting for a fated person to fix this bug.

https://github.com/fabricjs/fabric.js/blob/6d120a0075f0568fdfd00417bf0277a702bf81e3/src/canvas/Canvas.ts#L992-L1000 https://github.com/fabricjs/fabric.js/blob/6d120a0075f0568fdfd00417bf0277a702bf81e3/src/canvas/Canvas.ts#L781-L790

zhe-he commented 2 months ago

@sushuier 鼠标按下的时候，内部会设置临时变量鼠标移动的时候，内部会利用临时变量鼠标抬起的时候，内部会重置临时变量

你列举的这些，是我上面说到的临时变量的一部分。

When the mouse is pressed down, a temporary variable will be set internally. When the mouse is moved, the internal system will utilize the temporary variable. When the mouse is released, the temporary variable will be reset internally. The items you listed are part of the temporary variable I mentioned above.

asturur commented 2 months ago

This issue needs to be fixed in a way that if you press more buttons the events fire anyway. Also you had a discussion in a language i can't read, i m stopped looking honestly

zhe-he commented 2 months ago

This issue needs to be fixed in a way that if you press more buttons the events fire anyway. Also you had a discussion in a language i can't read, i m stopped looking honestly

I'm sorry, translating from Chinese to English and then back to Chinese can lead to ambiguity. Therefore, when I encounter Chinese speakers, I use Chinese to avoid misunderstandings. This is my issue. I have added English to all my responses. Regarding the "at" in the last reply, I tagged the wrong person.

fabricjs / fabric.js