Bug 142144 – [Cairo] Implement Path::addEllipse

• Component: Canvas
• Status: RESOLVED FIXED

[Hunseop Jeong]

Add support for addEllipse method for platforms using cairo.

[Hunseop Jeong]

Created attachment 247628 [details]
Patch

[Hunseop Jeong]

Pass the below tests after applying this patch on EFL, GTK+ port.
  fast/canvas/canvas-ellipse-360-winding.html [ Failure ]
  fast/canvas/canvas-ellipse-zero-lineto.html [ Failure ]
  fast/canvas/canvas-ellipse.html [ Failure ]
  fast/canvas/canvas-ellipse-connecting-line.html [ ImageOnlyFailure ]

[Hunseop Jeong]

Created attachment 247629 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #2)
> Pass the below tests after applying this patch on EFL, GTK+ port.
>   fast/canvas/canvas-ellipse-360-winding.html [ Failure ]
>   fast/canvas/canvas-ellipse-zero-lineto.html [ Failure ]
>   fast/canvas/canvas-ellipse.html [ Failure ]
>   fast/canvas/canvas-ellipse-connecting-line.html [ ImageOnlyFailure ]

Please unskip the passing tests with this patch.

[Gyuyoung Kim]

CC'ing Martin.

[Martin Robinson]

Comment on attachment 247629 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247629&action=review

> Source/WebCore/platform/graphics/cairo/PathCairo.cpp:287
> +void Path::addEllipse(FloatPoint p, float radiusX, float radiusY, float rotation, float startAngle, float endAngle, bool anticlockwise)

In WebKit we try to use full words for variable names, so p should be point, otherwise this looks good to me.

[Martin Robinson]

(In reply to comment #4)
> (In reply to comment #2)
> > Pass the below tests after applying this patch on EFL, GTK+ port.
> >   fast/canvas/canvas-ellipse-360-winding.html [ Failure ]
> >   fast/canvas/canvas-ellipse-zero-lineto.html [ Failure ]
> >   fast/canvas/canvas-ellipse.html [ Failure ]
> >   fast/canvas/canvas-ellipse-connecting-line.html [ ImageOnlyFailure ]
> 
> Please unskip the passing tests with this patch.

I definitely agree with Gyuyoung here. :)

[Hunseop Jeong]

(In reply to comment #4)
> (In reply to comment #2)
> > Pass the below tests after applying this patch on EFL, GTK+ port.
> >   fast/canvas/canvas-ellipse-360-winding.html [ Failure ]
> >   fast/canvas/canvas-ellipse-zero-lineto.html [ Failure ]
> >   fast/canvas/canvas-ellipse.html [ Failure ]
> >   fast/canvas/canvas-ellipse-connecting-line.html [ ImageOnlyFailure ]
> 
> Please unskip the passing tests with this patch.

That tests was added after 180790.

Already failed on EFL bot.
https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/20114/steps/layout-test/logs/stdio

[Gyuyoung Kim]

(In reply to comment #8)
> (In reply to comment #4)
> > (In reply to comment #2)
> > > Pass the below tests after applying this patch on EFL, GTK+ port.
> > >   fast/canvas/canvas-ellipse-360-winding.html [ Failure ]
> > >   fast/canvas/canvas-ellipse-zero-lineto.html [ Failure ]
> > >   fast/canvas/canvas-ellipse.html [ Failure ]
> > >   fast/canvas/canvas-ellipse-connecting-line.html [ ImageOnlyFailure ]
> > 
> > Please unskip the passing tests with this patch.
> 
> That tests was added after 180790.
> 
> Already failed on EFL bot.
> https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/
> builds/20114/steps/layout-test/logs/stdio

Do you mean the tests aren't gardening yet both on EFL and GTK ports ? If so, please change the argument name.

[Hunseop Jeong]

Created attachment 247644 [details]
Patch

[Hunseop Jeong]

(In reply to comment #10)
> Created attachment 247644 [details]
> Patch

Yes, (In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #4)
> > > (In reply to comment #2)
> > > > Pass the below tests after applying this patch on EFL, GTK+ port.
> > > >   fast/canvas/canvas-ellipse-360-winding.html [ Failure ]
> > > >   fast/canvas/canvas-ellipse-zero-lineto.html [ Failure ]
> > > >   fast/canvas/canvas-ellipse.html [ Failure ]
> > > >   fast/canvas/canvas-ellipse-connecting-line.html [ ImageOnlyFailure ]
> > > 
> > > Please unskip the passing tests with this patch.
> > 
> > That tests was added after 180790.
> > 
> > Already failed on EFL bot.
> > https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/
> > builds/20114/steps/layout-test/logs/stdio
> 
> Do you mean the tests aren't gardening yet both on EFL and GTK ports ? If
> so, please change the argument name.

Yes. You are right. 
I changed the argument name.

[Gyuyoung Kim]

Comment on attachment 247644 [details]
Patch

r+ed based on Martin's review.

[WebKit Commit Bot]

Comment on attachment 247644 [details]
Patch

Clearing flags on attachment: 247644

Committed r180881: <http://trac.webkit.org/changeset/180881>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.