Last Comment Bug 784573 - [Azure] Cairo path transforms broken
: [Azure] Cairo path transforms broken
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla18
Assigned To: Anthony Jones (:kentuckyfriedtakahe, :k17e)
:
Mentors:
http://www.kevs3d.co.uk/dev/asteroids...
Depends on: 1124549
Blocks: 781371 803658
  Show dependency treegraph
 
Reported: 2012-08-21 19:36 PDT by Anthony Jones (:kentuckyfriedtakahe, :k17e)
Modified: 2015-01-21 21:53 PST (History)
4 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Part 1.5: Fix cairo path transforms (12.61 KB, patch)
2012-08-22 00:26 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review-
Details | Diff | Splinter Review
Part2: Fixed some test failures and did more clean up (13.50 KB, patch)
2012-08-22 21:51 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
no flags Details | Diff | Splinter Review
Part 1: reftest to make sure the problem stays fixed (2.01 KB, patch)
2012-08-22 21:52 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
no flags Details | Diff | Splinter Review
Part 2: Fixed some test failures and did more clean up v2 (13.25 KB, patch)
2012-08-22 21:57 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review-
Details | Diff | Splinter Review
Part 3: RAII matrix helper (1.93 KB, patch)
2012-08-22 22:01 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review+
Details | Diff | Splinter Review
Part 4: Added reftest and removed todo v2 (3.15 KB, patch)
2012-08-23 16:47 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review+
Details | Diff | Splinter Review
Ugly lines on the left of the canvas (373.75 KB, image/png)
2012-08-26 14:43 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
no flags Details
Part 0: Fix cairo path transforms v2 (18.36 KB, patch)
2012-08-26 22:54 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review-
Details | Diff | Splinter Review
Part 2: RAII matrix helper v2 (rebased) (2.35 KB, patch)
2012-08-26 23:08 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
no flags Details | Diff | Splinter Review
Part 3: Fix cairo path transforms v3 (18.87 KB, patch)
2012-08-27 16:43 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review-
Details | Diff | Splinter Review
Part 2: RAII matrix helper v3 (1.17 KB, patch)
2012-08-27 22:36 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review
Part 4: Added reftest and removed todo v3 (3.54 KB, patch)
2012-08-29 15:24 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review
Part 3: Fix cairo path transforms v4 (22.58 KB, patch)
2012-08-30 22:35 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
joe: review+
Details | Diff | Splinter Review
Part 3: Fix cairo path transforms v5 (22.88 KB, patch)
2012-09-02 15:26 PDT, Anthony Jones (:kentuckyfriedtakahe, :k17e)
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-21 19:36:18 PDT
Paths don't draw correctly using Azure/Cairo if they need to move between user space and device space. This shows up on asteroidbench test 4 as a bunch of unwanted lines across the canvas when drawing enemy bullets.
Comment 1 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-22 00:26:55 PDT
Created attachment 654120 [details] [diff] [review]
Part 1.5: Fix cairo path transforms
Comment 2 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-22 21:51:47 PDT
Created attachment 654512 [details] [diff] [review]
Part2: Fixed some test failures and did more clean up

It would probably make sense to just combine these patches.
Comment 3 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-22 21:52:47 PDT
Created attachment 654513 [details] [diff] [review]
Part 1: reftest to make sure the problem stays fixed
Comment 4 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-22 21:57:53 PDT
Created attachment 654514 [details] [diff] [review]
Part 2: Fixed some test failures and did more clean up v2
Comment 5 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-22 22:01:01 PDT
Created attachment 654515 [details] [diff] [review]
Part 3: RAII matrix helper
Comment 6 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-23 16:47:19 PDT
Created attachment 654845 [details] [diff] [review]
Part 4: Added reftest and removed todo v2
Comment 7 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-26 14:43:38 PDT
Created attachment 655458 [details]
Ugly lines on the left of the canvas
Comment 8 Joe Drew (not getting mail) 2012-08-26 18:17:46 PDT
Comment on attachment 654845 [details] [diff] [review]
Part 4: Added reftest and removed todo v2

Review of attachment 654845 [details] [diff] [review]:
-----------------------------------------------------------------

This should be part n+1 because it depends on the fixes in the previous n patches.

::: layout/reftests/canvas/784573-1.html
@@ +1,1 @@
> +<!DOCTYPE html>

Can you add a comment saying what this is testing (that is, device space vs user space paths). Also, <!-- Any copyright is dedicated to the public domain. -->
Comment 9 Joe Drew (not getting mail) 2012-08-26 18:34:12 PDT
Comment on attachment 654120 [details] [diff] [review]
Part 1.5: Fix cairo path transforms

Review of attachment 654120 [details] [diff] [review]:
-----------------------------------------------------------------

Overall, looks OK, but I need more explanation for why things are correct before I r+.

::: gfx/2d/PathCairo.cpp
@@ +25,5 @@
> +  cairo_new_path(mContext);
> +
> +  if (mDrawTarget) {
> +    mDrawTarget->SetPathObserver(this);
> +  }

How do we get around needing to have a separate context from the draw target, as below?

@@ +40,5 @@
> + , mFillRule(aPathContext->mFillRule)
> +{
> +  Matrix inverse = aTransform;
> +  inverse.Invert();
> +  mTransform = aPathContext->mTransform * inverse;

Can you please add a comment explaining why you're doing this?

@@ +48,3 @@
>  
>    // If we don't have an identity transformation, we need to have a separate
>    // context from the draw target, because we can't set a transformation on its

Shouldn't we always have to duplicate the context and path here? IIRC CairoPathContexts don't know how to share contexts between each other.

@@ +98,5 @@
>    cairo_fill_rule_t rule = cairo_get_fill_rule(mContext);
>  
>    // Duplicate the context.
>    cairo_surface_t* surf = cairo_get_target(mContext);
> +  cairo_matrix_t matrix; 

Extra space at the end of this line.

@@ +104,3 @@
>    cairo_destroy(mContext);
>    mContext = cairo_create(surf);
> +  cairo_set_matrix(mContext, &matrix);  

Also here.

@@ +263,5 @@
>  
>  TemporaryRef<Path>
>  PathBuilderCairo::Finish()
>  {
> +  RefPtr<PathCairo> path = new PathCairo(mPathContext);

We should really file a follow-up bug for the fact that this will duplicate the context unnecessarily.

::: gfx/2d/PathCairo.h
@@ +37,5 @@
>  class CairoPathContext : public RefCounted<CairoPathContext>
>  {
>  public:
>    // Construct a CairoPathContext and set it to be the path observer of
> +  // aDrawTarget with an empty path.

aDrawTarget. Also, modify its path in-place to be empty.

@@ +42,4 @@
>    CairoPathContext(cairo_t* aCtx, DrawTargetCairo* aDrawTarget,
> +                   FillRule aFillRule, const Matrix& aTransform);
> +
> +  // Create a CairoPathContext which is a cope of a previous context with

"copy"

@@ +42,5 @@
>    CairoPathContext(cairo_t* aCtx, DrawTargetCairo* aDrawTarget,
> +                   FillRule aFillRule, const Matrix& aTransform);
> +
> +  // Create a CairoPathContext which is a cope of a previous context with
> +  // applying a new transform.

"an optional transform"?

@@ +85,5 @@
>    operator cairo_t* () const { return mContext; }
>  
>  private: // methods
>    CairoPathContext(const CairoPathContext&) MOZ_DELETE;
> +  bool ClassInvariant();

this is never used
Comment 10 Joe Drew (not getting mail) 2012-08-26 18:41:15 PDT
Comment on attachment 654514 [details] [diff] [review]
Part 2: Fixed some test failures and did more clean up v2

Review of attachment 654514 [details] [diff] [review]:
-----------------------------------------------------------------

This patch looks like it would be improved by folding it with patch 1.5.

::: gfx/2d/PathCairo.cpp
@@ +23,5 @@
>    cairo_reference(mContext);
>    cairo_set_fill_rule(mContext, GfxFillRuleToCairoFillRule(mFillRule));
> +
> +  aDrawTarget->SetPathObserver(this);
> +  mDrawTarget = aDrawTarget;

It looks like you do this dance on purpose. Can you explain why in a comment?

@@ +250,5 @@
> +  cairo_get_matrix(*mPathContext, &saveMatrix);
> +
> +  cairo_matrix_t matrix;
> +  GfxMatrixToCairoMatrix(mPathContext->GetTransform(), matrix);
> +  cairo_set_matrix(*mPathContext, &matrix);

When do these get out of sync? Shouldn't this always be the case?

::: gfx/2d/PathCairo.h
@@ +43,2 @@
>  
> +  // Transform an existing path

These comments seem worse than the previous ones.

@@ +60,5 @@
>    // our context.
>    void ForgetDrawTarget();
>  
>    // Create a duplicate context, and copy this path to that context. Optionally,
>    // the new context can be transformed.

Update this comment.
Comment 11 Joe Drew (not getting mail) 2012-08-26 18:42:29 PDT
Comment on attachment 654515 [details] [diff] [review]
Part 3: RAII matrix helper

Review of attachment 654515 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/HelpersCairo.h
@@ +206,5 @@
>  
>    return CAIRO_FILL_RULE_WINDING;
>  }
>  
> +class CairoTempMatrix {

Can you document this class? Also put { open braces on their own line for the class and functions.
Comment 12 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-26 22:54:33 PDT
Created attachment 655515 [details] [diff] [review]
Part 0: Fix cairo path transforms v2
Comment 13 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-26 23:08:10 PDT
Created attachment 655524 [details] [diff] [review]
Part 2: RAII matrix helper v2 (rebased)
Comment 14 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-26 23:09:42 PDT
http://tbpl.mozilla.org/?tree=Try&rev=f4c7e27c718e
Comment 15 Joe Drew (not getting mail) 2012-08-27 11:14:37 PDT
Comment on attachment 655515 [details] [diff] [review]
Part 0: Fix cairo path transforms v2

Review of attachment 655515 [details] [diff] [review]:
-----------------------------------------------------------------

Some unanswered questions (some left over from previous reviews) lead me to still need to see a follow-up before r+.

::: gfx/2d/DrawTargetCairo.h
@@ +168,5 @@
>    cairo_t* mContext;
>    cairo_surface_t* mSurface;
>    IntSize mSize;
>    std::vector<SourceSurfaceCairo*> mSnapshots;
> +  mutable CairoPathContext* mPathObserver;

Ownership rules deserve a comment here or on SetPathObserver. Seeing a bare pointer makes my tummy hurt.

::: gfx/2d/PathCairo.cpp
@@ +22,5 @@
>  {
>    cairo_reference(mContext);
>    cairo_set_fill_rule(mContext, GfxFillRuleToCairoFillRule(mFillRule));
>  
> +  aDrawTarget->SetPathObserver(this);

Note: You didn't answer my earlier question, "How do we get around needing to have a separate context from the draw target, as below?", but I think I have figured it out: the non-copy CairoPathContext constructor is only ever called when creating a new path, correct?

@@ +35,5 @@
> + , mFillRule(aPathContext->mFillRule)
> +{
> +  cairo_reference(mContext);
> +
> +  // Canvas is telling us what transform we would apply from device space to

Don't talk about canvas here - it is one of the users of the DrawTarget API, but not the only one.

@@ +75,5 @@
>    cairo_path_destroy(path);
> +
> +  // Transform the context.
> +  GfxMatrixToCairoMatrix(mTransform, matrix);
> +  cairo_set_matrix(mContext, &matrix);

You set the cairo context's matrix to mContext's old matrix, and then set it to mTransform. Why? When do those two get out of sync?

@@ +86,5 @@
> +{
> +  if (mDrawTarget) {
> +    // null the draw target so that we don't try to copy the path when we get
> +    // a PathWillChange notification back.
> +    DrawTargetCairo* drawTarget = mDrawTarget;

Seems like most of this can just be elided, with some comments explaining it and an unconditional mDrawTarget = nullptr;

@@ +250,5 @@
>    inverse.Invert();
>    Point transformed = inverse * aPoint;
>  
> +  // ContainsPoint can be called after the DrawTarget has been notified about
> +  // a change in transform but before the path has been notified by Canvas. In

s/Canvas/DrawTarget/ perhaps? References to canvas don't really make sense in backend code. Perhaps s/Canvas/user code/?

@@ +300,5 @@
>  
>  void
>  PathCairo::CopyPathTo(cairo_t* aContext, DrawTargetCairo* aDrawTarget)
>  {
> +  mPathContext->CopyPathTo(aContext);

Why do you do this unconditionally? Was the previous optimization incorrect? And, if mPathContext->GetContext() == aContext, isn't this going to do useless work?

::: gfx/2d/PathCairo.h
@@ +37,5 @@
>  class CairoPathContext : public RefCounted<CairoPathContext>
>  {
>  public:
> +  // Construct a new empty CairoPathContext that uses the given draw target and
> +  // it's cairo context. Using the existing context  may save having to copy the

its

@@ +62,5 @@
>    // our context.
>    void ForgetDrawTarget();
>  
>    // Create a duplicate context, and copy this path to that context. Optionally,
>    // the new context can be transformed.

Please fix this comment.

@@ +98,5 @@
>  
>    // This constructor, called with a CairoPathContext*, implicitly takes
>    // ownership of the path, and therefore makes aPathContext copy out its path
>    // regardless of whether it has a pointer to a DrawTargetCairo.
>    // The path currently set on aPathContext is not changed.

s/aPathContext/aContext/g
Comment 16 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-27 16:43:17 PDT
Created attachment 655817 [details] [diff] [review]
Part 3: Fix cairo path transforms v3
Comment 17 Mozilla RelEng Bot 2012-08-27 17:00:27 PDT
Try run for fa656a8c5699 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=fa656a8c5699
Results (out of 17 total builds):
    exception: 17
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-fa656a8c5699
Comment 18 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-27 22:36:51 PDT
Created attachment 655888 [details] [diff] [review]
Part 2: RAII matrix helper v3
Comment 19 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-27 22:56:51 PDT
(In reply to Joe Drew (:JOEDREW!) from comment #15)
> @@ +300,5 @@
> >  
> >  void
> >  PathCairo::CopyPathTo(cairo_t* aContext, DrawTargetCairo* aDrawTarget)
> >  {
> > +  mPathContext->CopyPathTo(aContext);
> 
> Why do you do this unconditionally? Was the previous optimization incorrect?
> And, if mPathContext->GetContext() == aContext, isn't this going to do
> useless work?

CairoPathContext::CopyPathTo() does this check anyway so it's duplicated. The previous code was deciding whether to subscribe to the DrawTarget more than to skip the call. Subscribing to the DrawTarget is a coin toss as to whether it'll save time or create the need for another copy.
Comment 20 Mozilla RelEng Bot 2012-08-28 13:45:27 PDT
Try run for 36ddd2e5b9b6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=36ddd2e5b9b6
Results (out of 2 total builds):
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-36ddd2e5b9b6
Comment 21 Mozilla RelEng Bot 2012-08-28 14:00:31 PDT
Try run for aace50d4c079 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=aace50d4c079
Results (out of 2 total builds):
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-aace50d4c079
Comment 22 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-29 15:24:42 PDT
Created attachment 656640 [details] [diff] [review]
Part 4: Added reftest and removed todo v3

Fuzzed reftest on D2D
Comment 23 Joe Drew (not getting mail) 2012-08-30 13:45:07 PDT
Comment on attachment 655817 [details] [diff] [review]
Part 3: Fix cairo path transforms v3

Review of attachment 655817 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/DrawTargetCairo.cpp
@@ -924,5 @@
> -  // We're about to logically change our transformation. Our current path will
> -  // need to change, because Cairo stores paths in device space.
> -  if (mPathObserver) {
> -    mPathObserver->MatrixWillChange(aTransform);
> -  }

As discussed on IRC, we can't remove this, because it'll break a perfectly reasonable use case:

[4:39 PM] <joe> PathBuilder* pb = dt->CreatePathBuilder()
[4:39 PM] <joe> pb->AddPoints() // or whatever
[4:39 PM] <joe> dt->SetMatrix(something)
[4:39 PM] <joe> pb->AddPoints()
[4:39 PM] <joe> those points will be transformed by the new matrix

Instead, maybe only copy out the path when the path is changed after the matrix is changed?
Comment 24 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-08-30 22:35:50 PDT
Created attachment 657157 [details] [diff] [review]
Part 3: Fix cairo path transforms v4
Comment 25 Mozilla RelEng Bot 2012-08-31 02:00:25 PDT
Try run for 2ff039275a33 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=2ff039275a33
Results (out of 24 total builds):
    success: 24
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-2ff039275a33
Comment 26 Joe Drew (not getting mail) 2012-08-31 13:42:23 PDT
Comment on attachment 657157 [details] [diff] [review]
Part 3: Fix cairo path transforms v4

Review of attachment 657157 [details] [diff] [review]:
-----------------------------------------------------------------

OK! Looks good with some small fixes. Hooray!

::: gfx/2d/PathCairo.cpp
@@ +61,5 @@
>    mContext = cairo_create(surf);
>  
> +  // Set the matrix to match the source context so that the path is copied in
> +  // device space.
> +  cairo_set_matrix(mContext, &matrix);

Ooh, better use a CairoTempMatrix here, so we're reset to identity afterwards.

@@ +88,5 @@
>    if (mDrawTarget) {
>      // The context we point to is going to change from under us. To continue
>      // using this path, we need to copy it to a new context.
>      DuplicateContextAndPath();
> +    mDrawTarget = nullptr;

Better to leave this as a call to ForgetDrawTarget().

@@ +100,3 @@
>      // cairo_copy_path gives us a user-space copy of the path, so we don't have
>      // to worry about transformations here.
> +    CairoTempMatrix tempMatrix(mContext, aTransform);

This line contradicts the comment above - better remove the comment :)

@@ +217,5 @@
> +PathBuilderCairo::PrepareForWrite()
> +{
> +  if (mPathContext->refCount() != 1) {
> +    mPathContext = new CairoPathContext(*mPathContext);
> +  }

Better add a comment saying that only PathBuilder and PathCairo maintain references to CairoPathContext, not DrawTarget, so this will ensure that we're duplicating if we're sharing a context.

::: gfx/2d/PathCairo.h
@@ +42,5 @@
> +  // path later.
> +  CairoPathContext(cairo_t* aCtx, DrawTargetCairo* aDrawTarget);
> +
> +  // Copy the path and supply a new fill rule.
> +  CairoPathContext(CairoPathContext& aPathContext);

This does not seem to supply a new fill rule.

@@ +86,4 @@
>    PathBuilderCairo(cairo_t* aCtx, DrawTargetCairo* aDrawTarget, FillRule aFillRule);
>  
> +  // Creates a path builder out of an existing CairoPathContext with a new fill
> +  // rule and transform. The fill 

Unfinished sentence in this comment.
Comment 27 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-09-02 14:23:35 PDT
(In reply to Joe Drew (:JOEDREW! \o/) from comment #26)
> ::: gfx/2d/PathCairo.cpp
> @@ +61,5 @@
> >    mContext = cairo_create(surf);
> >  
> > +  // Set the matrix to match the source context so that the path is copied in
> > +  // device space.
> > +  cairo_set_matrix(mContext, &matrix);
> 
> Ooh, better use a CairoTempMatrix here, so we're reset to identity
> afterwards.

We swap out the matrix every time so it doesn't matter what we set it to. We could set it to identity but it would serve no purpose.
Comment 28 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-09-02 15:26:20 PDT
Created attachment 657715 [details] [diff] [review]
Part 3: Fix cairo path transforms v5

Made changes as requested.
Comment 29 Mozilla RelEng Bot 2012-09-02 16:00:24 PDT
Try run for 51a9cd52811b is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=51a9cd52811b
Results (out of 16 total builds):
    exception: 15
    success: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-51a9cd52811b
Comment 30 Mozilla RelEng Bot 2012-09-02 17:45:30 PDT
Try run for 73be938dae67 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=73be938dae67
Results (out of 110 total builds):
    exception: 63
    success: 42
    warnings: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-73be938dae67
Comment 31 Mozilla RelEng Bot 2012-09-02 19:30:25 PDT
Try run for f5efca0c6882 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f5efca0c6882
Results (out of 194 total builds):
    exception: 1
    success: 168
    warnings: 21
    failure: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-f5efca0c6882
Comment 32 Mozilla RelEng Bot 2012-09-02 21:45:23 PDT
Try run for 23bf9f4a005b is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=23bf9f4a005b
Results (out of 2 total builds):
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-23bf9f4a005b
Comment 33 Mozilla RelEng Bot 2012-09-02 22:00:27 PDT
Try run for f5efca0c6882 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f5efca0c6882
Results (out of 196 total builds):
    exception: 1
    success: 169
    warnings: 21
    failure: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-f5efca0c6882
Comment 34 Mozilla RelEng Bot 2012-09-02 23:15:25 PDT
Try run for f5efca0c6882 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f5efca0c6882
Results (out of 197 total builds):
    exception: 1
    success: 170
    warnings: 21
    failure: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-f5efca0c6882
Comment 35 Mozilla RelEng Bot 2012-09-03 01:45:25 PDT
Try run for 0fcdb2834678 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0fcdb2834678
Results (out of 47 total builds):
    success: 45
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-0fcdb2834678
Comment 36 Mozilla RelEng Bot 2012-09-03 04:15:26 PDT
Try run for 0fcdb2834678 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0fcdb2834678
Results (out of 49 total builds):
    success: 47
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-0fcdb2834678
Comment 37 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-09-03 13:57:12 PDT
Here are the results for right patch set.

https://tbpl.mozilla.org/?tree=Try&rev=f5efca0c6882
https://tbpl.mozilla.org/?tree=Try&rev=0fcdb2834678
Comment 40 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-10-22 18:23:52 PDT
Bug 803658 suggests we need this fix on beta? If so, please request beta approval.
Comment 41 Anthony Jones (:kentuckyfriedtakahe, :k17e) 2012-10-24 15:11:32 PDT
Comment on attachment 655888 [details] [diff] [review]
Part 2: RAII matrix helper v3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 773460 enable azure canvas
User impact if declined: broken canvas transforms
Testing completed (on m-c, etc.): in m-c for several weeks before aurora uplift
Risk to taking this patch (and alternatives if risky): May cause other canvas path breakages or may not fix everything. Low risk on content because content doesn't use Azure Cairo backend.
String or UUID changes made by this patch: none

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