Opened 11 years ago
Closed 10 years ago
#3944 closed defect (fixed)
Neighbourhood view icons can collide with central buddy icon
Reported by: | garycmartin | Owned by: | humitos |
---|---|---|---|
Priority: | High | Milestone: | Unspecified |
Component: | Sugar | Version: | 0.97.x |
Severity: | Major | Keywords: | 13.1.0, regression |
Cc: | manuq, humitos | Distribution/OS: | Unspecified |
Bug Status: | Unconfirmed |
Description
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).
Attachments (7)
Change History (22)
Changed 11 years ago by garycmartin
comment:1 Changed 11 years ago by manuq
- Owner changed from erikos to manuq
- Status changed from new to assigned
comment:2 Changed 11 years ago by manuq
- Keywords regression added
comment:3 Changed 11 years ago by manuq
- Priority changed from Unspecified by Maintainer to High
- Severity changed from Unspecified to Major
Changed 11 years ago by manuq
Changed 10 years ago by 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.
comment:4 Changed 10 years ago by humitos
- Cc humitos added
- Owner changed from manuq to humitos
- Status changed from assigned to accepted
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.
comment:5 Changed 10 years ago by 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.
comment:6 Changed 10 years ago by humitos
A bit of history: http://dev.laptop.org/ticket/8372
comment:7 Changed 10 years ago by erikos
- Keywords r? added
comment:8 Changed 10 years ago by 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.
comment:9 Changed 10 years ago by 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.
comment:10 Changed 10 years ago by dsd
note: I haven't tested either humitos' patch or my rewrite.
comment:11 Changed 10 years ago by manuq
Pushed the Avoid... as d0a5c110 but let's keep this open for further improvement.
comment:12 Changed 10 years ago by erikos
- Keywords r? removed
comment:13 Changed 10 years ago by erikos
I can still see occasions where an AP icon can collide with the owner icon, tested with os32, hence Sugar 0.98.3.
comment:14 Changed 10 years ago by dnarvaez
- Milestone changed from 0.98 to Unspecified
comment:15 Changed 10 years ago by dnarvaez
- Resolution set to fixed
- Status changed from accepted to closed
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.
TestCase, copy to sugar/src/jarabe/desktop/ and run from a sugar shell.