search

Grubuntu / PieMenu

Fork of PieMenu, with some improvements
8 stars 4 forks source link

Pie doesn't close on "hover" trigger mode #1

Closed pgilfernandez closed 7 months ago

pgilfernandez commented 7 months ago

Hi,

Great to take care of this plugin and make it better!

Under macOS, when using "hover" trigger mode, it happens the following: 1) You hit the pie shortcut and the pie opens 2) you move the mouse pointer to the command you want and it runs 3) right after that the default pie opens and the last mouse pointer position 4) at that point the only way to go out of the loop is to press in the center to cancel the pie as if you move the mouse pointer to any other command it runs and again reopen the default pie

Here there is a litte video showing the behavior:

https://github.com/Grubuntu/PieMenu-Improved/assets/5942369/894f8792-905e-496e-979c-9bd690ad82ec

Thanks!

EDIT:

In the video you can also see a "ghost pie", I opened another issue about it: https://github.com/Grubuntu/PieMenu-Improved/issues/2

OS: macOS 12.6.7
Word size of FreeCAD: 64-bit
Version: 0.22.0dev.35485 (Git)
Build type: Release
Branch: main
Hash: a662fbb2ff267fb8dd56fcc3d1ec55a5968b7f5a
Python 3.10.13, Qt 5.15.8, Coin 4.0.2, Vtk 9.2.6, OCC 7.6.3
Locale: C/Default (C)
Installed mods:
  * PieMenu-Improved 1.2.7
  * SelectorToolbar
Grubuntu commented 7 months ago

I can't test under MacOS, so I'd like some help to find out where the problem is. Thank you for your contributions.

Grubuntu commented 7 months ago

https://github.com/Grubuntu/PieMenu-Improved/blob/198d2893364768d4ff27e2bce6f0bd2b8564a047/InitGui.py#L622C1-L628

see if the problem is not due to the timer?

pgilfernandez commented 7 months ago

Indeed! I'll be happy to test

pgilfernandez commented 7 months ago

https://github.com/Grubuntu/PieMenu-Improved/blob/198d2893364768d4ff27e2bce6f0bd2b8564a047/InitGui.py#L622C1-L628

see if the problem is not due to the timer?

How could I modify this code in order to test it?

Grubuntu commented 7 months ago

this the original code : ` class HoverButton(QtGui.QToolButton):

    def __init__(self, parent=None):
        super(HoverButton, self).__init__()

    def enterEvent(self, event):
        paramGet = App.ParamGet("User parameter:BaseApp/PieMenu")
        mode = paramGet.GetString("TriggerMode")
        if self.defaultAction().isEnabled() and mode == "Hover":
            PieMenuInstance.hide()
            self.defaultAction().trigger()
        else:
            pass

    def mouseReleaseEvent(self, event):
        paramGet = App.ParamGet("User parameter:BaseApp/PieMenu")
        mode = paramGet.GetString("TriggerMode")

        if self.defaultAction().isEnabled():
            PieMenuInstance.hide()
            self.defaultAction().trigger()
        else:
            pass

`

and this the the actual code : ' class HoverButton(QtGui.QToolButton):

    def __init__(self, parent=None):
        super(HoverButton, self).__init__()
        self.hoverTimer = QtCore.QTimer(self)
        self.hoverTimer.setSingleShot(True)
        self.hoverTimer.timeout.connect(self.onHoverTimeout)
        self.enterEventConnected = False
        self.isMouseOver = False 
        self.leaveEvent = self.onLeaveEvent

    def onHoverTimeout(self):
        mode = paramGet.GetString("TriggerMode")
        if self.isMouseOver and self.defaultAction().isEnabled() and mode == "Hover":
            PieMenuInstance.hide()
            self.defaultAction().trigger()
            try:
                docName = App.ActiveDocument.Name
                g = Gui.ActiveDocument.getInEdit()
                module = g.Module
            except:
                module = ''
            if (module is not None and module != 'SketcherGui'):
                PieMenuInstance.showAtMouse()
        else:
            pass

    def onLeaveEvent(self, event):
        self.isMouseOver = False 

    def enterEvent(self, event):
        if not self.enterEventConnected:
            self.hoverTimer.start(250) # timer to avoid too fast triggering at  hover
            self.enterEventConnected = True
        self.hoverTimer.stop()  
        self.hoverTimer.start(250) # timer to avoid too fast triggering at hover
        self.isMouseOver = True  

    def mouseReleaseEvent(self, event):
        mode = paramGet.GetString("TriggerMode")
        if self.isMouseOver and self.defaultAction().isEnabled():
            PieMenuInstance.hide()
            self.defaultAction().trigger()
            try:
                docName = App.ActiveDocument.Name
                g = Gui.ActiveDocument.getInEdit()
                module = g.Module
                if (module is not None and module != 'SketcherGui'):
                    PieMenuInstance.showAtMouse()
            except:
                pass
        else:
            pass

'

It seems that with the original code, it works correctly : https://forum.freecad.org/viewtopic.php?t=84101&start=10#p731573 I don't know what it is wrong for MacOs in these lines.

pgilfernandez commented 7 months ago

https://github.com/Grubuntu/PieMenu-Improved/blob/198d2893364768d4ff27e2bce6f0bd2b8564a047/InitGui.py#L622C1-L628

see if the problem is not due to the timer?

Making changes here doesn't seem to change anything...

I tried the original code with "hover" trigger mode and it works like a charm. The only thing that stops working are the fillet/chamfer fast radius".

Testing that "fillet/chamfer fast radius" with the actual code and with "hover" trigger mode, it works fine and after pressing either "accept" or "cancel" buttons make the pie to close as expected.

Could it be something related with the pie not closing (or being destroyed) after running a command?

Grubuntu commented 7 months ago 
  `  def onHoverTimeout(self):
        mode = paramGet.GetString("TriggerMode")
        if self.isMouseOver and self.defaultAction().isEnabled() and mode == "Hover":
            PieMenuInstance.hide()
            self.defaultAction().trigger()
            try:
                docName = App.ActiveDocument.Name
                g = Gui.ActiveDocument.getInEdit()
                module = g.Module
            except:
                module = ''
            if (module is not None and module != 'SketcherGui'):
                PieMenuInstance.showAtMouse()
        else:
            pass

`

try to replace module = '' by pass at line 613 https://github.com/Grubuntu/PieMenu-Improved/blob/198d2893364768d4ff27e2bce6f0bd2b8564a047/InitGui.py#L613

pgilfernandez commented 7 months ago

try to replace module = '' by pass at line 613

It kinda works:

Grubuntu commented 7 months ago

Ok thank you I made this commit to fix it :https://github.com/Grubuntu/PieMenu-Improved/commit/330aa21db1c4f8ea2034fdacc91b7d4d5b5252e2

I will have to improve this part of the code a little...

pgilfernandez commented 7 months ago

I tried removing that if conditional and now it works, that is, the whole function now is:

        def onHoverTimeout(self):
            mode = paramGet.GetString("TriggerMode")
            if self.isMouseOver and self.defaultAction().isEnabled() and mode == "Hover":
                PieMenuInstance.hide()
                self.defaultAction().trigger()
                try:
                    docName = App.ActiveDocument.Name
                    g = Gui.ActiveDocument.getInEdit()
                    module = g.Module
                except:
                    pass
                    # module = ''
                # if (module is not None and module != 'SketcherGui'):
                #     PieMenuInstance.showAtMouse()
            else:
                pass

It looks like it needs some time (delay) to trigger the command when you hover the icon but it works properly... may that delay be configured somewhere or could it be a new bug?

Thanks

pgilfernandez commented 7 months ago

It looks like it needs some time (delay) to trigger the command when you hover the icon but it works properly... may that delay be configured somewhere or could it be a new bug?

I continued testing and I think I'm right to say that it was now a too high delay, setting it from 250 to 25 made it works perfectly.

        def enterEvent(self, event):
            if not self.enterEventConnected:
                self.hoverTimer.start(25) # timer to avoid too fast triggering at  hover
                self.enterEventConnected = True
            self.hoverTimer.stop()  
            self.hoverTimer.start(25) # timer to avoid too fast triggering at hover
            self.isMouseOver = True

I don't know if setting this delay as default could make things work bad in other computers or OSs (in that case, another idea would be to expose the delay as a preference option so that users could set it up at their will).

Thanks!

pgilfernandez commented 7 months ago

Oh, further testing showed up that the "fast fillet/chamfer pie" doesn't work now...

Grubuntu commented 7 months ago

https://github.com/Grubuntu/PieMenu-Improved/blob/330aa21db1c4f8ea2034fdacc91b7d4d5b5252e2/InitGui.py#L625-L629

The delay of 250ms normally is to avoid triggering the command too quickly if you are (like me) not very precise with the mouse and you inadvertently hover over another command first. 150 to 300ms seemed correct to me, but perhaps it is too long. We could add a setting box for this parameter.

The fast Radius/Chamfer is dependent on the lines that you commented on it seems to me.

Try the new code I fixed: https://github.com/Grubuntu/PieMenu-Improved/commit/330aa21db1c4f8ea2034fdacc91b7d4d5b5252e2

pgilfernandez commented 7 months ago

The delay of 250ms normally is to avoid triggering the command too quickly if you are (like me) not very precise with the mouse and you inadvertently hover over another command first. 150 to 300ms seemed correct to me, but perhaps it is too long. We could add a setting box for this parameter.

Then it is a good idea to let users decide the delay that better works for them.

The fast Radius/Chamfer is dependent on the lines that you commented on it seems to me.

Try the new code I fixed: 330aa21

Perfect, now it works fine, thanks!

I would say you could close the issue if you merge it =)

Grubuntu commented 7 months ago

Thank you very much for all these tests.