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

Command + Drag does not work (Apple) #2890

Open
tresf opened this issue Jul 2, 2016 · 95 comments · May be fixed by #7325
Open

Command + Drag does not work (Apple) #2890

tresf opened this issue Jul 2, 2016 · 95 comments · May be fixed by #7325
Assignees
Milestone

Comments

@tresf
Copy link
Member

tresf commented Jul 2, 2016

Edit: Thanks to @follower for the hint.... Here's the workaround:

[...] through random key mashing happened to make drag copy appear to work by pressing command (butterfly) and option (alt) simultaneously!

Edit: 2024 @tresf: It appears Ctrl can fix this on nightly versions, but it's very hard to time properly.


On Mac, the Ctrl shortcuts generally map to the Command button.

On current master branch, the Command + Drag should clone a Piano roll or BB Editor segment but does not.

Steps to reproduce:

  1. Using OSX 10.7 or higher, install latest master branch (mirror available here: @tresf/lmms-1.1.91-mac10.8.dmg)
  2. Create a piano roll segment
  3. Try to clone it using a modifier key
  4. No modifier keys work for cloning, instead a Right Click, Copy and Right Click, Paste is needed.
@follower
Copy link
Contributor

This also affects automation on OS X as it's not possible to drag a control knob etc onto the automation track.

In both cases the drag "ghost" image of the block/control appears but it doesn't have the green "+" copy symbol.

It works on RC1.

Tested on Mac OS X 10.8.x with the 1.1.91 DMG file from OP.

@tresf tresf added this to the 1.2.0 milestone Sep 15, 2016
@tresf
Copy link
Member Author

tresf commented Sep 15, 2016

Marking for 1.2 milestone.

This bug is really making usability of 1.2 difficult on Mac.

This may be a Qt5 problem. This works perfectly fine in the 1.1.3 installer which was packaged with Qt4.

@follower
Copy link
Contributor

When did the Qt5 change happen? FWIW it works fine with the 1.2.0-RC1 / lmms-1.1.90-mac10.7.dmg file.

@tresf
Copy link
Member Author

tresf commented Sep 15, 2016

When did the Qt5 change happen

Between RC1 and the upcoming RC2 per #2611

@tresf
Copy link
Member Author

tresf commented Oct 8, 2016

Moving to #2611. This is a blocker for Qt5 release. Help appreciated.

@tresf tresf closed this as completed Oct 8, 2016
@tresf
Copy link
Member Author

tresf commented Oct 10, 2016

Some more details for those that may be able to help...

untitled

AFAIK, the check happens here: src/core/Track.cpp#L657

if( m_trackView->trackContainerView()->allowRubberband() == true &&
        me->button() == Qt::LeftButton )
    {
        if( m_trackView->trackContainerView()->rubberBandActive() == true )
        {
            // Propagate to trackView for rubberbanding
            selectableObject::mousePressEvent( me );
        }
+       else if ( me->modifiers() & Qt::ControlModifier )
+       {
+           if( isSelected() == true )
+           {
+               m_action = CopySelection;
+           }
            else
            {
                m_action = ToggleSelected;
            }
        }
        else if( !me->modifiers() )
        {
            if( isSelected() == true )
            {
                m_action = MoveSelection;
            }
        }
    }

And according to the Qt 5.7 docs, this logic should still be valid...

"Note: On OS X, the ControlModifier value corresponds to the Command keys on the Macintosh keyboard, and the MetaModifier value corresponds to the Control keys. The KeypadModifier value will also be set when an arrow key is pressed as the arrow keys are considered part of the keypad."

But some people claim modifiers() can't be trusted. Is this an underlying Qt5 bug? Note, we use Qt 5.5 due to a bug with macdeployqt on 5.6 so if this is a bug with 5.5, we may between a rock and a hardplace in regards to moving from Qt4.

@BaraMGB
Copy link
Contributor

BaraMGB commented Oct 10, 2016

@tresf have you tried to code that hard? I've seen that somewhere in the code with that flower string. “⌘”

@tresf
Copy link
Member Author

tresf commented Oct 10, 2016

I've seen that somewhere in the code with that flower string.

Thanks for the hint however what you are referring to is most likely just a translated tool-tip.

@tresf tresf mentioned this issue Oct 10, 2016
16 tasks
@tresf
Copy link
Member Author

tresf commented Oct 10, 2016

untitled

Edit: Turns out the "bug" is with qDebug per https://bugreports.qt.io/browse/QTBUG-47316. The underlying data is fine. This can be observed by changing qDebug() << txt.toUtf8() to qDebug() << txt.toUtf8().data().

Back to the drawing board...


Ok... I'm fairly certain the bug stems from the StringPairDrag object constructor. For some reason with Qt5 it's escaping the newlines and double quotes.

According to qDebug()

Qt4:

"tco:0:<?xml version="1.0"?>
<!DOCTYPE>
..."

Qt5:

"tco:0:<?xml version=\"1.0\"?>\n<!DOCTYPE>
..."

As far as I understand, this will break the construction of a new TCO because it's contents will be unparsable, but if that's the case, why only on Mac?

Thoughts? Is there a flag or a QDomDocument QString or toUTF8() change I'm missing here?

@jasp00
Copy link
Member

jasp00 commented Oct 10, 2016

To help you, someone should work on Linux, add traces (fprintf(stderr,) to the drag and drop process, and make a new branch so that you can test and provide feedback.

@tresf
Copy link
Member Author

tresf commented Oct 11, 2016

untitled

The above video demonstrates what happens when replacing Qt::ControlModifier with Qt::ShiftModifier (and subsequently actually using the Shift key while dragging).

After a few hours of testing, I'm fairly certain Qt 5.5 is ignoring any mousePressEvent when ⌘ Command key is being pressed. All other shortcuts seem to be OK.

I tested this by placing a qDebug statement at the top of TrackContentObjectView::mousePressEvent(...).

This debug line shows with all modifiers except ⌘ Command. I think this is a bug with Qt5.5 and possibly others...

Components that are broken with this behavior:

  • Clone track using ⌘ Command + Drag handle
  • Linking Automation Editor using ⌘ Command + (any control within LMMS)
  • ⌘ Command + Drag-cloning patterns inside Song Editor, Beat/Bassline Editor

@teeberg
Copy link
Contributor

teeberg commented Oct 11, 2016

In Qt 5.6, at least that part seems to work for me. With

qDebug() << "TCOV::mousePressEvent" << me->modifiers();

as the first line in TrackContentObjectView::mousePressEvent and dragging while holding , I get

TCOV::mousePressEvent QFlagsQt::KeyboardModifiers(ControlModifier)

as expected. The pattern still doesn't copy though. Where I see it break down is around TrackContentWidget::dropEvent, which gets triggered in Qt 4.8.7, but not in 5.6.1.

I'm having trouble installing 5.5, so I'm afraid I can't reproduce what you're seeing. Do you have a 5.6 installation at hand to see whether that maybe fixes what you're seeing?

@tresf
Copy link
Member Author

tresf commented Oct 11, 2016

In Qt 5.6, at least that part seems to work for me. With

Sorry, it was a mistake on my part...

I've put debug statements all over Track.cpp and you're right, it IS firing on mousePressEvent. It's not firing on TrackContentWidget::dropEvent. That seems to be the place where the actual pasteSelection code gets fired.

If you blindly replace all instances of Qt::ControlModifier with Qt::ShiftModifier it all works with the shift key.

Side note... DON'T use Qt::MetaModifier because Mac translates Ctrl to toggle the Qt::RightButton and then all of the Qt::LeftButton logic fails. ⚱

@teeberg
Copy link
Contributor

teeberg commented Oct 13, 2016

I just recompiled with Qt 5.6.2 (released yesterday) and it works 🚀

image

lmmsqt5fix

@tresf
Copy link
Member Author

tresf commented Oct 13, 2016

If macdeployqt is still broken then we can't package with 5.6 due to QTBUG-53533 although I do consider this stellar news.

See also #2862.

Ugh...
https://bugreports.qt.io/browse/QTBUG-53533
https://bugreports.qt.io/browse/QTBUG-56814
https://bugreports.qt.io/browse/QTBUG-54086

@tresf
Copy link
Member Author

tresf commented Dec 4, 2016

@teeberg I've patched the installation process to use the 5.5 version of the macdeployqt for bundling, while still using the latest version for compiling here tresf@fc89997. This allows us to use Qt 5.6 or higher without suffering from #2862.

screen shot 2016-12-04 at 3 24 13 am

Unfortunately Option + Drag is still broken.

I'm starting to wonder if this issue is related to the build environment I'm using (10.9 for backwards compat reasons).

I'd happily use a newer build environment if we can keep backwards compatibility for older MacOS versions.

@tresf
Copy link
Member Author

tresf commented Dec 5, 2016

To put the "Why do we build on 10.8 anyway?" conversation into some context, I tried running a 10.11 build on 10.8 and this is what I get:

  1. Splash screen
  2. Crash while attempting to initialize the audio devices

https://gist.github.com/tresf/b52f70300970b6f733e9c173bf43fc84

Option+Drag is a crucial feature for LMMS. How crucial is having backward compatibility?

BTW, Removing SDL gets us a bit further but still crashes...

https://gist.github.com/tresf/b55e2d7510921f42c30bfaff8a65aa4c

The Command + Drag bug may be caused by this:

https://bugreports.qt.io/browse/QTBUG-48795

@follower
Copy link
Contributor

follower commented Jan 8, 2017

For the record, I'd be disappointed but understanding if LMMS chose not to support OS X 10.8 as that's what I'm currently on. (I suspect older hardware running older OS versions might be more common due to less compelling Mac hardware releases of late but could be wrong.)

By way of comparison, for similar FLOSS creative apps (but with presumably larger dev teams):

  • Blender supports Mac OS X 10.6+ (Snow Leopard onward)
  • Gimp supports Mac OS X 10.6+ (Snow Leopard onward)
  • Inkscape supported Mac OS X 10.7+ (and somewhat 10.5-10.6) for the immediate previous release but currently has no OS X binaries of the latest release.
  • Ardour supports Mac OS X x 10.5+ (Leopard onward)

If the decision is made not to support older versions the release notes need to be updated to indicate this--currently they still say OS X 10.7 is the minimum supported.

@michaelgregorius
Copy link
Contributor

@tresf, attached you can find a small test program that I have just put together. It builds a Qt6 application which shows a window that dumps many of its pointer interactions to the output.

EventTester.zip

You can extend the method inspectMouseEvent with any additional information needed. If you uncomment the line with setMouseTracking in the constructor you will get lots of additional output as soon as the pointer moves over the widget. It's likely overkill though.

What's interesting is that under Linux/Wayland my mouse is reported as a touchpad:

Mouse press event
Event time: QDateTime(2024-06-14 19:49:19.033 MESZ Qt::LocalTime)
Name: "touchpad"
Seat name: ""
Type: QInputDevice::DeviceType::TouchPad

@tresf
Copy link
Member Author

tresf commented Jun 14, 2024

Whoa, thanks! And it's a macOS bundle, far out!

What's interesting is that under Linux/Wayland my mouse is reported as a touchpad:

Mouse press event
Event time: QDateTime(2024-06-14 19:49:19.033 MESZ Qt::LocalTime)
Name: "touchpad"
Seat name: ""
Type: QInputDevice::DeviceType::TouchPad

Hilariously, my trackpad is reported as a mouse (as is my 2-button mouse):

Mouse release event
Event time: QDateTime(2024-06-14 15:20:53.387 EDT Qt::LocalTime)
Name: "core pointer"
Seat name: ""
Type: QInputDevice::DeviceType::Mouse

Sadly, nothing stands out as being different. My theory is because the problem is not actually with Command key nor with the drag start, but rather with the Command button inadvertently cancelling the drop/release action on the target.

Of course, I haven't actually tested this recently with LMMS+Qt6 so the behavior may vary. I'll test a quick build with Qt6 so as not to spin our wheels to much.

@tresf
Copy link
Member Author

tresf commented Jun 14, 2024

Ok, Qt6 is interesting! The bug is now reprocible with a two-button mouse, so this problem will get worse with the update to Qt6.

The symptoms are identical, unless Command is release, the drop will not work.

@michaelgregorius
Copy link
Contributor

This is just a shot in the dark, but I wonder if the problem is perhaps related to the resetting of modifiers in the destructor of the StringPairDrag class:

getGUI()->mainWindow()->clearKeyModifiers();

If the modifiers somehow get reset while the item is still being dragged then this might lead to a change of mode.

Can you try how it behaves if you comment out the code, @tresf?

@michaelgregorius
Copy link
Contributor

Another idea would be to change the last line with the exec call in the constructor of StringPairDrag as follows:

qDebug() << "Executing drag";
Qt::DropAction dropAction = exec( Qt::LinkAction, Qt::LinkAction );
qDebug() << dropAction;

In the successful case this should output the following:

Executing drag
Qt::LinkAction

If the drag is cancelled, e.g. via the escape key, then the output is the following:

Executing drag
Qt::IgnoreAction

I guess in the buggy case you will get the latter output.

Having the exec call in the constructor of StringDragPair makes the code look very confusing by the way. There's an allocation in the middle of the code which looks like a memory leak (it isn't because it passes a parent for the underlying QDrag class) and then nothing is done with it:

new StringPairDrag( QString( "clip_%1" ).arg(

IMO it would be cleaner to have something like a static method StringDragPair::executeStringDrag with the same parameters as the constructor which would then create the drag object and execute it. This method then could also return the Qt::DropAction to report the result of the drag. I don't expect constructors to execute potentially long running actions with event loop character.

@michaelgregorius
Copy link
Contributor

@tresf, I saw your comment from October 11th, 2016. I guess if you set a break point in TrackContentWidget::dropEvent the code is still not making it there?

If that's the case then I suspect that it really is a problem with Qt and MacOS. Once the code enters the exec in the constructor of StringPairDrag the Qt framework code should take over and call the drag & drop related event methods on all the widgets that are visited by the pointer.

The documentation states that the exec call is not blocking on Linux and MacOS though (see here). So it would be interesting to see if some key events and modifiers are still processed while dragging the pattern and if these lead to cancelled drag actions. It will also be interesting to see if there is only one "Executing drag" output on MacOS.

Last but not least, can you please set a break point in TrackContentWidget::dragEnterEvent and see if that gets hit as soon as you start to drag the pattern? It should be one of the first methods that's executed by the Qt framework when a drag action takes place.

@tresf
Copy link
Member Author

tresf commented Jun 15, 2024

qDebug() << "Executing drag";
Qt::DropAction dropAction = exec( Qt::LinkAction, Qt::LinkAction );
qDebug() << dropAction;

@michaelgregorius yes! It fires IgnoreAction when this issue occurs. It fires LinkActions when it succeeds.

@michaelgregorius
Copy link
Contributor

Then it would be interesting to find out what's happening during the exec call because it is non-blocking on MacOS:

  1. If you set a break point in TrackContentWidget::dragEnterEvent does it get hit as soon as you start to drag the pattern? Does it get hit if the mouse is over the TrackContentWidget?
  2. If you add debug output to all relevant mouse events in ClipView, e.g. the mouse events, do you get output between the "Executing drag" and "Qt::IgnoreAction" output?

@michaelgregorius
Copy link
Contributor

The MacOS implementation for Qt 5.15 can be found here. I did not see any key handling though. Would be interesting to know how applications that are not written in Qt behave with regards to drag & drop and the command key.

@tresf
Copy link
Member Author

tresf commented Jun 16, 2024

I'm unaware of a program that uses Command + Drag to copy. Garage Band uses Option + Drag.

@tresf
Copy link
Member Author

tresf commented Jun 16, 2024

I did not see any key handling though.

Key handling is here. It confirms that copy should be using Alt|Option instead of ⌘ Command, although we use the LinkAction, would expect this to work.

https://github.com/qt/qtbase/blob/86c62c8f6088ec148512457cb7e964661ba643b0/src/plugins/platforms/cocoa/qcocoadrag.mm#L98-L103

@michaelgregorius
Copy link
Contributor

michaelgregorius commented Jun 16, 2024

Why is Qt::LinkAction used in the first place (documentation)? If I understand correctly, technically it is a copy, isn't it? If I adjust the original the clone is not adjusted or is it?

@tresf
Copy link
Member Author

tresf commented Jun 16, 2024

Why is Qt::LinkAction used in the first place (documentation)? If I understand correctly, technically it is a copy, isn't it? If I adjust the original the clone is not adjusted or is it?

My assumption is that it uses Link historically because the behavior was originally copied from the Automation drag/drop. Unfortunately this issue also occurs when trying to link knobs to automation segments -- where we'd probably want Qt::LinkAction instead of Qt::CopyAction, but no combination of supportedActions seem to fix this behavior.

@tresf
Copy link
Member Author

tresf commented Jun 16, 2024

A stop-gap may be to fix this on Mac to match other applications -- e.g. switch to Qt::AltModifier -- which maps to the Option key on the Mac keyboard... but this only masks the issue... If we're to want Command as a modifier key, this bug bars us from ever using it.

@Veratil
Copy link
Contributor

Veratil commented Jun 16, 2024

Why is Qt::LinkAction used in the first place (documentation)? If I understand correctly, technically it is a copy, isn't it? If I adjust the original the clone is not adjusted or is it?

Copy:

A -> B
B is a copy of A
Any changes to B aren't reflected in A
Any changes to A aren't reflected in B

Link:

A -> B
B is linked to A
Any changes to B are reflected in A (because editing B actually edits A)
Any changes to A are reflected in B (because B is just a pointer to A)

@tresf
Copy link
Member Author

tresf commented Jun 16, 2024

@Veratil, right, but in Automation editor and BBEditor, it's effectively a link, so the UI is using the the right indicator there. With Piano Roll segments, the indicator should be copy, not link.

@tresf
Copy link
Member Author

tresf commented Jun 16, 2024

A stop-gap may be to fix this on Mac to match other applications -- e.g. switch to Qt::AltModifier -- which maps to the Option key on the Mac keyboard... but this only masks the issue... If we're to want Command as a modifier key, this bug bars us from ever using it.

Furthemore, this makes a lot of #ifdef LMMS_BUILD_MACOS guards all over the codebase.

@tresf
Copy link
Member Author

tresf commented Jun 16, 2024

@michaelgregorius would you be willing to enhance the demo you created to allow us to test this out without the LMMS cruft? For example, if we can prove that drag-and-drop works normally with Command we might be able to narrow down why it won't work in LMMS.

@michaelgregorius
Copy link
Contributor

@tresf, here's a new version of the event tester. EventTester.zip

Ironically my file browser froze the first time I dragged and dropped the zip file onto the text area of the browser. 😅

Here's a quick video that demonstrates how to use it:

Bildschirmaufnahme_20240616_185901.webm

The "features" are:

  • You can select the allowed actions that should be used for the drag with the check boxes.
  • The default action is selected with the radio buttons.
  • You can then drag the label (in fact any area of the widget) to the big window that accepts drops and prints some information about them.

That way you should be able to test most of the relevant scenarios.

Interestingly, I cannot add additional keys once the drag is started. So if I press the "Alt" key exclusively when starting the drag and then later add the "Control" key as well when dropping on the target widget the event still only contains the "Alt" key.

Please note that this is still for Qt 6.

@tresf
Copy link
Member Author

tresf commented Jun 17, 2024

@michaelgregorius thank you! First, I have to say that the behavior with EventTester matches that of LMMS. The Command + Drag is broken in an identical way.

Some oddities though:

  • The only modifier key that works reliably with "Link" on MacOS is Control
  • The only modifier key that works reliably with "Copy" on MacOS is Option
  • No modifiers work properly with Command

This seems to suggest that Command is causing the issue and that the behavior appears to be a bug with the QDrag implementation.

What wasn't obvious to me in the LMMS source code is that Default action is for items that don't necessarily need a shortcut key to being dragging. This is why the Command behavior is so confusing, we force the user to press it, but Qt prohibits it from being pressed.

Ideally, we would be able to override this behavior, as it prevents us from ever using the Command key for a drag-and-drop event.

The workaround would be to rearrange these shortcuts to play nicer with the default Qt behavior.

@tresf
Copy link
Member Author

tresf commented Jun 17, 2024

Ok, another oddity...

  • Qt6 permits the Command key if it's combined with Control (which maps to MetaModifier).
  • Qt5 permits the Command key if it's combined with Option (with maps to the AltModifer).

Of course, any Control presses are problematic on Mac if there's a context menu, because, reasons.

So, for the time being, Option + Command is a viable workaround for Qt5 builds.
Scratch that, I was accidentally using a two-button mouse, so my test results were invalid. :(

@tresf tresf linked a pull request Jun 17, 2024 that will close this issue
@michaelgregorius
Copy link
Contributor

What wasn't obvious to me in the LMMS source code is that Default action is for items that don't necessarily need a shortcut key to being dragging. This is why the Command behavior is so confusing, we force the user to press it, but Qt prohibits it from being pressed.

Yes, that confuses me as well. The API seems to indicate that the developers have to find out which default action to use, e.g. by using key modifiers. And then the implementation performs some checks if the developers did right. 😅

I'm starting to wonder if the intention of the API is to simply let users start dragging without evaluating the modifiers and the system then determines during the drag what's the current action to be taken. This would likely be done to ensure system consistency.

The workaround would be to rearrange these shortcuts to play nicer with the default Qt behavior.

I think for that the main implementations (Windows, Linux, MacOS) would have to be compared so that we do not run into similar problems on other platforms.

@michaelgregorius
Copy link
Contributor

@tresf, here you can find an extended version of the event tester: EventTester.zip

It has the following new functionality:

  • It prints the result of the exec call, i.e. what kind of action was taken.
  • It provides an additional check box which determines which variant of the exec call is used. If it is unchecked then the variant of exec that only takes the allowed actions is used. Otherwise the default action is provided as well.

The default action indeed seems to describe what action is to be executed if no modifier key is pressed. If the default action is not given then the system will determine the action to be taken by evaluating the set of allowed actions and the keys that are pressed before the drag was started.

In my case the mapping is as follows if all three allowed actions are checked and "Evaluate default action" is unchecked:

  • Shift -> Move
  • Control -> Copy
  • Alt -> Link

If the default is not given then a drag without a key modifier results in a "Move" action.

Also if the set of allowed actions is not the full set then the keys map by some other rules.

It is really a very complex set of rules, e.g. try experimenting with disallowing all actions and evaluating the default action. Then do just a drag with no modifier keys. In my case it is always a "Copy" action regardless of the default action. Allow just one action and it will always be the action regardless of the selected default action.

@tresf
Copy link
Member Author

tresf commented Jun 17, 2024

The API seems to indicate that the developers have to find out which default action to use, e.g. by using key modifiers. And then the implementation performs some checks if the developers did right. 😅

My exact thoughts.

I think for that the main implementations (Windows, Linux, MacOS) would have to be compared so that we do not run into similar problems on other platforms.

This is what puzzles me the most. It's as if we've been barely dodging this issue elsewhere, or Qt hilariously cares less about consistency on non-Apple OSs. The later would make sense given Apple's track record, as they do curate applications for their platform and human interface guidelines are a major part of this, but it's still bewildering that they're no one else complaining about this. I assume other applications choose to implement their own QDrag equivalents, or they're writing new enough code to not attempt to use keys that aren't allowed.

I've opened up #7325 for discussion on this matter. Initial testing is pretty positive. As it turns out, most code that allows fine-tuned dragging (e.g. via AltModifier was already coded to allow ControlModifier.

What's perhaps more concerning is the long-term effects of placing #ifdefs everywhere. I'd love a proposed solution that's more centralized so that we can slowly refactor all shortcut code to a central, manageable location. e.g. #1488

@tresf
Copy link
Member Author

tresf commented Jun 17, 2024

Allow just one action and it will always be the action regardless of the selected default action.

I noticed this nonsense too. IMO this is broken behavior.

@michaelgregorius
Copy link
Contributor

I think it would also be beneficial to get a better understanding of Qt's concepts with regards to the source and the target of a drag and their responsibilities.

The source of the drag determines the (mime) type of the data that is transported via the drag and drop. What I find interesting is that it also has a say with regards to the actions that are allowed for the target, i.e. how the data is interpreted by the target.

The target of a drag on the other hand can evaluate the data that's transported by the drag and can then also inspect the drop action which is partly determined by Qt's code. This can be done in dragEnterEvent, e.g. by inspecting the value of proposedAction which seems to be the value as determined by Qt. However, there's also QDropEvent::dropAction and QDropEvent::setDropAction which seems to let the target communicate the action that it would perform with the given data. The event tester does not really implement something like this because it accepts all drags by calling event->acceptProposedAction().

IMO the solution could be to not add more ifdefs and complexity but to try to remove it and to just go with the flow of Qt and with that perhaps the conventions of the underlying platform.

I am not really sure if the dragEnterEvents are implemented in LMMS with the remarks and possibilities noted above in mind. In the end each drag target in LMMS should be able to inspect the data of the drag and the intended action. It should then determine if it can perform the action on itself or not, e.g. "Can I copy the data of the drag into me?", "Can I link the data of the drag to me?", etc.

It would also be interesting to know the concepts behind "Copy", "Move" and "Link" in Qt because they are not really explained, at least in the documentation of the corresponding action enums. Does "Move" for example imply that the data is moved from the source to the target in that is destroyed or not available anymore in the source after a successful action?

@tresf
Copy link
Member Author

tresf commented Jun 17, 2024

IMO the solution could be to not add more ifdefs and complexity but to try to remove it and to just go with the flow of Qt and with that perhaps the conventions of the underlying platform.

I like this proposal, however this doesn't solve the originating problem, and that's the we have other mouse listeners on these same widgets. For example, knobs can be dragged to adjust values WITHOUT QDrag. Patterns can be dragged to adjust position or length WITHOUT QDrag.

It could be argued that move should use QDrag, but I'm not sure it's going to behave as intended. This means that we don't have much choice but to listen for keypress to filter out non-QDrag events. Perhaps to put this simpler, if EventTester.zip had a default action of changing the color as you drag your mouse away, but still honoring the Copy behavior when using the modifier key, this would replicate -- in part -- how we use manual mouse listeners in combination with QDrag today.

@tresf
Copy link
Member Author

tresf commented Jun 17, 2024

In the end each drag target in LMMS should be able to inspect the data of the drag and the intended action

We can and do inspect the data. My understanding of the problem is that the default QDrop behavior makes decisions based on keypress, breaking the exec() contract, which is super sad. It may be a "blessing in disguise" though if it helps standardizing our inputs to match other application behavior.

@tresf
Copy link
Member Author

tresf commented Jun 17, 2024

Does "Move" for example imply that the data is moved from the source to the target in that is destroyed or not available anymore in the source after a successful action?

I've thought about this as well. From my understanding it expects the implementation to be done by the application, not the framework (which is why this exec() behavior is so overbearing of a framework decision).

From my understanding, the various actions are for filtering intent as well as providing a UI indicator of intent only and that all other implementation details are to be handled by the application, quoting:

QDrag only deals with the drag and drop operation itself. It is up to the developer to decide when a drag operation begins, and how a QDrag object should be constructed and used. For a given widget, it is often necessary to reimplement mousePressEvent() to determine whether the user has pressed a mouse button, and reimplement mouseMoveEvent() to check whether a QDrag is required.

I do like the idea of using it for all drag actions, if it's possible. 🤞

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

Successfully merging a pull request may close this issue.

7 participants