Another bug in desktop icon positions

[Federico Mena Quintero <federico ximian com>]

Hi,

My patch from http://bugzilla.gnome.org/show_bug.cgi?id=45953#c32 was
incorrect.  This was to fix the case where you 

	1. insert a CD or other removable media

	2. the media already had position metadata

	3. but there's another icon in the desktop in that position

	4. so you get an overlap

It turns out that my patch *didn't* work if there was no metadata to
begin with, i.e. for a new user, or for media with a totally new display
name.  The new icon would end up in both the semi_position_icons and
no_position_icons lists (ugh!).

The attached patch fixes this with very simple logic:

  if (has_position_metadata (icon)) {
     if (no_auto_layout && icon.has_lazy_position)) {
         semi_position_icons += icon;
     }
  } else {
     no_position_icons += icon;
  }

Also, this patch fixes another bug when snapping new icons to the grid.
Icons could get placed with the icon image at x=0.  The ChangeLog
describes when this could happen.

With this, icon placement in the desktop seems to be totally reliable
for me, and I tested a bunch of cases --- no metadata, existing metadata
with and without overlaps, files that appear and disappear.

The only remaining case where you can get overlaps when icons appear out
of the blue is case (2) in this comment:
http://bugzilla.gnome.org/show_bug.cgi?id=45953#c30

This is where you

	cd ~/Desktop
	touch foo
	rm foo
	touch bar
	touch foo

... and foo ends up overlapping bar.  We can solve this by simply
deleting foo's metadata when we notice that it got removed.  I haven't
really looked for the code that notices this, but I'm sure others can
pinpoint it in a second :)

Is this OK to commit?

  Federico

2006-02-23  Federico Mena Quintero  <federico novell com>

	* libnautilus-private/nautilus-icon-container.c
	(finish_adding_new_icons): Do not place icons both in the
	no_position_icons and semi_position_icons lists!
	(snap_position): The final *x could be negative if (start_x + icon_width / 2)
	is less than SNAP_SIZE_X, as SNAP_NEAREST_HORIZONTAL() would
	return DESKTOP_PAD_HORIZONTAL.  Then, we would subtract icon_width / 2,
	getting a negative number.  So, we initially test for this and
	start that the first snap column.  The same reasoning applies to
	*y and the baseline.

Index: nautilus-icon-container.c
===================================================================
RCS file: /cvs/gnome/nautilus/libnautilus-private/nautilus-icon-container.c,v
retrieving revision 1.408
diff -u -p -r1.408 nautilus-icon-container.c
--- nautilus-icon-container.c	6 Feb 2006 16:48:08 -0000	1.408
+++ nautilus-icon-container.c	24 Feb 2006 02:23:27 -0000
@@ -1140,18 +1140,18 @@ snap_position (NautilusIconContainer *co
 	int icon_height;
 	ArtDRect icon_position;
 	
-	if (*x < DESKTOP_PAD_HORIZONTAL) {
-		*x = DESKTOP_PAD_HORIZONTAL;
+	icon_position = nautilus_icon_canvas_item_get_icon_rectangle (icon->item);
+	icon_width = icon_position.x1 - icon_position.x0;
+	icon_height = icon_position.y1 - icon_position.y0;
+
+	if (*x + icon_width / 2 < DESKTOP_PAD_HORIZONTAL + SNAP_SIZE_X) {
+		*x = DESKTOP_PAD_HORIZONTAL + SNAP_SIZE_X - icon_width / 2;
 	}
 
-	if (*y < DESKTOP_PAD_VERTICAL) {
-		*y = DESKTOP_PAD_VERTICAL;
+	if (*y + icon_height < DESKTOP_PAD_VERTICAL + SNAP_SIZE_Y) {
+		*y = DESKTOP_PAD_VERTICAL + SNAP_SIZE_Y - icon_height;
 	}
 
-	icon_position = nautilus_icon_canvas_item_get_icon_rectangle (icon->item);
-	icon_width = icon_position.x1 - icon_position.x0;
-	icon_height = icon_position.y1 - icon_position.y0;
-	
 	center_x = *x + icon_width / 2;
 	*x = SNAP_NEAREST_HORIZONTAL (center_x) - (icon_width / 2);
 
@@ -5629,13 +5629,14 @@ finish_adding_new_icons (NautilusIconCon
 	no_position_icons = semi_position_icons = NULL;
 	for (p = new_icons; p != NULL; p = p->next) {
 		icon = p->data;
-		if (!assign_icon_position (container, icon)) {
-			no_position_icons = g_list_prepend (no_position_icons, icon);
-		}
+                if (assign_icon_position (container, icon)) {
+                        if (!container->details->auto_layout && icon->has_lazy_position) {
+                                semi_position_icons = g_list_prepend (semi_position_icons, icon);
+                        }
+                } else {
+                        no_position_icons = g_list_prepend (no_position_icons, icon);
+                }
 
-		if (!container->details->auto_layout && icon->has_lazy_position) {
-			semi_position_icons = g_list_prepend (semi_position_icons, icon);
-		}
 		finish_adding_icon (container, icon);
 	}
 	g_list_free (new_icons);