Merge lp:~7-eric/inkscape/boolop-fix into lp:~inkscape.dev/inkscape/trunk

[Eric Greveson]

Fix for Bug #1209281. "Difference" and "Intersection" Boolean operations should now behave in a reasonable way when one or more input shapes is small enough that the automatic quantization step (before the Boolean operation takes place) shrinks it to zero. "Union" and "Exclusion" operations behave as before (already correct).

[su_v (suv-lp)]

I can't review the proposed code itself, but based on tests (r12472+diff on OS X 10.7.5) with files from related bug reports I approve: no regression or unexpected results encountered.

review: Approve

[Alvin Penner (apenner)]

looks good to me, thanks!

review: Approve

=== modified file 'src/splivarot.cpp'
--- src/splivarot.cpp	2013-07-04 14:01:44 +0000
+++ src/splivarot.cpp	2013-08-08 16:13:45 +0000
@@ -277,11 +277,37 @@
 
             theShapeB->ConvertToShape(theShape, origWind[curOrig]);
 
-            if (theShapeA->numberOfEdges() == 0) {
-                Shape *swap = theShapeB;
-                theShapeB = theShapeA;
-                theShapeA = swap;
-            } else if (theShapeB->numberOfEdges() > 0) {
+            // Due to quantization of the input shape coordinates, we may end up with A or B being empty.
+            // If this is a union or symdiff operation, we just use the non-empty shape as the result:
+            //   A=0  =>  (0 or B) == B
+            //   B=0  =>  (A or 0) == A
+            //   A=0  =>  (0 xor B) == B
+            //   B=0  =>  (A xor 0) == A
+            // If this is an intersection operation, we just use the empty shape as the result:
+            //   A=0  =>  (0 and B) == 0 == A
+            //   B=0  =>  (A and 0) == 0 == B
+            // If this a difference operation, and the upper shape (A) is empty, we keep B.
+            // If the lower shape (B) is empty, we still keep B, as it's empty:
+            //   A=0  =>  (B - 0) == B
+            //   B=0  =>  (0 - A) == 0 == B
+            //
+            // In any case, the output from this operation is stored in shape A, so we may apply
+            // the above rules simply by judicious use of swapping A and B where necessary.
+            bool zeroA = theShapeA->numberOfEdges() == 0;
+            bool zeroB = theShapeB->numberOfEdges() == 0;
+            if (zeroA || zeroB) {
+            	// We might need to do a swap. Apply the above rules depending on operation type.
+            	bool resultIsB = ((bop == bool_op_union || bop == bool_op_symdiff) && zeroA)
+            			|| ((bop == bool_op_inters) && zeroB)
+            			|| (bop == bool_op_diff);
+                if (resultIsB) {
+                	// Swap A and B to use B as the result
+                    Shape *swap = theShapeB;
+                    theShapeB = theShapeA;
+                    theShapeA = swap;
+                }
+            } else {
+            	// Just do the Boolean operation as usual
                 // les elements arrivent en ordre inverse dans la liste
                 theShape->Booleen(theShapeB, theShapeA, bop);
                 Shape *swap = theShape;