932500 - PathD2D::TransformedCopyToBuilder ignores even odd fill

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Rik Cabanier]

I noticed that TransformedCopyToBuilder with even odd winding returns a path that is still using nzo winding.
This looks like a small oversight in the TransformedCopyToBuilder function.

Comment 1

[Rik Cabanier]

Attachment: Add case for even odd winding

Comment 2

[Rik Cabanier]

try server: https://tbpl.mozilla.org/?tree=Try&rev=017f7e3dc9ab

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 824268 [details] [diff] [review]
Add case for even odd winding

Review of attachment 824268 [details] [diff] [review]:
-----------------------------------------------------------------

The default is ALTERNATE. Does this patch have a behaviour change?

Comment 4

[Rik Cabanier]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> Comment on attachment 824268 [details] [diff] [review]
> Add case for even odd winding
> 
> Review of attachment 824268 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The default is ALTERNATE. Does this patch have a behaviour change?

Well, it fixes when you go from D2D1_FILL_MODE_ALTERNATE to D2D1_FILL_MODE_WINDING which was ignored before.
I could add an assert that checks if the result of TransformedCopyToBuilder doesn't give the correct winding back.

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 824268 [details] [diff] [review]
Add case for even odd winding

Review of attachment 824268 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/PathD2D.cpp
@@ +268,5 @@
>  
>    if (aFillRule == FILL_WINDING) {
>      sink->SetFillMode(D2D1_FILL_MODE_WINDING);
> +  } else {
> +    sink->SetFillMode(D2D1_FILL_MODE_ALTERNATE);

This is unneeded as ALTERNATE is the default for a new sink. We should probably just remove this line.

@@ +282,5 @@
>                          D2DMatrix(aTransform),
>                          sink);
>    }
>  
> +  RefPtr<PathBuilderD2D> pathBuilder = new PathBuilderD2D(sink, path, aFillRule);

Good catch.

Comment 6

[Rik Cabanier]

Attachment: Add case for even odd winding

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0308e23bc0ab

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0308e23bc0ab