After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 669291 - box layout bug when callin the clutter_actor_hide() function
box layout bug when callin the clutter_actor_hide() function
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: ClutterActor
1.8.x
Other Linux
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2012-02-03 06:48 UTC by Kun.Liu2
Modified: 2012-05-20 13:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
box-layout: Fix allocation brain farts (6.57 KB, patch)
2012-02-14 15:48 UTC, Emmanuele Bassi (:ebassi)
committed Details | Review
box-layout: Fix allocation brain farts (5.05 KB, patch)
2012-02-14 16:01 UTC, Emmanuele Bassi (:ebassi)
committed Details | Review
The bug is not fully fixed. (1.54 KB, patch)
2012-05-18 13:41 UTC, sdegrande
committed Details | Review

Description Kun.Liu2 2012-02-03 06:48:09 UTC
when we use the box layout and called the clutter_actor_hide() function, it will cause some strange bug.

Such as:
We want add two actor into a box layout container.
actor1:
ClutterText : "haha"
actor2:
ClutterText : "hehe"

After added to the box container it will be shown like:
      "haha" "hehe"

BUG:
If you called function clutter_actor_hide(actor1), then the two actor will disappeared at the same time.

If we called function clutter_actor_hide(actor2), then it will be shown like:
             "haha"

the position is not right.
Comment 1 Emmanuele Bassi (:ebassi) 2012-02-14 12:10:06 UTC
can confirm this happens with master as well.

it seems that the BoxLayout is allocating the actor at the wrong position if the first actor is not visible.
Comment 2 Emmanuele Bassi (:ebassi) 2012-02-14 15:47:56 UTC
ugh, in turns out that the ClutterBoxLayout::allocate is full of FAIL since we moved to the same algorithm of GtkBox - given the impedance mismatch between Clutter and Gtk.
Comment 3 Emmanuele Bassi (:ebassi) 2012-02-14 15:48:07 UTC
Created attachment 207532 [details] [review]
box-layout: Fix allocation brain farts

The allocation code for BoxLayout contains a sequence of brain farts
that make it barely working since the synchronization of the layout
algorithm to the one in GtkBox.

The origin of the layout is inverted, and it doesn't take into
consideration a modified allocation origin (for actors the provide
padding or margin).

The pack-start property is broken, and it only works because we walk the
children list backwards; this horribly breaks when a child changes
visibility. Plus, we count invisible children, which leads to
allocations getting insane origins (either close to -MAX_FLOAT or
MAX_FLOAT).

Finally, the allocation is applied twice even for non-animated cases.
Comment 4 Emmanuele Bassi (:ebassi) 2012-02-14 15:49:42 UTC
the patch above is against master, and won't apply cleanly to 1.8; I'll prepare an equivalent patch for the stable branch.
Comment 5 Emmanuele Bassi (:ebassi) 2012-02-14 16:01:03 UTC
Created attachment 207534 [details] [review]
box-layout: Fix allocation brain farts

Patch for the clutter-1.8 branch.
Comment 6 Emmanuele Bassi (:ebassi) 2012-02-14 17:10:16 UTC
Comment on attachment 207532 [details] [review]
box-layout: Fix allocation brain farts

attachment 207532 [details] [review] pushed to master as commit c287e91
Comment 7 Emmanuele Bassi (:ebassi) 2012-02-15 11:14:07 UTC
Comment on attachment 207534 [details] [review]
box-layout: Fix allocation brain farts

attachment 207534 [details] [review] pushed to master as commit 0eaa897
Comment 8 sdegrande 2012-05-18 13:41:24 UTC
Created attachment 214309 [details] [review]
The bug is not fully fixed.

When the child of a box layout container is invisible, the
position of the last displayed child is wrong.

ClutterBoxLayout::allocate() first computes the sizes needed for
each of the visible children and stores those sizes into a 'sizes'
array (the array counter is incremented on visible children only).

Later on, ClutterBoxLayout::allocate() computes the position of
each visible children, using the 'sizes' array. However, it uses the
child index to access the array, instead of a 'visible child index'.


The patch has been created and checked with clutter-1.8, but the same fix is needed with master.
Comment 9 Emmanuele Bassi (:ebassi) 2012-05-18 14:15:37 UTC
Review of attachment 214309 [details] [review]:

looks good
Comment 10 Emmanuele Bassi (:ebassi) 2012-05-18 14:22:06 UTC
(In reply to comment #8)
> The patch has been created and checked with clutter-1.8, but the same fix is
> needed with master.

as far as I can see, both clutter-1.10 and master increment the i variable only at the end of the cycle, which is what the patch does.
Comment 11 sdegrande 2012-05-20 13:48:34 UTC
(In reply to comment #10)
> as far as I can see, both clutter-1.10 and master increment the i variable only
> at the end of the cycle, which is what the patch does.

Yes, indeed... I should have looked at the wrong branches... Sorry.