Bug 27292 - Improve handling of <canvas> path operations on an empty path.
Summary: Improve handling of <canvas> path operations on an empty path.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-15 02:22 PDT by Dean McNamee
Modified: 2009-07-16 08:10 PDT (History)
2 users (show)

See Also:


Attachments
Patch (6.56 KB, patch)
2009-07-15 02:47 PDT, Dean McNamee
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean McNamee 2009-07-15 02:22:36 PDT
There was originally some work on this:

https://bugs.webkit.org/show_bug.cgi?id=27187

This patch further does a few more things:
  - Don't emit a line or curve, just set the current point (moveTo).
  - For curves, emit the end point and not the control point.
  - Update the Skia backend to implement hasCurrentPoint.
Comment 1 Dean McNamee 2009-07-15 02:47:50 PDT
Created attachment 32772 [details]
Patch
Comment 2 Oliver Hunt 2009-07-15 02:51:53 PDT
Comment on attachment 32772 [details]
Patch

In general this seems okay to me -- how does the output compare to firefox?
Comment 3 Dean McNamee 2009-07-15 03:04:05 PDT
Differs from Firefox, looks like the control point vs endpoint, but I really don't see how control point could ever make any sense.  I'll look at their code today, and maybe file a bug with them.

(In reply to comment #2)
> (From update of attachment 32772 [details])
> In general this seems okay to me -- how does the output compare to firefox?
Comment 4 Dean McNamee 2009-07-15 03:10:04 PDT
(In reply to comment #3)
> Differs from Firefox, looks like the control point vs endpoint, but I really
> don't see how control point could ever make any sense.  I'll look at their code
> today, and maybe file a bug with them.
> 
> (In reply to comment #2)
> > (From update of attachment 32772 [details] [details])
> > In general this seems okay to me -- how does the output compare to firefox?

Ok, so Firefox just passes this directly to cairo:

162 void
163 gfxContext::LineTo(const gfxPoint& pt)
164 {
165     cairo_line_to(mCairo, pt.x, pt.y);
166 }

And Cairo has the control point behavior:

http://cairographics.org/manual/cairo-paths.html#cairo-curve-to

If there is no current point before the call to cairo_curve_to() this function will behave as if preceded by a call to cairo_move_to(cr, x1, y1).

I think they should cairo_has_current_point() and cairo_move_to the endpoints if not.  I'll file a bug with Mozilla.  I wonder why Cairo has this behavior.
Comment 5 Dean McNamee 2009-07-15 03:20:01 PDT
Discussion of the cairo behavior is here:

http://lists.freedesktop.org/archives/cairo/2008-June/014262.html

I guess the reason cairo behaves like it does, is because it still wants to emit a curve.  I think the current behavior for browser should be to not emit anything, and just do the moveTo.  I will follow up with Mozilla.

(In reply to comment #4)
> (In reply to comment #3)
> > Differs from Firefox, looks like the control point vs endpoint, but I really
> > don't see how control point could ever make any sense.  I'll look at their code
> > today, and maybe file a bug with them.
> > 
> > (In reply to comment #2)
> > > (From update of attachment 32772 [details] [details] [details])
> > > In general this seems okay to me -- how does the output compare to firefox?
> 
> Ok, so Firefox just passes this directly to cairo:
> 
> 162 void
> 163 gfxContext::LineTo(const gfxPoint& pt)
> 164 {
> 165     cairo_line_to(mCairo, pt.x, pt.y);
> 166 }
> 
> And Cairo has the control point behavior:
> 
> http://cairographics.org/manual/cairo-paths.html#cairo-curve-to
> 
> If there is no current point before the call to cairo_curve_to() this function
> will behave as if preceded by a call to cairo_move_to(cr, x1, y1).
> 
> I think they should cairo_has_current_point() and cairo_move_to the endpoints
> if not.  I'll file a bug with Mozilla.  I wonder why Cairo has this behavior.
Comment 6 Dimitri Glazkov (Google) 2009-07-16 08:10:40 PDT
Landed as http://trac.webkit.org/changeset/45973.