Created attachment 252494 [details] [review]
box-layout: Fix RTL layout swapping with non-zero container offsets

*** Bug 706449 has been marked as a duplicate of this bug. ***

Review of attachment 252494 [details] [review]:

::: clutter/clutter-box-layout.c
@@ +1218,3 @@
               gfloat width = child_allocation.x2 - child_allocation.x1;
 
+              child_allocation.x1 = box->x1 + (box->x2 - child_allocation.x1 - width);

shouldn't this be (box.x2 + child_allocation.x1 - width)?

I don't see how that would be correct. If we have a parent box spanning from 200-500, let's say, and we allocate a child spanning 50-150 through the above logic, then this condition needs to swap it around so the child is allocated spanning 350-450 (50 on the left becomes 500-50 on the right, and then the width is taken from that to make 350):

  200 + (500 - 50 - 100) = 350

It might make sense to do x2 first, and then we fill in x1 by subtracting the width instead, so it becomes:

  child_allocation.x2 = box->x1 + (box->x2 - child_allocation.x1);
  child_allocation.x1 = child_allocation.x2 - width;

Which might be a bit more obvious

I do prefer obvious to clever, so the second form looks better to me.

feel free to commit that.

Attachment 252494 [details] pushed as 05f56af - box-layout: Fix RTL layout swapping with non-zero container offsets


Pushed with suggested changes.