Last Comment Bug 691646 - Pattern tile pixelated on patternTransform
: Pattern tile pixelated on patternTransform
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla10
Assigned To: Robert Longson
: Jet Villegas (:jet)
Depends on: 780880
Blocks: 697057
  Show dependency treegraph
Reported: 2011-10-03 19:55 PDT by Robert Longson
Modified: 2012-08-07 10:21 PDT (History)
4 users (show)
longsonr: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (2.80 KB, patch)
2011-10-03 19:55 PDT, Robert Longson
dholbert: review+
Details | Diff | Splinter Review
address comments (3.05 KB, patch)
2011-10-04 00:08 PDT, Robert Longson
no flags Details | Diff | Splinter Review
try to fix reftest failures (3.23 KB, patch)
2011-10-04 03:12 PDT, Robert Longson
no flags Details | Diff | Splinter Review

Description Robert Longson 2011-10-03 19:55:26 PDT
Created attachment 564442 [details] [diff] [review]
Comment 1 Robert Longson 2011-10-03 20:17:07 PDT
I've just realised the patch should be fabs(patternMatrix->xx) and fabs(patternMatrix->yy) in case someone has written scale(-40) which should invert the image as well as scaling it. The patch as written would not render scale(-40).
Comment 2 Daniel Holbert [:dholbert] 2011-10-03 22:29:35 PDT
Comment on attachment 564442 [details] [diff] [review]

Could you add a reftest for the situation described in comment 1?  (with inverting coming into play)  r=me with that.
Comment 3 Robert Longson 2011-10-04 00:08:43 PDT
Created attachment 564471 [details] [diff] [review]
address comments
Comment 5 Robert Longson 2011-10-04 03:01:32 PDT
And backed out as the reftest fails on Linux.
Comment 6 Robert Longson 2011-10-04 03:12:11 PDT
Created attachment 564502 [details] [diff] [review]
try to fix reftest failures
Comment 7 Mozilla RelEng Bot 2011-10-04 06:51:05 PDT
Try run for 696085573f75 is complete.
Detailed breakdown of the results available here:
Results (out of 9 total builds):
    success: 4
    warnings: 5
Builds available at
Comment 8 Mozilla RelEng Bot 2011-10-04 08:21:37 PDT
Try run for 6b114ae3b4c4 is complete.
Detailed breakdown of the results available here:
Results (out of 9 total builds):
    success: 9
Builds available at
Comment 9 Robert Longson 2011-10-04 08:24:07 PDT
fixed reftest and landed as
Comment 10 Robert Longson 2011-10-04 08:43:54 PDT
antialiasing sucks when you're writing reftests :-(
Comment 12 Marco Bonardo [::mak] 2011-10-05 05:03:53 PDT
Comment 13 Daniel Holbert [:dholbert] 2011-10-25 13:20:18 PDT
Looking back on this, I think we should have used patternMatrix->GetScaleFactors() to get the scale factors, instead of using patternMatrix->xx / patternMatrix->yy.  (assuming patternMatrix is non-singular)  Otherwise, I think rotations / skews could mess us up here.

Robert: am I missing something, or does that make sense?  I'm happy to file a followup on that, just wanted to sanity-check here first.
Comment 14 Robert Longson 2011-10-25 14:26:49 PDT
Comment 15 Daniel Holbert [:dholbert] 2011-10-25 14:49:28 PDT
Yup, sorry for getting the wrong name there.

Actually I'm getting less sure about ScaleFactors, though, from looking at the impl... I'm not sure we'd always be able to pass the right value of "xMajor". (the existing code also only has 2 calls to ScaleFactors, with 'true' in both places, but I don't think that's necessarily right in this spot)

So, I think I rescind comment 13 (at least partly)... sorry for my confusion on this.

Note You need to log in before you can comment on or make changes to this bug.