I took some time to look through your comments. The attached patch is meant to show the location of the changes I propose for the stacked icons issue - not necessarily to be used (though it should compile).
On Mon, 2010-01-18 at 14:42 +0100, Alexander Larsson wrote: > While I agree that in some cases the original snap values are to small > I'm not sure just bumping them is the ideal solution. > > There are a few issues I see, first of all it doesn't take into account > a different zoom factor. If you change the default zoom to 50% then your > values are to large. I see the issues of simply increasing the grid size. There definitely is an increase in whitespace with my spacings. For the zoom, however, this doesn't seem to be an issue. I tested my changes at a number of zoom levels, and the grid scales with each level. I searched through nautilus_icon_container.c, and it seems zoom is handled through the conversion of global coordinates (where snap size works) to pixel coordinates. > Third, with multiple size icons in the same layout (stretched, > thumbnails, normal icons) increasing the snap size is risky as if some > icon is accidentally a tiny amount larger than the snap size we almost > double the size used for that icon. This can create large holes in the > layout. If the desired behavior of the desktop is to be a grid, I would think this is what we want. If we didn't allow massive icons to take up more grid space, the desktop would allow overlapping icons (or it wouldn't be aligned). > This isn' right. icon_set_position is the really lowlevel positioning > function. The code tries to handle keep_aligned before changing the > position, obviously we're missing it somewhere, but forcing snapping > always (even after e.g. a relayout which uses icon_set_position()) isn't > the right solution. > > In fact, isn't there a risk of infinite loops since align_icons() calls > icon_set_position(). This was poor thinking on my part - using icon_set_position() could have many unintended consequences. It worked because I used schedule_align_icons instead of align_icons, though that probably had an infinite loop in idle cycles anyway. Looking at the higher level functions (e.g. nautilus_icon_container_move_icon()), I think the snap problem comes from the way icon alignment is handled when an icon is moved or added. I tried the following changes and it gave the same functionality: - In nautilus_icon_container_move_icon(), icon alignment is handled through snap_position(), but snap_position() snaps icons to the grid with no regard for other icons. I think this the true cause of the "stacked icons" issue. I propose replacing snap_position with schedule_align_icons(), and placing the statement after setting the icon position (to give a starting position to the icon). - In nautilus_icon_container_add(), I can't find any reference to alignment in the current code. I propose adding schedule_align_icons() to the end of the function (because it should occur after the icon is fully created). Note my changes use schedule_align_icons() rather than align_icons() because the former is better behaved. I tried align_icons(), and though it somewhat works, it occasionally refuses to move icons and acts strangely. I'm not sure if there is a better way to enforce grid alignment, so I'd welcome any suggestions. Though the above may fix the overlapping issue, I noticed the realignment interrupts renaming mode for a newly created icon / folder. Looking at icon_set_position, moving an icon ends the renaming mode and doesn't start it again. Do we want icons to start snapped to the grid when renaming? Another way could be to have renaming occur in a non-aligned position (e.g. where the user clicked) and then snap once renaming is finished. I'm not sure which way is preferred.
>From b54ee565f8e30319d248e5005de62750fc16ce15 Mon Sep 17 00:00:00 2001 From: Bill Smith <[email protected]> Date: Mon, 18 Jan 2010 12:46:16 -0500 Subject: [PATCH] Proposed changes for "Keep Aligned" Replaces snap_position() with schedule_align_icons() when moving icons, and fixes icon alignment when new icons are added. --- libnautilus-private/nautilus-icon-container.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/libnautilus-private/nautilus-icon-container.c b/libnautilus-private/nautilus-icon-container.c index ae58a49..ca271e1 100644 --- a/libnautilus-private/nautilus-icon-container.c +++ b/libnautilus-private/nautilus-icon-container.c @@ -225,6 +225,8 @@ static int compare_icons_vertical (NautilusIconContainer *container, static void store_layout_timestamps_now (NautilusIconContainer *container); +static void schedule_align_icons (NautilusIconContainer *container); + static gpointer accessible_parent_class; static GQuark accessible_private_data_quark = 0; @@ -2428,15 +2430,15 @@ nautilus_icon_container_move_icon (NautilusIconContainer *container, } if (!details->auto_layout) { - if (details->keep_aligned && snap) { - snap_position (container, icon, &x, &y); - } - if (x != icon->x || y != icon->y) { icon_set_position (icon, x, y); emit_signal = update_position; } + if (details->keep_aligned && snap) { + schedule_align_icons (container); + } + icon->saved_ltr_x = nautilus_icon_container_is_layout_rtl (container) ? get_mirror_x_position (container, icon, icon->x) : icon->x; } @@ -7324,6 +7326,10 @@ nautilus_icon_container_add (NautilusIconContainer *container, /* Run an idle function to add the icons. */ schedule_redo_layout (container); + if (container->details->keep_aligned) { + schedule_align_icons(container); + } + return TRUE; } -- 1.6.3.3
-- nautilus-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/nautilus-list
