These files have gotten out of sync and I suspect they are currently quite buggy. The original chromium changelists that caused this were http://codereview.chromium.org/17633 and http://codereview.chromium.org/17454 .
Created attachment 27511 [details] patch
Comment on attachment 27511 [details] patch Looks fine + // Current path in global coordinates. isn't actually right, is it? If you change the current matrix, we don't update the path. So won't currentPath() do the wrong thing? Also WebKit style has { +SkPath PlatformContextSkia::currentPath() const { on its own line. r- for the concerns about currentPath() doing the wrong thing if you have changed the current transform since setting the path.
(In reply to comment #2) > (From update of attachment 27511 [details] [review]) > Looks fine > > + // Current path in global coordinates. > > isn't actually right, is it? > > If you change the current matrix, we don't update the path. So won't > currentPath() do the wrong thing? > > Also WebKit style has { > +SkPath PlatformContextSkia::currentPath() const { > on its own line. > > r- for the concerns about currentPath() doing the wrong thing if you have > changed the current transform since setting the path. > That is actually what this patch is fixing. The path is only supposed to be transformed at the time when it is added. That is, we *do* want these two code paths to do different things: * context->addPathPoints(a) * context->appendTransform(t) * context->addPathPoints(b) ========== vs. =========== * context->appendTransform(t) * context->addPathPoints(a) * context->addPathPoints(b)
Created attachment 27669 [details] fix curly brace
Comment on attachment 27669 [details] fix curly brace I would name currentPath() to currentPathInLocalCoordinates() then you don't need the comment and all the callsites are clear. I would add a comment next to m_path explaining how it's stored transformed to global coordinates *at time of setting* and that currentPathInLocalCoordinates applies the inverse globalTransform at time of getting so that getCTM().transform(currerntPathInLocalCoordinates()) works. WebKit uses localPath instead of local_path and inverseMatrix instead of inverse_matrix. r- for the style issues.
Created attachment 27760 [details] try3 changed currentPath() to currentPathInLocalCoordinates(), fixed local variable naming style issues
Comment on attachment 27760 [details] try3 r=me
Landed as http://trac.webkit.org/changeset/41064