Re: [PATCH] Fix canvas item bounds calculation

[Alexander Larsson <alexl redhat com>]

On Wed, 2006-02-15 at 21:31 +0100, Martin Wehner wrote:
> On Sat, 2005-11-12 at 13:44 +0100, Christian Neumair wrote:
> > The attached patch fixes various glitches, inter alia problems with
> > icons drags of icons containing emblems which are not totally inside the
> > icon rect, drawing artefacts after moving icons with zoom levels !=
> > 100%, and hit box problems with zoom levels != 100%.
> 
> I looked into a regression with the icon layout code and this patch
> turns out to be the culprit. It actually breaks the emblem bounds
> calculation: Create two files in the icon view and give both an emblem.
> Then zoom < 100%. There'll be a huge gap between the icons as the emblem
> rectangle gets larger and larger (it's divided by a value < 1 after all)
> as the zoom level decreases.
> I don't think bringing pixels_per_unit into play at that point is right
> at all but even if the calculation would be correct, it'd have to go
> into update_bounds() too.

No, I don't think that patch is wrong. Something else must be wrong.
Here is what is happening in nautilus_icon_canvas_item_bounds:

We're asked to calculate the bounds of the item in item-relative
coordinates (which, since we don't use groups w/ scaling is the same
unit size as world coords). However, the icon size data we have is in
pixel coordinates, so to convert this we divide by pixels_per_unit. 

You can compare this to the text handling in the same function, it calls
measure_label_text which sets item->details->text_width and
item->details->text_height to the text height in pixels. We then call:
 text_rect = compute_text_rectangle (icon_item, icon_rect, FALSE);
where the last FALSE means the data is in pixel coords, causing
compute_text_rectangle to divide by pixels_per_unit.

Things are different in nautilus_icon_canvas_item_update_bounds. There
we first calculate the new bounding rect (by calling
recompute_bounding_box, which will result in a call to
nautilus_icon_canvas_item_bounds and that result will be converted to
canvas coords involving a multiplication by pixels_per_unit). We then
continue to calculate various extent caches, but these are all stored in
pixel coords (i.e. passing TRUE to compute_text_rectangle).

This is a hellhole of various coordinate spaces, transforms and
similarly named functions that are slightly different. I'm sure there
are various bugs in it, but I think this particular change was right.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a war-weary coffee-fuelled cowboy with a robot buddy named Sparky. She's 
a bloodthirsty tempestuous former first lady in the wrong place at the wrong 
time. They fight crime!