Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 333375

Summary: Inconsistent handling of Tree client area when scrolling under MacOSX Cocoa
Product: [Eclipse Project] Platform Reporter: Alexander Nyßen <nyssen>
Component: SWTAssignee: Platform-SWT-Inbox <platform-swt-inbox>
Status: RESOLVED WORKSFORME QA Contact:
Severity: normal    
Priority: P3 CC: markus.tiede, peter, Silenio_Quarti, skovatch
Version: 3.6.1   
Target Milestone: ---   
Hardware: PC   
OS: Mac OS X   
Whiteboard:
Bug Depends on:    
Bug Blocks: 332956, 333805    

Description Alexander Nyßen CLA 2011-01-01 10:40:03 EST 
Using the following snippet, one can reproduce (by dragging the first tree item downwards to that the tree starts scrolling) that on Windows XP the client area's y coordinate remains unchanged during the scroll operation, while on MacOSX Cocoa, the y coordinate is indeed increased.

import org.eclipse.swt.SWT;
import org.eclipse.swt.dnd.DND;
import org.eclipse.swt.dnd.DragSource;
import org.eclipse.swt.dnd.DragSourceEvent;
import org.eclipse.swt.dnd.DragSourceListener;
import org.eclipse.swt.dnd.DropTarget;
import org.eclipse.swt.dnd.DropTargetEvent;
import org.eclipse.swt.dnd.DropTargetListener;
import org.eclipse.swt.dnd.TextTransfer;
import org.eclipse.swt.dnd.Transfer;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Tree;
import org.eclipse.swt.widgets.TreeItem;

public class Bug332956 {

	public static void main(String[] args) {
		Display display = new Display();
		Shell shell = new Shell(display);
		shell.setLayout(new FillLayout());
		final Tree tree = new Tree(shell, SWT.MULTI | SWT.H_SCROLL
				| SWT.V_SCROLL);
		for (int i = 0; i < 20; i++) {
			TreeItem iItem = new TreeItem(tree, 0);
			iItem.setText("TreeItem (0) -" + i);
			for (int j = 0; j < 4; j++) {
				TreeItem jItem = new TreeItem(iItem, 0);
				jItem.setText("TreeItem (1) -" + j);
				for (int k = 0; k < 4; k++) {
					TreeItem kItem = new TreeItem(jItem, 0);
					kItem.setText("TreeItem (2) -" + k);
					for (int l = 0; l < 4; l++) {
						TreeItem lItem = new TreeItem(kItem, 0);
						lItem.setText("TreeItem (3) -" + l);
					}
				}
			}
		}
		DragSource ds = new DragSource(tree, DND.DROP_COPY | DND.DROP_MOVE
				| DND.DROP_LINK);
		ds.setTransfer(new Transfer[] { TextTransfer.getInstance() });
		ds.addDragListener(new DragSourceListener() {

			public void dragStart(DragSourceEvent event) {
				// System.out.println("drag started " + event);
			}

			public void dragSetData(DragSourceEvent event) {
				// System.out.println("drag set data " + event);
				event.data = "Test";
			}

			public void dragFinished(DragSourceEvent event) {
				// System.out.println("drag finished " + event);
			}
		});

		DropTarget dt = new DropTarget(tree, DND.DROP_COPY | DND.DROP_MOVE
				| DND.DROP_LINK);
		dt.setTransfer(new Transfer[] { TextTransfer.getInstance() });
		dt.addDropListener(new DropTargetListener() {
			public void dragEnter(DropTargetEvent event) {
				// System.out.println("drag enter " + event);
			}

			public void dragLeave(DropTargetEvent event) {
				// System.out.println("drag leave " + event);
			}

			public void dragOperationChanged(DropTargetEvent event) {
				// System.out.println("drag operation changed " + event);
			}

			public void dragOver(DropTargetEvent event) {
				// System.out.println("drag over " + event);
				event.feedback = DND.FEEDBACK_SCROLL | DND.FEEDBACK_EXPAND;
				System.out.println("CLIENT AREA: " + tree.getClientArea());
			}

			public void drop(DropTargetEvent event) {
				// System.out.println("drop " + event);
			}

			public void dropAccept(DropTargetEvent event) {
				// System.out.println("drop accept " + event);
			}
		});

		shell.setSize(200, 200);
		shell.open();
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch())
				display.sleep();
		}
		display.dispose();
	}
}

I assume that the behavior on other platforms is the same as on Windows XP, because GEF bug #332956, which is indirectly caused by this misbehavior on MacOSX Cocoa, does not seem to occur on any other platforms.
Comment 1 Scott Kovatch CLA 2011-01-04 12:13:27 EST 
Fixed > 20100103. Same behavior on Carbon, Cocoa, and Win32 now; getClientArea now returns a rectangle with origin 0, 0.
Comment 2 Peter Severin CLA 2011-01-08 07:09:10 EST 
Scott, any chance for this to be fixed in 3.6?
Comment 3 Scott Kovatch CLA 2011-01-08 19:01:11 EST 
I can clone this for 3.6.2, but it needs to go in ASAP. I think builds are underway now for the 3.6.2 candidate.
Comment 4 Silenio Quarti CLA 2011-01-10 10:44:41 EST 
I believe the previous behaviour was correct. The client area is not guaranteed to stay unchanged as you scroll (this is also true on GTK tree widget for the x coordinate).

With these changes, client area is inconsistent with the coordinate system of the widget. Mouse events,  bounds of items and children controls, display coordinate mapping and drawing done with GC are all wrong.   Note that when running the code below on windows, the bounds of the item and the coordinates of the mouse down event change as you scroll the tree, but the client area remains unchanged.

If you want to keep client area at 0,0 as the user scrolls a tree/table on cocoa, we would have to fix all the other places.  My vote is remove these changes.


import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.layout.*;
import org.eclipse.swt.widgets.*;


public class PR333375 {
public static void main(String[] args) {
	final Display display = new Display();
	Shell shell = new Shell(display);
	shell.setLayout(new FillLayout());

	final Tree tree = new Tree(shell, SWT.NONE);
	for (int i = 0; i < 10; i++) {
		TreeItem item = new TreeItem(tree, SWT.NONE);
		item.setText("Item xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + i);
	}
	
	tree.addListener(SWT.Paint, new Listener() {
		public void handleEvent(Event event) {
			TreeItem item = tree.getItem(3);
			GC gc = event.gc;
			gc.setForeground(display.getSystemColor(SWT.COLOR_RED));
			Rectangle rect = item.getBounds();
			Rectangle client = tree.getClientArea();
			gc.drawRectangle(rect);
			System.out.println(rect + " " + client);
		}
	});

	tree.addListener(SWT.MouseDown, new Listener() {
		public void handleEvent(Event event) {
			System.out.println("mouse down=" + event.x + " " + event.y);
		}
	});
	
	
	shell.setSize(200, 200);
	shell.open();
	while (!shell.isDisposed()) {
		if (!display.readAndDispatch())
			display.sleep();
	}
	display.dispose();
}
}
Comment 5 Silenio Quarti CLA 2011-01-10 10:49:42 EST 
I meant to say:

Note that when running the code below on windows, the bounds of the item change as you scroll the tree, but the client area remains unchanged. The x,y coordinate of the mouse event does not change. On cocoa,  
it is the reverse.
Comment 6 Alexander Nyßen CLA 2011-01-10 11:55:08 EST 
(In reply to comment #5)
> I meant to say:
> 
> Note that when running the code below on windows, the bounds of the item change
> as you scroll the tree, but the client area remains unchanged. The x,y
> coordinate of the mouse event does not change. On cocoa,  
> it is the reverse.

Well, that pretty much looks like an inconsistency to me...
Comment 7 Scott Kovatch CLA 2011-01-10 11:56:18 EST 
(In reply to comment #4)
> I believe the previous behaviour was correct. The client area is not guaranteed
> to stay unchanged as you scroll (this is also true on GTK tree widget for the x
> coordinate).
> 
> With these changes, client area is inconsistent with the coordinate system of
> the widget. Mouse events,  bounds of items and children controls, display
> coordinate mapping and drawing done with GC are all wrong.   Note that when
> running the code below on windows, the bounds of the item and the coordinates
> of the mouse down event change as you scroll the tree, but the client area
> remains unchanged.

I plead ignorance on this one, then. I forgot that a Tree and Table are also Scrollable, so the scroll view is considered part of the control. GIven that, it makes sense that the client area can change, as it's a window onto the underlying NSTableView.
Comment 8 Silenio Quarti CLA 2011-01-10 12:31:40 EST 
(In reply to comment #6)
> (In reply to comment #5)
> > I meant to say:
> > 
> > Note that when running the code below on windows, the bounds of the item change
> > as you scroll the tree, but the client area remains unchanged. The x,y
> > coordinate of the mouse event does not change. On cocoa,  
> > it is the reverse.
> 
> Well, that pretty much looks like an inconsistency to me...

This is not an inconsistency for me. It is a difference between platforms. The whole coordinate system (any API in the toolkit that takes or returns x,y) was consistent before this change and now it is not. 

Is there anything you cannot do because of this platform difference?
Comment 9 Scott Kovatch CLA 2011-01-10 12:37:21 EST 
I have undone the previous change to Scrollable.
Comment 10 Alexander Nyßen CLA 2011-01-10 12:38:50 EST 
(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > I meant to say:
> > > 
> > > Note that when running the code below on windows, the bounds of the item change
> > > as you scroll the tree, but the client area remains unchanged. The x,y
> > > coordinate of the mouse event does not change. On cocoa,  
> > > it is the reverse.
> > 
> > Well, that pretty much looks like an inconsistency to me...
> 
> This is not an inconsistency for me. It is a difference between platforms. The
> whole coordinate system (any API in the toolkit that takes or returns x,y) was
> consistent before this change and now it is not. 
> 
> Is there anything you cannot do because of this platform difference?

Well, I see the problem that the DropTargetEvent's location does not seem to fit into this. If you replace dragOver in my first snippet with the following implementation, you will notice that when you start to scroll the event location is always outside the tree's bounds and client area on Cocoa, while on windows its inside of both (or am I doing something wrong here?).

public void dragOver(DropTargetEvent event) {
				// System.out.println("drag over " + event);
				event.feedback = DND.FEEDBACK_SCROLL | DND.FEEDBACK_EXPAND;
				System.out.println("BOUNDS: " + tree.getBounds());
				System.out.println("CLIENT AREA: " + tree.getClientArea());
				Point location = tree.toControl(event.x, event.y);
				System.out.println("location: (" + location.x + ", "
						+ location.y + ")");
				if (location.x < 0 || location.y < 0
						|| location.x >= tree.getBounds().width
						|| location.y >= tree.getBounds().height) {
					System.out.println("OUTSIDE BOUNDS");
				} else {
					System.out.println("INSIDE BOUNDS");
				}
				if (location.x < 0 || location.y < 0
						|| location.x >= tree.getClientArea().width
						|| location.y >= tree.getClientArea().height) {
					System.out.println("OUTSIDE CLIENT AREA");
				} else {
					System.out.println("INSIDE CLIENT AREA");
				}
			}
Comment 11 Alexander Nyßen CLA 2011-01-10 12:41:31 EST 
(In reply to comment #10)
> (In reply to comment #8)
> > (In reply to comment #6)
> > > (In reply to comment #5)
> > > > I meant to say:
> > > > 
> > > > Note that when running the code below on windows, the bounds of the item change
> > > > as you scroll the tree, but the client area remains unchanged. The x,y
> > > > coordinate of the mouse event does not change. On cocoa,  
> > > > it is the reverse.
> > > 
> > > Well, that pretty much looks like an inconsistency to me...
> > 
> > This is not an inconsistency for me. It is a difference between platforms. The
> > whole coordinate system (any API in the toolkit that takes or returns x,y) was
> > consistent before this change and now it is not. 
> > 
> > Is there anything you cannot do because of this platform difference?
> 
> Well, I see the problem that the DropTargetEvent's location does not seem to
> fit into this. If you replace dragOver in my first snippet with the following
> implementation, you will notice that when you start to scroll the event
> location is always outside the tree's bounds and client area on Cocoa, while on
> windows its inside of both (or am I doing something wrong here?).
> 
> public void dragOver(DropTargetEvent event) {
>                 // System.out.println("drag over " + event);
>                 event.feedback = DND.FEEDBACK_SCROLL | DND.FEEDBACK_EXPAND;
>                 System.out.println("BOUNDS: " + tree.getBounds());
>                 System.out.println("CLIENT AREA: " + tree.getClientArea());
>                 Point location = tree.toControl(event.x, event.y);
>                 System.out.println("location: (" + location.x + ", "
>                         + location.y + ")");
>                 if (location.x < 0 || location.y < 0
>                         || location.x >= tree.getBounds().width
>                         || location.y >= tree.getBounds().height) {
>                     System.out.println("OUTSIDE BOUNDS");
>                 } else {
>                     System.out.println("INSIDE BOUNDS");
>                 }
>                 if (location.x < 0 || location.y < 0
>                         || location.x >= tree.getClientArea().width
>                         || location.y >= tree.getClientArea().height) {
>                     System.out.println("OUTSIDE CLIENT AREA");
>                 } else {
>                     System.out.println("INSIDE CLIENT AREA");
>                 }
>             }

Forget it, I have seen it myself. The following will be ok:

if (location.x < 0
						|| location.y < 0
						|| location.x >= tree.getClientArea().x
								+ tree.getClientArea().width
						|| location.y >= tree.getClientArea().y
								+ tree.getClientArea().height) {
					System.out.println("OUTSIDE CLIENT AREA");
				} else {
					System.out.println("INSIDE CLIENT AREA");
				}
Comment 12 Alexander Nyßen CLA 2011-01-10 12:45:17 EST 
While I still think this is not nice, I may live with it. I can work around it in GEF properly by no longer assuming that the client area has to be fixed to (0,0).
Comment 13 Alexander Nyßen CLA 2011-01-10 13:01:54 EST 
I have changed the related GEF code to take into account that client area may change during scrolling (bug #332956). I suppose to then close this one as WORKSFORME, as well as the related clone for 3.6.2 (bug #333805).
Comment 14 Scott Kovatch CLA 2011-01-10 13:02:40 EST 
> Forget it, I have seen it myself. The following will be ok:
> 
> if (location.x < 0
>                         || location.y < 0
>                         || location.x >= tree.getClientArea().x
>                                 + tree.getClientArea().width
>                         || location.y >= tree.getClientArea().y
>                                 + tree.getClientArea().height) {
>                     System.out.println("OUTSIDE CLIENT AREA");
>                 } else {
>                     System.out.println("INSIDE CLIENT AREA");
>                 }

You also want:

if (location.x < tree.getClientArea().x || location.y < tree.getClientArea().y || ....

for the same reasons.

The way the code is written now, you can only compare the mouse location to the client area, because toControl (and, indirectly, Display#map()) isn't taking the scrollView into account. toControl is giving you a client area-relative location. Is that what it's supposed to do?
Comment 15 Alexander Nyßen CLA 2011-01-10 13:11:05 EST 
(In reply to comment #14)
> > Forget it, I have seen it myself. The following will be ok:
> > 
> > if (location.x < 0
> >                         || location.y < 0
> >                         || location.x >= tree.getClientArea().x
> >                                 + tree.getClientArea().width
> >                         || location.y >= tree.getClientArea().y
> >                                 + tree.getClientArea().height) {
> >                     System.out.println("OUTSIDE CLIENT AREA");
> >                 } else {
> >                     System.out.println("INSIDE CLIENT AREA");
> >                 }
> 
> You also want:
> 
> if (location.x < tree.getClientArea().x || location.y < tree.getClientArea().y
> || ....
> 
> for the same reasons.
> 

Thank's for the hint. I have done it exactly this way in my fix for bug #332956. Just forget the snippet here, it was written a bit overhasty :-).

> The way the code is written now, you can only compare the mouse location to the
> client area, because toControl (and, indirectly, Display#map()) isn't taking
> the scrollView into account. toControl is giving you a client area-relative
> location. Is that what it's supposed to do?

Yes, I think that's ok. We had a check whether the location is inside the client area, which was 

Rectangle area = tree.getClientArea();
if (pt.x < 0 || pt.y < 0 || pt.x >= area.width || pt.y >= area.height)
  return null;

before, and which I have now adopted to the following:

Rectangle area = tree.getClientArea();
if (pt.x < area.x || pt.y < area.y || pt.x >= area.x + area.width || pt.y >= area.y + area.height)
  return null;
Comment 16 Silenio Quarti CLA 2011-01-10 13:31:33 EST 
(In reply to comment #14)
> You also want:
> 
> if (location.x < tree.getClientArea().x || location.y < tree.getClientArea().y
> || ....
> 
> for the same reasons.
> 
> The way the code is written now, you can only compare the mouse location to the
> client area, because toControl (and, indirectly, Display#map()) isn't taking
> the scrollView into account. toControl is giving you a client area-relative
> location. Is that what it's supposed to do?

Yes, that is correct. Every coordinate pair is relative to the client area.
Comment 17 Felipe Heidrich CLA 2011-08-05 13:49:28 EDT 
*** Bug 353907 has been marked as a duplicate of this bug. ***
Comment 18 Felipe Heidrich CLA 2011-08-05 13:50:38 EDT 
*** Bug 353905 has been marked as a duplicate of this bug. ***