Boolean subtract with closed: false doesn't work as expected

[lehni]

Sketch 1

Result:

Expected:

[lehni]

Here Sketch 2 for discussion.

[mokafolio]

I think there should be options to control how the boolean subtract operation affects the resulting path (This might also be relevant for operations other than subtract, not sure right now). For subtracting I can think of two possible results that make sense, lets name them trace and split.
In my opinion, these options should be separate from the existing closed option as they are not the same, here is an illustration:

The api could be something like: path1.subtract(path2, {mode: "trace/split"});

The naming is just my initial idea and could possibly be better!
Does that make sense at all?

[mokafolio]

To give a real world example of why I think this is very useful: If you want to remove overlapping/occluded paths (i.e. for pen plotting), the split version makes a lot of sense to prevent drawing any outline more than once, while the trace version is I guess the expected result in the traditional sense!

[mokafolio]

the closed flag does not appear to work in develop branch. i rolled all my current boolean ops stuff back to the bower/stable version as that seems to work alright.

[lehni]

For the record, here the other issues that related to the treatment of open paths:

#757
#1036
#1089

[lehni]

@mokafolio I am still waiting from some test-cases from you here.

[mokafolio]

@lehni sorry zoned out on that. I put together a quick thing here to compare the bower paper version vs the current develop:

http://stuff.mokafolio.de/DevelopVsBowerBoolean/BowerSubtract.html
http://stuff.mokafolio.de/DevelopVsBowerBoolean/DevelopSubtract.html

made a repo here too: https://github.com/mokafolio/PaperJSDevelopVSBowerSubtract

The examples take 200 circles and subtract them from back to front. Both appear to perform very similarily, here are 5 results I got for each:

Bower for 200 circles:
6598.660000000001ms
6135.215ms
6436.640000000001ms
6612.035000000001ms
6517.075000000001


Develop for 200 circles:
6261.555ms
6305.515ms
6505.865000000001ms
6608.64ms
6590.790000000001ms

I think the difference in speed that I was seeing in my actual app was due to the open flag that is available in the current bower branch but not working in current develop. My guess is that that flag made it faster, we should re-evaluate it as soon as it works in develop again. That said, I could not get my actual app to work with the develp branch at all (I would love to try again as soon as we have a way to support open paths. I recommend providing an API similar to what I described in the comment with the illustration above).

Hope that makes sense!

[lehni]

In the same way, divide() should work with a line and a path:

Sketch 3

[lehni]

@mokafolio, @niravdhanorkar: I finally found some time to look into this, and it turns out silly me got options.closed confused. It's called options.stroke since quite a while! And there was a tiny error that prevented options from being passed on to computeBoolean() in PathItem#subtract(path), which I fixed in dd56f86

With that in place, everything already works as it should:

• options.stroke = false is the same as 'trace' in @mokafolio's example
• options.stroke = true is the same as 'split' in @mokafolio's example

I don't like the naming of the current option all that much though. I also don't think mode is good, but what about options.split instead of options.stroke, as a boolean?

Another question that we still have to decide:

Should the results of this split-mode be collected in a Group item as right now, or should they end up in a CompoundPath?

I tend to think CompoundPath is better, as it would help with reducing the joint result in PathItem#divide(), which otherwise would result in a group of groups, see Sketch 3 above...

[lehni]

Another possibility is to call the option options.trace, as a boolean that defaults to true and has to be explicitly set to false in order to use the split mode. This may be better and more explicit?

Thoughts?

[lehni]

I have implemented all my suggestions now in 3c9d2ee, along with unit tests for them.

Let's keep the discussion going regarding terminology and CompoundPath VS Group.

I still need to adjust the docs once this is finalized.

[mokafolio]

@lehni Great! I like both, options.split or options.trace. I'll give the develop branch another shot for my overlay removal application and see how it goes soon.

Regarding CompoundPath and Group, I think there is a couple of decent approaches:

1. Either only return a CompoundPath when absolutely needed (i.e. due to holes etc.) and use Group for everything else. This would give you some indirect information about what the boolean operation did to your input path.

2. Always return CompoundPath. This has the advantage that you don't have to do any extra logic for handling Groups at all (which makes writing recursive functions which my overlay removal app heavily relies on a lot easier).

Overall I think approach 2 is the better way to go.

[lehni]

I like approach 2 better too. I wonder what we should do about divide()... at the moment, it returns a Group of both bits, so you know what was divided. If your original path is already a compound-path, and you're flattening the result into a new compound path, then you would loose that information? hmmm...

[lehni]

@mokafolio what are your thoughts on options.stroke? can't decide what's better / clearer...

[mokafolio]

@lehni I think I like options.trace or options.split better because stroke is somewhat ambiguous with all the other uses of stroke in the realm of vector graphics.

For the divide question, I just don't see a whole lot of reasons where that extra information would be relevant? That said, I think maybe by default always return a CompoundPath and if you care about that extra information and want a group of the resulting items allow to pass in another option for that.

[lehni]

Actually, since divide() is defined in this way, you might as well call them separately if you need that information:

return new Group([
    path1.subtract(path2),
    path1.intersect(path2)
]);

But the problem will be that we're breaking backward compatibility...

[mokafolio]

Good call.

About backwards compatibility, didn't we already break it multiple times? Boolean operations in the master/bower branch return other things than the current develop branch, so I don't think that should stop us at this point.

[lehni]

It hasn't stopped us before, so why now : )

[lehni]

@mokafolio I've implemented this now, and there was a nice simplification in a43db84

I think we're done : )

[lehni]

@niravdhanorkar with the latest prebuilt version, you can now pass { trace: false } to the subtract(), intersect() and divide() boolean operations to get the desired behavior, e.g.:

var path1 = PathItem.create("M534,273C171.7,111,60.5,117.1,30,158c-40.5,54.3,31.5,210.2,111,222c60.8,9,88-71.9,159-66c81.6,6.8,99.6,118.3,179,128c33.8,4.1,83.1-9.7,150-90")
var path2 = new Path.Rectangle({
    point: [150, 85],
    size: [287, 167]
});

path1.strokeColor = 'red';
path2.strokeColor = 'green';

var res = path1.subtract(path2, { trace: false });

res.position += [0, 300];

[niravdhanorkar]

@lehni
item.divide(path,{stroke:true}); works for me. usgin latest prebuilt version.

Thanks

[lehni]

@niravdhanorkar please use trace instead of stroke, as stroke is deprecated now.

[niravdhanorkar]

Hi @lehni ,
I had used it
item.divide(path,{stroke:true});
now for some kit (SVG on canvas). it doesn't give right division (compound path).
Please help as early as possible.

e.g.
var division = item.divide(path,{stroke:true});
console.log(division.children.length);

Sometimes I get 4 children, and sometimes 3 children.
4 is correct but when 3 comes it breaks my output.