Bug 393514 - DND feedback and a drag event with image cause repaint issues on Windows
Summary: DND feedback and a drag event with image cause repaint issues on Windows
Status: NEW
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy GEF (MVC) (show other bugs)
Version: 3.8   Edit
Hardware: PC Windows All
: P3 normal with 1 vote (vote)
Target Milestone: ---   Edit
Assignee: gef-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 393868
Blocks:
  Show dependency tree
 
Reported: 2012-11-04 23:34 EST by Peter Severin CLA
Modified: 2012-11-08 08:08 EST (History)
1 user (show)

See Also:


Attachments
Patch for GEF logic example that adds a drop listener for resources (3.06 KB, patch)
2012-11-04 23:34 EST, Peter Severin CLA
no flags Details | Diff
Pure SWT test case (5.94 KB, text/x-java)
2012-11-04 23:35 EST, Peter Severin CLA
no flags Details
Screenshot of repaint bug with logic example (2.53 KB, image/png)
2012-11-04 23:39 EST, Peter Severin CLA
no flags Details
Screenshot of repaint bug with SWT test case (8.61 KB, image/png)
2012-11-04 23:40 EST, Peter Severin CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Severin CLA 2012-11-04 23:34:16 EST
Created attachment 223155 [details]
Patch for GEF logic example that adds a drop listener for resources

I am trying to implement drag & drop of resource files from Project Explorer into a GEF editor. When the file is dragged over I want to show some feedback under the mouse cursor. This results in a bad repaint issue on Windows only. Specifically the area under the dragged image is not repainted correctly.

I created 2 test cases that show what happens. One is a patch for Logic example that adds a drop listener for resources. Dragging files from Project Explorer over a Circuit or Flow Container shows the repaint issue.

Second test case is pure SWT but which uses the same technique GEF does. Specifically, it calls Control#update method on drag over. This is what GEF essentially does by calling Viewer#flush() (see DeferredUpdateManager#repairDamage).

After looking through differences between platforms I narrowed my search to the following code in DropTarget class:

DropTarget#DragOver {
...
	notifyListeners(event.type, event);
	refresh();
...
}

The notify listeners method calls the drop listeners. But there is an additional call to refresh on Windows platform. Refresh method seems to do some additional repaint like this:

void refresh() {
	if (control == null || control.isDisposed()) return;
	int /*long*/ handle = control.handle;
	RECT lpRect = new RECT();
	if (OS.GetUpdateRect(handle, lpRect, false)) {
		OS.ImageList_DragShowNolock(false);
		OS.RedrawWindow(handle, lpRect, 0, OS.RDW_UPDATENOW | OS.RDW_INVALIDATE);
		OS.ImageList_DragShowNolock(true);
	}
}

I found another SWT bug Bug 218823 that looks similar but I am not sure this is the same issue.

It this cannot be fixed then please suggest some workarounds. Not using a drag image is out of the question as it's done in platform code.
Comment 1 Peter Severin CLA 2012-11-04 23:35:38 EST
Created attachment 223156 [details]
Pure SWT test case

The test case shows the differences between calling and not calling Control#update method in drag over event.
Comment 2 Peter Severin CLA 2012-11-04 23:39:54 EST
Created attachment 223157 [details]
Screenshot of repaint bug with logic example
Comment 3 Peter Severin CLA 2012-11-04 23:40:19 EST
Created attachment 223158 [details]
Screenshot of repaint bug with SWT test case
Comment 4 Peter Severin CLA 2012-11-08 02:52:45 EST
After looking more into it I could find a workaround for GEF. I am convinced that the underlying problem is an SWT bug but meantime here's how to fix it.

The main fix is to disable the call to Control#update() in NativeGraphicsSource#getGraphics(e) during drag & drop. I am not sure this call is needed at all. The way I did it is by copying NativeGraphicsSource and adding a flag "dndInProgress" and conditioning the call to Control#update() using this flag:

		public Graphics getGraphics(Rectangle r) {
			canvas.redraw(r.x, r.y, r.width, r.height, false);
			if (!dndInProgress)
				canvas.update();
			return null;
		}

Next I replaced the original graphics source by extending ScrollingGraphicalViewer and by hacking hookControl method like this:

	protected void hookControl() {
		super.hookControl();
		Control c = getControl();
		if ((c.getStyle() & SWT.DOUBLE_BUFFERED) != 0) {
			graphicsSource = new WindowsGraphicsSource(c);
			UpdateManager updateManager = getLightweightSystem()
					.getUpdateManager();
			updateManager.setGraphicsSource(graphicsSource);
		}
	}

In hookDropTarget I added an drop listener that positions the dndInProgress flag like this:

	protected void hookDropTarget() {
		super.hookDropTarget();

		getDropTarget().addDropListener(new DropTargetAdapter() {
			@Override
			public void dragEnter(DropTargetEvent event) {
				if (graphicsSource != null)
					graphicsSource.setDndInProgress(true);
			}

			@Override
			public void dragOperationChanged(DropTargetEvent event) {
				flush();
			}

			@Override
			public void dragLeave(DropTargetEvent event) {
				if (graphicsSource != null)
					graphicsSource.setDndInProgress(false);
			}
		});
	}

There is also a call to flush() method in dragOperationChanged method. Without this call there is still a redraw artifact although less visible than before. This might be another SWT issue. The call to flush() should have been actually done in GraphicalViewerImpl#hookDropTarget.

I hope this workaround will be helpful for others and also for fixing this issue.
Comment 5 Alexander Nyßen CLA 2012-11-08 07:44:02 EST
Peter, can you please file an SWT related bugzilla (containing your pure SWT test case), so the SWT team gets notified about the issue (and we can create a dependency on it)?
Comment 6 Peter Severin CLA 2012-11-08 08:04:55 EST
Alexander, I created Bug 393868 with a better analysis since I now have a better understanding of the problem.
Comment 7 Alexander Nyßen CLA 2012-11-08 08:08:14 EST
Added dependency to underlying SWT bug #393868