Re: [PATCH] Improve icon container size calculation

[Alexander Larsson <alexl redhat com>]

On Wed, 2006-02-22 at 23:35 +0100, Christian Neumair wrote:
> The attached patch is meant to fix issues where the offset of the icons
> wrt the visible icon container border vary based on the panel width.
> This patch isn't tested too well but it is obvious that the old logic
> was wrong, i.e. pixel sizes were added to canvas unit sizes.

I don't think this patch is completely right:

@@ -806,12 +806,14 @@ nautilus_icon_container_update_scroll_re
 			(EEL_CANVAS (container),
 			 (double) - container->details->left_margin,
 			 (double) - container->details->top_margin,
-			 (double) (allocation->width - 1) / pixels_per_unit
+			 ((double) (allocation->width - 1)
 			 - container->details->left_margin
-			 - container->details->right_margin,
-			 (double) (allocation->height - 1) / pixels_per_unit
+			 - container->details->right_margin)
+			 / pixels_per_unit,
+			 ((double) (allocation->height - 1)
 			 - container->details->top_margin
-			 - container->details->bottom_margin);
+			 - container->details->bottom_margin)
+			 / pixels_per_unit);
 		return;
 	}

This applies the scaling factor to *_margins when subtracting from
allocation->width/height, which is right, since both values are in
pixels, and eel_canvas_set_scroll_region takes world coords. However,
the initial left/top_margin args aren't scaled.

@@ -1270,10 +1273,16 @@ placement_grid_mark (PlacementGrid *grid
}
 static void
-canvas_position_to_grid_position (PlacementGrid *grid,
+canvas_position_to_grid_position (NautilusIconContainer *container,
+				  PlacementGrid *grid,
 				  ArtIRect canvas_position,
 				  ArtIRect *grid_position)
 {
+	canvas_position.x0 -= container->details->left_margin / EEL_CANVAS (container)->pixels_per_unit;
+	canvas_position.x1 -= container->details->left_margin / EEL_CANVAS (container)->pixels_per_unit;
+	canvas_position.y0 -= container->details->top_margin / EEL_CANVAS (container)->pixels_per_unit;
+	canvas_position.y1 -= container->details->top_margin / EEL_CANVAS (container)->pixels_per_unit;

The set_scroll_region stuff we do above sets the top of the desktop
window to -top_margin precisely in order to make y-coordinate 0 start at
the visible region (below the top panel margin). So, this shouldn't be
necessary I think.

I commited the attached patch. I verified that it at lest fixed the top
icon moving wrongly when using != 100% default zoom and changing the
size of the top panel.

> The original motivation was that I hoped to be able to easily fix the
> problems reported by Martin where icons randomly jumped around
> (reproducible when pressing ctrl-R).

Does this still happen with federicos patch applied. I'm a bit confused
about the various icon positioning issues atm.

> b) replace the NAUTILUS_IS_DESKTOP_ICON_FILE check in
> fm_icon_view_add_file with has_volume || has_drive. I think that's how I
> wrote it originally. Alex, was this modified to not break computer:///?
> Maybe we should just pass FALSE for ordinary icon views and override the
> _add handler in fm-desktop-icon-view.c, adding extra volume/drive
> checks. I still think we'd have issues with network mounts that are
> close to other icons.

I'm not sure how this is related to the jumping around, but isn't
IS_DESKTOP_ICON better, since it only additionally handles things like
the home icon and the trash that can be turned on and off.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's an impetuous one-eyed barbarian searching for his wife's true killer. 
She's a foxy kleptomaniac research scientist operating on the wrong side of 
the law. They fight crime! 

Index: libnautilus-private/nautilus-icon-container.c
===================================================================
RCS file: /cvs/gnome/nautilus/libnautilus-private/nautilus-icon-container.c,v
retrieving revision 1.409
diff -u -p -r1.409 nautilus-icon-container.c
--- libnautilus-private/nautilus-icon-container.c	27 Feb 2006 13:16:04 -0000	1.409
+++ libnautilus-private/nautilus-icon-container.c	27 Feb 2006 14:01:55 -0000
@@ -804,14 +804,16 @@ nautilus_icon_container_update_scroll_re
 		allocation = &GTK_WIDGET (container)->allocation;
 		eel_canvas_set_scroll_region
 			(EEL_CANVAS (container),
-			 (double) - container->details->left_margin,
-			 (double) - container->details->top_margin,
-			 (double) (allocation->width - 1) / pixels_per_unit
+			 (double) - container->details->left_margin / pixels_per_unit,
+			 (double) - container->details->top_margin / pixels_per_unit,
+			 ((double) (allocation->width - 1)
 			 - container->details->left_margin
-			 - container->details->right_margin,
-			 (double) (allocation->height - 1) / pixels_per_unit
+			 - container->details->right_margin)
+			 / pixels_per_unit,
+			 ((double) (allocation->height - 1)
 			 - container->details->top_margin
-			 - container->details->bottom_margin);
+			 - container->details->bottom_margin)
+			 / pixels_per_unit);
 		return;
 	}
 
@@ -999,7 +1001,8 @@ lay_down_icons_horizontal (NautilusIconC
 		/ EEL_CANVAS (container)->pixels_per_unit;
 	canvas_height = (GTK_WIDGET (container)->allocation.height
 			 - container->details->top_margin
-			 - container->details->bottom_margin) / EEL_CANVAS (container)->pixels_per_unit;
+			 - container->details->bottom_margin)
+		/ EEL_CANVAS (container)->pixels_per_unit;
 
 	max_icon_width = max_text_width = 0.0;
 
@@ -1192,14 +1195,14 @@ placement_grid_new (NautilusIconContaine
 	int i;
 
 	/* Get container dimensions */
-	width  = GTK_WIDGET (container)->allocation.width /
-		EEL_CANVAS (container)->pixels_per_unit
-		- container->details->left_margin
-		- container->details->right_margin;
-	height = GTK_WIDGET (container)->allocation.height /
-		EEL_CANVAS (container)->pixels_per_unit
-		- container->details->top_margin
-		- container->details->bottom_margin;
+	width  = (GTK_WIDGET (container)->allocation.width
+		  - container->details->left_margin
+		  - container->details->right_margin) /
+		EEL_CANVAS (container)->pixels_per_unit;
+	height = (GTK_WIDGET (container)->allocation.height
+		  - container->details->top_margin
+		  - container->details->bottom_margin) /
+		EEL_CANVAS (container)->pixels_per_unit;
 
 	num_columns = width / SNAP_SIZE_X;
 	num_rows = height / SNAP_SIZE_Y;
@@ -1328,14 +1331,14 @@ find_empty_location (NautilusIconContain
 	gboolean collision;
 
 	/* Get container dimensions */
-	canvas_width  = GTK_WIDGET (container)->allocation.width /
-		EEL_CANVAS (container)->pixels_per_unit
-		- container->details->left_margin
-		- container->details->right_margin;
-	canvas_height = GTK_WIDGET (container)->allocation.height /
-		EEL_CANVAS (container)->pixels_per_unit
-		- container->details->top_margin
-		- container->details->bottom_margin;
+	canvas_width  = (GTK_WIDGET (container)->allocation.width
+			 - container->details->left_margin
+			 - container->details->right_margin) /
+		EEL_CANVAS (container)->pixels_per_unit;
+	canvas_height = (GTK_WIDGET (container)->allocation.height
+			 - container->details->top_margin
+			 - container->details->bottom_margin) /
+		EEL_CANVAS (container)->pixels_per_unit;
 
 	icon_get_bounding_box (icon, 
 			       &icon_position.x0, &icon_position.y0,