Bug 142144

Summary: [Cairo] Implement Path::addEllipse
Product: WebKit Reporter: Hunseop Jeong <hs85.jeong>
Component: CanvasAssignee: Hunseop Jeong <hs85.jeong>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, commit-queue, d-r, gyuyoung.kim, mrobinson, ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Hunseop Jeong 2015-03-01 07:21:50 PST 
Add support for addEllipse method for platforms using cairo.
Comment 1 Hunseop Jeong 2015-03-01 07:25:23 PST 
Created attachment 247628 [details]
Patch
Comment 2 Hunseop Jeong 2015-03-01 07:29:44 PST 
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 ]
Comment 3 Hunseop Jeong 2015-03-01 07:31:27 PST 
Created attachment 247629 [details]
Patch
Comment 4 Gyuyoung Kim 2015-03-01 16:55:17 PST 
(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.
Comment 5 Gyuyoung Kim 2015-03-01 16:55:38 PST 
CC'ing Martin.
Comment 6 Martin Robinson 2015-03-01 17:00:16 PST 
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.
Comment 7 Martin Robinson 2015-03-01 17:00:40 PST 
(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. :)
Comment 8 Hunseop Jeong 2015-03-01 17:04:05 PST 
(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
Comment 9 Gyuyoung Kim 2015-03-01 17:07:50 PST 
(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.
Comment 10 Hunseop Jeong 2015-03-01 17:08:54 PST 
Created attachment 247644 [details]
Patch
Comment 11 Hunseop Jeong 2015-03-01 17:22:35 PST 
(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.
Comment 12 Gyuyoung Kim 2015-03-01 17:23:40 PST 
Comment on attachment 247644 [details]
Patch

r+ed based on Martin's review.
Comment 13 WebKit Commit Bot 2015-03-01 20:33:28 PST 
Comment on attachment 247644 [details]
Patch

Clearing flags on attachment: 247644

Committed r180881: <http://trac.webkit.org/changeset/180881>
Comment 14 WebKit Commit Bot 2015-03-01 20:33:34 PST 
All reviewed patches have been landed.  Closing bug.