Bug 142144 - [Cairo] Implement Path::addEllipse
Summary: [Cairo] Implement Path::addEllipse
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hunseop Jeong
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-01 07:21 PST by Hunseop Jeong
Modified: 2015-03-01 20:33 PST (History)
6 users (show)

See Also:

Attachments
Patch (1.67 KB, patch)
2015-03-01 07:25 PST, Hunseop Jeong 		no flags Details | Formatted Diff | Diff
Patch (1.85 KB, patch)
2015-03-01 07:31 PST, Hunseop Jeong 		no flags Details | Formatted Diff | Diff
Patch (1.84 KB, patch)
2015-03-01 17:08 PST, Hunseop Jeong 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.