Neighbourhood view icons can collide with central buddy icon

[garycmartin]

In the gtk3 version of the Sugar shell, Neighbourhood view icons can now sometimes collide with the central buddy icon, they should avoid overlapping with this icon (as well as each other). See attached screen shot (though I've seen much worse, just didn't managed to get a screen shot at the time).

[manuq]

TestCase, copy to sugar/src/jarabe/desktop/ and run from a sugar shell.

[humitos]

This patch reduces the probability to collide with the central buddy (locked position). It marks the icon to be re-arranged if it collides with the central buddy.

[humitos]

A little improved version of _detect_collisions

[humitos]

I've just uploaded a patch that mark the icon to be re-arranged if it collides with the central buddy (locked icon). This reduces the possibility to collide with it.

I tried many times the test case that manuq uploaded some weeks ago and I think the patch it's acceptable. Sometimes I get the icons collided by that it's a normal behaviour because of the way that the SugarExt.Grid is implemented.

Well, this patch needs testing because maybe I am skipping some possibilities.

[dsd]

humitos has figured out whats going wrong here, but I don't think the patch really approaches the issue in a way that can get a good review. I'll try to help with the explanation:

Two icons are colliding: the owner icon, and an AP icon being added. The owner icon is stored on the grid as locked (it should not be move), the AP icon is not locked (it can be moved). The collision detection code here is failing to resolve the collision.

_detect_collisions() is called for the AP icon. It iterates over each of the other children in the grid, checking for collisions.

It reaches the owner icon, and this happens:

            if c != child and intersection.width > 0:

This condition passes: the owner is not the AP icon, and an intersection(collision) was found.

                if (c not in self._locked_children and
                    c not in self._collisions):
                    collision_found = True

This condition fails: c(the owner) is locked, so we never record a collision as happening. And that is the only place where a collision would possibly be recorded. So we take no action on the collision, resulting in the behaviour (colliding icons) that we see here.

Maybe a simple and logical fix can be found to this that would be more suitable for this late stage of 0.98. Failing that, I'm attaching my take on redoing this logic in a way that is easy to understand, inclusive of this case, and heavily commented. Its not tested.

[dsd]

improved collision detection behaviour

[humitos]

A bit of history: ​http://dev.laptop.org/ticket/8372

[manuq]

Testing improved algorithm

[manuq]

Testing improved algorithm 2

[manuq]

I have been testing the improved code for some days now. I can still find some overlap with the central buddy's head as the screenshots show, but is minor, and I think this is an improvement. In the random view you can force the collision dragging activity icons into the central buddy. They are moved to the outside now (there is the jump described in #3960). I would say let's push and improve further later.

[dsd]

I think humitos' 0001-Avoid-collisions-with-central-buddy-SL-3944.patch is a good solution for 0.98. It means that when a collision is found with a locked icon, instead of not moving any icon, the unlocked one is moved. The logic is hard to follow, but that can be fixed in the next cycle with my rewrite above.

[dsd]

note: I haven't tested either humitos' patch or my rewrite.

[manuq]

Pushed the Avoid... as d0a5c110 but let's keep this open for further improvement.

[erikos]

I can still see occasions where an AP icon can collide with the owner icon, ​tested with os32, hence Sugar 0.98.3.

[dnarvaez]

It seems like the patch improves the situation a lot. If it still happens rarely I think it's not such a big deal, compared to the other collab issues we have.