PatternSkia should respect the local transformation in createPlatformPattern and apply the local transformation to shader. This patch fixes the following layout tests in chromium: LayoutTests/svg/batik/paints/patternRegionA.svg LayoutTests/svg/W3C-SVG-1.1/pservers-grad-06-b.svg LayoutTests/svg/W3C-SVG-1.1/pservers-pattern-01-b.svg LayoutTests/svg/W3C-SVG-1.1/coords-units-01-b.svg Note that Skia is bad about having positive translation in tile mode. The code takes special care of that. See the code for more details. Review in chromium is here: http://codereview.chromium.org/42100/show
Created attachment 28509 [details] Patch to patternSkia.cpp
Created attachment 28511 [details] patch + change log
Brett, can you look at this change?
I would like to let the Skia folks know about this before we work around something. If it is a bug, it should be fixed there, or we should know a fix is pending before checking in. I'll forward it.
(In reply to comment #4) > I would like to let the Skia folks know about this before we work around > something. If it is a bug, it should be fixed there, or we should know a fix is > pending before checking in. I'll forward it. > Any updates from skia people?
There is maybe another way to support transformations for patterns. On CG and Qt we transform the context right before we stroke/fill anything with a pattern, the path should not be affected. Just look at fillPath/strokePath in GraphicsContextCg.cpp or GraphicsContextQt.cpp to know what I mean. Could that avoid this hack at the end of the pattern-patch? To round and cut off some values seems to be incorrect for me.
Comment on attachment 28511 [details] patch + change log r- based on Dirk's comments. Mostly I can't quite tell if this is right, Dirk is questioning the correctness, and we're trying to clear out the review queue one. Feel free to mark this r=? with answers to Dirk's questions.
No progress on this bug? :-(
Created attachment 47643 [details] New fix The good news is, the Skia bug should be fixed, so the hack part of Alpha's fix (above) should no longer be necessary. The bad news is, I changed the allocation of SkShaders a few months back to be done once on first draw, so it was necessary to rework Patterns a bit so that the m_patternSpaceTransform could be forwarded to the SkShader.
Created attachment 47644 [details] ChangeLog and non-Skia fixes
Attachment 47644 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/217145
Created attachment 47652 [details] Same as above, plus QT fix
Comment on attachment 47652 [details] Same as above, plus QT fix I would have probably put the "else" into its own function, but OK.
Comment on attachment 47652 [details] Same as above, plus QT fix Clearing flags on attachment: 47652 Committed r54203: <http://trac.webkit.org/changeset/54203>
All reviewed patches have been landed. Closing bug.