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
support evenodd fill rule #762
Conversation
Do you think that you could add in a small test as well? I would appreciate it a lot, otherwise I'll try and do it myself. It could just draw some black and white shape that would cause a specific pixel to be black when using "nonzero" and white when using "evenodd", or vice versa. Also, do you think that it makes sense to merge this now, and later add support to |
Well, both clip and isPointInPath has an optional parameter 'fillRule'. I will trying to implement them with some tests then. |
Thank you! |
@LinusU done |
If I'm not misstaken, the What do you think about always calling out This function could then simply be called in all three functions with e.g. I think that this approach would be very nice 👍 |
The index of fillRule can be 0 or 1, so |
The changes to |
@LinusU From https://html.spec.whatwg.org/multipage/scripting.html#drawing-paths-to-the-canvas, I can't find the description about below:
Could you help to verify? |
ctxSaved = true; | ||
context->setFillRule(rule); | ||
} | ||
cairo_save(ctx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need cairo_save
and cairo_restore
here?
I meant that the code currently did that, and that it was a bug :) I still think is the case though, could you add a test that tries doing first with evenodd, and then without a rule... |
So for code following: ctx.rect(0, 0, 100, 50);
ctx.rect(50, 0, 50, 100);
ctx.fill('evenodd');
console.log(ctx.isPointInPath(70, 10, 'evenodd')); // 1
console.log(ctx.isPointInPath(70, 10)); // 2 Do you mean that
But I test it at https://codepen.io/anon/pen/PNMLqe, the result is
|
I would like it to be |
Test added. I have consider that And if I remove |
Then you need to add If you make a similar test for e.g. |
@LinusU You are right. |
Looks awesome 🙌 I will merge when I get home |
Landed manually, sorry for the delay, thank you for your contribution 🙌 |
faae57a v1.4.0 |
fix #384