Safari is failing 2d.path.stroke.prune.arc
Created attachment 61566 [details] Patch
Comment on attachment 61566 [details] Patch > + if ((p1 == m_path.currentPoint()) || (p1 == p2) || !r) No need for the extra parentheses here since == binds more tightly than || and it's customary to not include these. > Index: WebCore/platform/graphics/cg/PathCG.cpp > =================================================================== > --- WebCore/platform/graphics/cg/PathCG.cpp (revision 63275) > +++ WebCore/platform/graphics/cg/PathCG.cpp (working copy) > @@ -249,6 +249,11 @@ bool Path::hasCurrentPoint() const > { > return !isEmpty(); > } > + > +FloatPoint Path::currentPoint() const > +{ > + return CGPathGetCurrentPoint(m_path); > +} I believe adding this for CG only will break the build for all platforms that don’t use CG. That’s probably why the EWS failed on Qt. So you’ll need to add this for other platforms as well. review- because we need this to at least compile on the non-CG platforms
Attachment 61566 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3447295
Created attachment 61589 [details] Patch
Comment on attachment 61589 [details] Patch > +FloatPoint Path::currentPoint() const > +{ > + // FIXME: implement safe way to return current point of subpath. > + return 0; > +} I don’t think this will compile. There’s no way to convert the number 0 into a FloatPoint. You probably need to return something like FloatPoint(NAN, NAN). You should test by actually compiling the code, perhaps in PathCG.cpp. Not sure what "safe" means in the comment.
Created attachment 61592 [details] Patch
Comment on attachment 61592 [details] Patch Come to think of it, I’m not sure NAN works on all the platforms. We might need to use "numeric_limits<float>::quiet_NaN()" instead. The reason I suggested NAN is that is safe. The optimization won't do any harm if you return a point that can never be equal.
Good stuff :-) The Qt implementation of Path::currentPoint() would be: FloatPoint Path::currentPoint() const { return m_path.currentPosition(); }
The cairo equivelent is probably. FloatPoint Path::currentPoint() const { double x; double y; cairo_get_current_point(platformPath()->m_cr, x, y) return FloatPoint(x, y); }
Created attachment 61839 [details] Patch
Comment on attachment 61839 [details] Patch r=me
Attachment 61839 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/3396494
Created attachment 61847 [details] Previous + Cairo fix Ooops on the Cairo impl slip. This should be good. I checked their code use below to double/triple check.
Comment on attachment 61847 [details] Previous + Cairo fix Clearing flags on attachment: 61847 Committed r63599: <http://trac.webkit.org/changeset/63599>
All reviewed patches have been landed. Closing bug.