Summary: | [GTK3][DnD] Dragging parts does not show rectangle | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Leo Ufimtsev <lufimtse> | ||||||||
Component: | SWT | Assignee: | Leo Ufimtsev <lufimtse> | ||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||
Severity: | major | ||||||||||
Priority: | P2 | CC: | akurtakov, alhashash, ericwill, gautier.desaintmartinlacaze, ipun, Lars.Vogel, marc.khouzam, peter, sit1way, sravankumarl, sxenos | ||||||||
Version: | 4.8 | Keywords: | helpwanted, triaged | ||||||||
Target Milestone: | 4.8 RC3 | Flags: | akurtakov:
review+
sravankumarl: review+ ericwill: review+ |
||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=534509 https://bugs.eclipse.org/bugs/show_bug.cgi?id=369821 https://bugs.eclipse.org/bugs/show_bug.cgi?id=529431 https://bugs.eclipse.org/bugs/show_bug.cgi?id=535083 https://git.eclipse.org/r/123290 https://git.eclipse.org/r/123370 https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=34be01e01753fd8ff9109cdad0dad9052e65dd4f https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=f387b612b083bd9768b6782dac9356762e8ed1ef |
||||||||||
Whiteboard: | RHT | ||||||||||
Bug Depends on: | 509657 | ||||||||||
Bug Blocks: | 492861 | ||||||||||
Attachments: |
|
Description
Leo Ufimtsev
2016-07-20 12:06:08 EDT
Created attachment 263215 [details]
Video of issue.
Tracker doesn't seem to get painted on GTK3. Or sometimes it appears when you let go of the mouse, not while you drag things around.
I need to investigate if it's related to my DND patch: 428852: [GTK3] SWT - DND broken https://bugs.eclipse.org/bugs/show_bug.cgi?id=428852 During DND, GTK didn't emit motion events, maybe lack of motion events is causing tracker to mailfunction also... Assigning to self for now. Might be related to: 492783: [GTK2] Tracker doesn't work properly when using the constructor that accepts a Display https://bugs.eclipse.org/bugs/show_bug.cgi?id=492783 Not related to bug 492783. The drag affordance used for views in the E4 workbench is actually a transparent ON_TOP shell, not a Tracker. A Tracker is present but it is hidden offscreen and you're not supposed to see it. Also, notice that your drop cursor is changing to an X in addition to the missing drag rectangles. This looks to me like a bug in the workbench, not SWT. (In reply to Stefan Xenos from comment #5) > Also, notice that your drop cursor is changing to an X in addition to the > missing drag rectangles. > > This looks to me like a bug in the workbench, not SWT. Thank you for feedback. Hmm, I sort of wonder why it only occurs on Gtk3 though..? > Hmm, I sort of wonder why it only occurs on Gtk3 though..? Yes, that does normally point to an SWT bug, which may be evidence against my hypothesis from comment 4. It occurs to me that we've been having many problems with Tracker-based drag & drop over the years and we could easily make this work using an event filter (like I did in the patch for the "select control" button in bug 498030). The only downside of the event filter is that we'd lose control over the custom cursor... but that may be a small price to pay in exchange for getting reliable and bug-free drag & drop on all platforms. Maybe we could implement both and use a hidden preference to flip between the two. Then whenever folks report Drag & Drop problems, we could ask them to toggle the preference. If flipping the preference makes a difference, we'd know that the Tracker was to blame. Another possible cause: when I was working on bug 498030 I had to add a special case to my target detection in order to prevent my own ON_TOP shell from being considered as a possible drop target. I can't find any similar special case in the workbench code, so it's possible that the drag rectangle itself is blocking possible drop targets. (In reply to comment #7) > > Hmm, I sort of wonder why it only occurs on Gtk3 though..? > > Yes, that does normally point to an SWT bug, which may be evidence against my > hypothesis from comment 4. > > It occurs to me that we've been having many problems with Tracker-based drag & > drop over the years and we could easily make this work using an event filter > (like I did in the patch for the "select control" button in bug 498030). The > only downside of the event filter is that we'd lose control over the custom > cursor... but that may be a small price to pay in exchange for getting reliable > and bug-free drag & drop on all platforms. > > Maybe we could implement both and use a hidden preference to flip between the > two. Then whenever folks report Drag & Drop problems, we could ask them to > toggle the preference. If flipping the preference makes a difference, we'd know > that the Tracker was to blame. > > Another possible cause: when I was working on bug 498030 I had to add a special > case to my target detection in order to prevent my own ON_TOP shell from being > considered as a possible drop target. I can't find any similar special case in > the workbench code, so it's possible that the drag rectangle itself is blocking > possible drop targets. I see. So the suggestion is to implement an option that would switch to using the tracker instead of an invisible shell? Just before we go down that road, do you happen to know if there is a (swt/jface) snippet that reproduces the behaviour via invisible shell? (or one that could potentially be customized to do so..) Maybe it's just a matter of the boarder not being drawn properly or some CSS property that needs to be applied on GTK side. > So the suggestion is to implement an option that would switch to using > the tracker instead of an invisible shell? No. Both would use an invisible Shell. One option would use a Tracker to capture mouse events and set the cursor and the other would use an event filter attached to the Display (with no Tracker). > Just before we go down that road, do you happen to know if there is a > (swt/jface) snippet that reproduces the behaviour via invisible shell? > (or one that could potentially be customized to do so..) If you suspect the low-level code that draws drag-over affordances, check out the layout spy dialog from the lastest code in master in the PDE repository. It's bound to ctrl-alt-shift-F9. Click the "select control" button and you'll get the same sort of semitransparent shell, but it uses an event filter rather than a Tracker for capturing mouse movement. The code for that bit is in ControlSelector.java. I suspect the problem may be related to bug 492861. While messing with that code, I've discovered a bunch of bugs related to leaking state during drag and drop... and it seems plausible that some of that state may have broken the dragover affordances in some cases. I'm working on a rewrite that is purely functional (I've deleted all the member variables from the drop targets), which makes it impossible for any side-effects to occur. (In reply to Stefan Xenos from comment #9) > > So the suggestion is to implement an option that would switch to using > > the tracker instead of an invisible shell? > > No. Both would use an invisible Shell. One option would use a Tracker to > capture mouse events and set the cursor and the other would use an event > filter attached to the Display (with no Tracker). > > > > Just before we go down that road, do you happen to know if there is a > > (swt/jface) snippet that reproduces the behaviour via invisible shell? > > (or one that could potentially be customized to do so..) > > If you suspect the low-level code that draws drag-over affordances, check > out the layout spy dialog from the lastest code in master in the PDE > repository. It's bound to ctrl-alt-shift-F9. > > Click the "select control" button and you'll get the same sort of > semitransparent shell, but it uses an event filter rather than a Tracker for > capturing mouse movement. The code for that bit is in ControlSelector.java. I tested ControlSelector, it works well. The yellow tracker wraps niceley around all controls. But the tracker for parts doesn't work properly in the same eclipse instance. I tested with various GTK versions. I found that up to and including gtk3.8, the part-dragging works well. In Gtk3.10 and onwards it broke. I suspect some underlying gtk caching might be the culprit. I will investigate further. (In reply to Stefan Xenos from comment #4) > Not related to bug 492783. > > The drag affordance used for views in the E4 workbench is actually a > transparent ON_TOP shell, not a Tracker. > > A Tracker is present but it is hidden offscreen and you're not supposed to > see it. I can confirm this. The part-dragging still works in Gtk3.6, Gtk3.8, but breaks on Gtk3.10. But the tracker doesn't work on Gtk3.6, Gtk3.8 or any further Gtk3.* version. I will try to bisect gtk's codebase between gtk3.8 and gtk3.10 to find which gtk commit broke the part-dragging. Leo, although I have nothing wrong with fixing the Tracker class, I'm not sure that using a Tracker is really the best solution here. AFAIK, the only reason the workbench uses a Tracker is to get exclusive control over the mouse cursor in a way that won't be overridden when you move the cursor over controls that use a custom cursor. This is kind-of an abuse of the Tracker class to begin with since the workbench doesn't actually want the tracker rectangle and intentionally moves it offscreen to make it invisible. IMO, a more elegant solution for Drag & Drop would be to introduce new API into SWT that provides direct control over the mouse cursor and to do away with the Tracker entirely. (In reply to Stefan Xenos from comment #12) > Leo, although I have nothing wrong with fixing the Tracker class, I'm not > sure that using a Tracker is really the best solution here. > > AFAIK, the only reason the workbench uses a Tracker is to get exclusive > control over the mouse cursor in a way that won't be overridden when you > move the cursor over controls that use a custom cursor. > > This is kind-of an abuse of the Tracker class to begin with since the > workbench doesn't actually want the tracker rectangle and intentionally > moves it offscreen to make it invisible. > > IMO, a more elegant solution for Drag & Drop would be to introduce new API > into SWT that provides direct control over the mouse cursor and to do away > with the Tracker entirely. Thank you for your reply. Interesting proposition. I've never looked into platform.ui deeply, I don't have an understanding of how part-dragging works beyond the SWT layer. If you think new api is better, maybe we should investigate further down that road. I don't have a good understanding of what you mean by 'direct control over the mouse cursor', can you elaborate on what functions the api should provide, what input/output the functions would have? > can you elaborate on what functions the api should provide, what
> input/output the functions would have?
I'm referring specifically to the Tracker.setCursor(Cursor) method. It sets the mouse cursor in a way that is different from all other setCursor methods, since it continues to have an effect no matter where you move the mouse on screen.
One possible alternative might be to create a Display.overrideCursor(Cursor) method which either accepts a cursor or null. Passing it a Cursor causes it to use that cursor everywhere on screen, and passing it null returns it to the default mode of getting the cursor appearance from the control below the mouse.
As I'm writing this, I've thought of a third option for fixing drag & drop. It could continue to use the Tracker for its setCursor method and nothing else, but switch from using the Tracker's event loop to using a global event filter like ControlSelector does. From your tests, above, it seems as though the event filter approach should work fine... and by ignoring any events fired from the Tracker, the workbench would no longer be susceptible to flakey Tracker behaviors.
(In reply to comment #14) > > can you elaborate on what functions the api should provide, what > > input/output the functions would have? > > I'm referring specifically to the Tracker.setCursor(Cursor) method. It sets the > mouse cursor in a way that is different from all other setCursor methods, since > it continues to have an effect no matter where you move the mouse on screen. > > One possible alternative might be to create a Display.overrideCursor(Cursor) > method which either accepts a cursor or null. Passing it a Cursor causes it to > use that cursor everywhere on screen, and passing it null returns it to the > default mode of getting the cursor appearance from the control below the mouse. > > As I'm writing this, I've thought of a third option for fixing drag & drop. It > could continue to use the Tracker for its setCursor method and nothing else, but > switch from using the Tracker's event loop to using a global event filter like > ControlSelector does. From your tests, above, it seems as though the event > filter approach should work fine... and by ignoring any events fired from the > Tracker, the workbench would no longer be susceptible to flakey Tracker > behaviors. Hmmm. Interesting. I'll do some reading on Tracker.setCursor(Cursor) . In the mean time I found the GTK commit that broke part-dragging: http://osdir.com/ml/commits.gnome/2013-05/msg04138.html In the mean time I will investigate if we can fix this on the gtk side of things. (In reply to Leo Ufimtsev from comment #15) > Hmmm. Interesting. I'll do some reading on Tracker.setCursor(Cursor) . > In the mean time I found the GTK commit that broke part-dragging: > http://osdir.com/ml/commits.gnome/2013-05/msg04138.html > In the mean time I will investigate if we can fix this on the gtk side of > things. I'm not having much luck with resolving this on the gtk side at the moment. The offending GTK+ commit is: a60ccd3672467efb454b121993febc36f33cbc79 (#a60ccd36) If I checkout maybe 100 commits after #a60ccd36, then revert a60ccd36, then Drag-part still works. However, if I checkout master, attempting to revert causes many merge conflicts. I tried to revert by manually editing code, but drag-part issue still persists with that approach. I will either need to reproduce this on a snippet level to peruse a gtk solution further, or we should alter the part-drag mechanism as per your suggestion. Or go implement rectangle drawing similar to how "layout spy dialog" does it at the moment, because that seems to work. Hmm... It seems either way fixing this would take a while. It might be a while before I'm able commit a big chunk of time to this. (Have to finish webkit2 port first, then stabilise Eclipse on Wayland, could work on this after). If someone wants to work on it in the meantime, feel free to do so. Assigning to platform-swt for now. I've been meaning to work on this for a while, but webkit2 port is taking some more time than expected. I'd be really great to have this fixed in near future... *** Bug 436211 has been marked as a duplicate of this bug. *** Created attachment 274052 [details]
gtk commit that broke things.
Found the gtk commit that caused this breakage.
It's a change in transparent shell handling.
Gtk implemented an optimization where transparent shells no longer have their clip tracked anymore. Since the partTracker is a transparent shape, things broke for us.
Will investigate solution.
Created attachment 274054 [details] Snippet to reproduce tracker issue. org.eclipse.e4.ui.workbench.addons.dndaddon.(DnDManager) has an offscreen Tracker: > private static final Rectangle offScreenRect = new Rectangle(10000, -10000, 1, 1); As of one of Gtk3.9.1's commits: a60ccd3672467efb454b121993febc36f33cbc79, clipping for off-screen elements is not computed anymore for optimization purposes. This results in off-screen widgets not emitting events. However, DnDManager relies on the off-Screen Tracker to send MouseMove events as trigger for it's custom drawing. So the result is that due to the offScreen tracker no longer sending mouseMove events, custom drawnings are no longer done. I fiddled around with it, if the tracker is put on Screen, then everything works again. This issue can be observed in the attached snippet. I will look for a solution. New Gerrit change created: https://git.eclipse.org/r/123290 New Gerrit change created: https://git.eclipse.org/r/123370 Gerrit change https://git.eclipse.org/r/123290 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=34be01e01753fd8ff9109cdad0dad9052e65dd4f Gerrit change https://git.eclipse.org/r/123370 was merged to [R4_8_maintenance]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=f387b612b083bd9768b6782dac9356762e8ed1ef Awesome! Thanks for the fix. Verified in Eclipse SDK Version: Photon (4.8) Build id: I20180529-0600 OS: Linux, v.4.15.0-22-generic, x86_64 / gtk 3.22.30 Java version: 1.8.0_161 Thanks a bunch, I was really missing this drag indicator. (In reply to Lars Vogel from comment #28) > Verified in Eclipse SDK > Version: Photon (4.8) > Build id: I20180529-0600 > OS: Linux, v.4.15.0-22-generic, x86_64 / gtk 3.22.30 > Java version: 1.8.0_161 > > > Thanks a bunch, I was really missing this drag indicator. Thanks for verification. Yea, it's been bugging me for 4 years also =). *** Bug 414119 has been marked as a duplicate of this bug. *** |