Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework mask #2110

Closed
wants to merge 11 commits into from
Closed

Rework mask #2110

wants to merge 11 commits into from

Conversation

rowanwins
Copy link
Member

This PR reworks the mask module to be much simpler.

It's a bit slower on some of the tests (like surprisingly so) although I haven't quite dug into why deeply, it's most likely something to do with the union operation being called when it probably doesn't need to be.

This work was originally included in my v7 branch.

Resolves #1944
Resolves #1917
Resolves #2054
Resolves #1454

Before

basic x 2,599,035 ops/sec ±0.45% (93 runs sampled)
mask-outside x 2,445,808 ops/sec ±1.50% (86 runs sampled)
multi-polygon x 5,336 ops/sec ±1.80% (84 runs sampled)
overlapping x 15,862 ops/sec ±0.98% (93 runs sampled)

After

basic x 4,627 ops/sec ±25.23% (21 runs sampled)
mask-outside x 3,892 ops/sec ±34.80% (15 runs sampled)
multi-polygon x 5,837 ops/sec ±3.03% (91 runs sampled)
overlapping x 22,326 ops/sec ±1.34% (90 runs sampled)

@mfedderly
Copy link
Collaborator

Can you rework this PR to remove the commits from the release? its hard to examine the diff. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants