Patterns that have some scale patternTransform specified are not displayed correctly when applied on elements that are too small. They are displayed correctly on bigger element. (see attachment). The same SVG file displays correctly for all element sizes in Firefox, Batik and Adobe IE SVG plugin.
Created attachment 29911 [details] svg file with a pattern with a scale patternTransform - not correctly displayed in webkit r42962
Created attachment 29912 [details] screenshot with webkit r42962
SVGResourcePattern builds the pattern at runtime and calls SVGPatternElement::buildPattern(FloatRect), where FloatRect is the objectBoundingBox of the object, that is filled with the pattern. buildPattern() clips the patternimage to object size, if the object is smaler than the pattern. I think this clipping is wrong in this place. I wonder why we don't have a test for this.
Created attachment 29993 [details] SVG pattern size bigger than target size and scaled by patternTransformation I can't create a result of the test case. I don't have access to a Mac at the moment.
Comment on attachment 29993 [details] SVG pattern size bigger than target size and scaled by patternTransformation I don't remember what the clipping is for? Do you know?
I wonder if the clipping is trying to allow us to render really really large patterns in a small object w/o having to allocate a huge pattern buffer? As in, if the pattern tile is displayed less than once..
Created attachment 30317 [details] SVG pattern size bigger than target size + patternTransformation I still believe that deleting this code is better. patterTransform contains skewing and rotating. It makes it realy difficult up to impossible to be sure that our optimisation can handle all combinations of patterTransform. It would need a lot of logic. I added two test cases. One for the problem itself and one for a pattern with an excessive malloc.
Maybe the clipping optimization could be kept if there is no patternTransformation at all?
Comment on attachment 30317 [details] SVG pattern size bigger than target size + patternTransformation What I'm interested in is a test which "passes" before you remove the clips, but "fails" now that they are removed. I want to understand the behavior change via a test. Also your excessive-malloc values are large enough that they're wrapping negative. console.log is showing parseMappedAttribute complaing about negative values, which is not the code path you're trying to test, I think. :)
The console errors I see when loading your excessive-malloc test case are: Error: A negative value for pattern attribute <width> is not allowed Error: A negative value for pattern attribute <height> is not allowed Error: A negative value for rect <width> is not allowed Error: A negative value for rect <height> is not allowed
Created attachment 30318 [details] SVG pattern size bigger than target size + patternTransformation Added a third test to with patternTransform="skewX(45)" that also shows the problem and fixed the malloc test.
(In reply to comment #8) > Maybe the clipping optimization could be kept if there is no > patternTransformation at all? Sorry, haven't seen your comment. Yes and no. It should work but it doesn't like expected. The code doesnt' respect patternUnits. If you move your object, it can happen that the pattern is cut on the wrong place. Short example: <?xml version="1.0" encoding="UTF-8"?> <svg xmlns="http://www.w3.org/2000/svg"> <defs> <pattern id="pattern" height="200" width="200" patternUnits="userSpaceOnUse"> <rect x="0" y="0" width="100" height="200" fill="blue" /> <rect x="60" y="0" width="100" height="200" fill="red" /> </pattern> </defs> <rect x="10" y="140" width="100" height="100" fill="url(#pattern)" /> </svg> I don't use patternTransform, but the calculation of the cutting is still wrong.
Comment on attachment 30318 [details] SVG pattern size bigger than target size + patternTransformation This is an improvement over our previous behavior, even if it "breaks" extra-large patterns which "worked" before.
Comment on attachment 30318 [details] SVG pattern size bigger than target size + patternTransformation These tests change with this patch: svg/W3C-SVG-1.1/coords-units-01-b.svg expected actual diff pretty diff svg/W3C-SVG-1.1/pservers-grad-03-b.svg expected actual diff pretty diff svg/W3C-SVG-1.1/pservers-grad-06-b.svg expected actual diff pretty diff svg/W3C-SVG-1.1/pservers-pattern-01-b.svg expected actual diff pretty diff svg/batik/paints/patternPreserveAspectRatioA.svg expected actual diff pretty diff svg/batik/paints/patternRegions.svg expected actual diff pretty diff svg/custom/deep-dynamic-updates.svg expected actual diff pretty diff svg/custom/js-late-pattern-and-object-creation.svg expected actual diff pretty diff svg/custom/js-late-pattern-creation.svg expected actual diff pretty diff svg/custom/js-update-pattern-child.svg expected actual diff pretty diff svg/custom/js-update-pattern.svg expected actual diff pretty diff svg/custom/pattern-cycle-detection.svg expected actual diff pretty diff svg/custom/pattern-deep-referencing.svg expected actual diff pretty diff svg/custom/pattern-in-defs.svg expected actual diff pretty diff svg/custom/pattern-with-transformation.svg expected actual diff pretty diff svg/custom/pattern-y-offset.svg expected actual diff pretty diff svg/custom/pointer-events-path.svg expected image image diffs 0.60% svg/custom/stroked-pattern.svg expected actual diff pretty diff I was not comfortable making the changes on my own.
The failing pixel test has nothing to do with our pattern patch. The other tests fail because LayoutTests display the values of pattternBoundaries and we don't modify the patternBoundaries now (like the clipping code did). I think it's the best to update the tests. The clipping is not only wrong for the use with patternTransform, userSpaceOnUse, but for objectBoundingBox too. Short example: <pattern id="pattern" height="100%" width="110%"> The Clipping would cut of the patter size at 100% width. I still believe it's the best to delete the code and not to add a new but also wrong code and just update the LayoutTests.
Can the patch be checked-in or does it need more review? You can see this bug in a real application here: http://scrapcoloring.com I would love to be able to recommend Webkit as much as Firefox. :-) Thanks a lot for your work, Stephane
Any resolution on this? Can Dirk's patch be checked-in? https://bugs.webkit.org/attachment.cgi?id=30318
(In reply to comment #17) > Any resolution on this? Can Dirk's patch be checked-in? > https://bugs.webkit.org/attachment.cgi?id=30318 I checked the patch with trunk. I can confirm the failing tests eric mentioned. The pixel tests are still passing. I check if they just need an update.
Created attachment 47452 [details] pattern transform fix The current pattern sizes in some DRT test do not relate to the real pattern sizes. This is fixed aft the patch and the results got an update.
Comment on attachment 47452 [details] pattern transform fix reseting the review flag. We might have a bug in DRT and schould fix this first.
Created attachment 48366 [details] pattern transform fix updated to trunk
Talked with Niko on IRC and he reviewed it. The Changes on DRT are covered by another bug: DRT ans Resources https://bugs.webkit.org/show_bug.cgi?id=33105 landed in 54560