Bug 814952: Further cleanup state management surrounding paths and pathbuilders. r=joedrew
authorBas Schouten <bschouten@mozilla.com>
Thu, 13 Dec 2012 16:34:51 +0100
changeset 125036 fa72584497cbe20dfadaaf0018c7bd45de0f1ba8
parent 125035 ab30da702673f66409016bbd568b42ff7430bda1
child 125037 950611b8b91c5f924860ef5cc603919fcecebb70
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoedrew
bugs814952
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 814952: Further cleanup state management surrounding paths and pathbuilders. r=joedrew
gfx/thebes/gfxContext.cpp
gfx/thebes/gfxContext.h
--- a/gfx/thebes/gfxContext.cpp
+++ b/gfx/thebes/gfxContext.cpp
@@ -202,28 +202,19 @@ gfxContext::Restore()
 
     if (CurrentState().clipWasReset &&
         CurrentState().drawTarget == mStateStack[mStateStack.Length() - 2].drawTarget) {
       PushClipsToDT(mDT);
     }
 
     mStateStack.RemoveElementAt(mStateStack.Length() - 1);
 
-    if ((mPathBuilder || mPath || mPathIsRect) && !mTransformChanged) {
-      // Support here isn't fully correct if the path is continued -after-
-      // the restore. We don't currently have users that do this and we should
-      // make sure there will not be any. Sadly we can't assert this easily.
-      mTransformChanged = true;
-      mPathTransform = mTransform;
-    }
-
     mDT = CurrentState().drawTarget;
 
-    mTransform = CurrentState().transform;
-    mDT->SetTransform(GetDTTransform());
+    ChangeTransform(CurrentState().transform, false);
   }
 }
 
 // drawing
 void
 gfxContext::NewPath()
 {
   if (mCairo) {
@@ -285,16 +276,18 @@ gfxContext::CurrentPoint()
 void
 gfxContext::Stroke()
 {
   if (mCairo) {
     cairo_stroke_preserve(mCairo);
   } else {
     AzureState &state = CurrentState();
     if (mPathIsRect) {
+      MOZ_ASSERT(!mTransformChanged);
+
       mDT->StrokeRect(mRect, GeneralPattern(this),
                       state.strokeOptions,
                       DrawOptions(1.0f, GetOp(), state.aaMode));
     } else {
       EnsurePath();
 
       mDT->Stroke(mPath, GeneralPattern(this), state.strokeOptions,
                   DrawOptions(1.0f, GetOp(), state.aaMode));
@@ -469,20 +462,20 @@ gfxContext::Rectangle(const gfxRect& rec
         rec = ToRect(mat.TransformBounds(newRect));
       }
     }
 
     if (!mPathBuilder && !mPathIsRect) {
       mPathIsRect = true;
       mRect = rec;
       return;
-    } else if (!mPathBuilder) {
-      EnsurePathBuilder();
     }
-    
+
+    EnsurePathBuilder();
+
     mPathBuilder->MoveTo(rec.TopLeft());
     mPathBuilder->LineTo(rec.TopRight());
     mPathBuilder->LineTo(rec.BottomRight());
     mPathBuilder->LineTo(rec.BottomLeft());
     mPathBuilder->Close();
   }
 }
 
@@ -1133,17 +1126,19 @@ gfxContext::Clip(const gfxRect& rect)
 }
 
 void
 gfxContext::Clip()
 {
   if (mCairo) {
     cairo_clip_preserve(mCairo);
   } else {
-    if (mPathIsRect && !mTransformChanged) {
+    if (mPathIsRect) {
+      MOZ_ASSERT(!mTransformChanged);
+
       AzureState::PushedClip clip = { NULL, mRect, mTransform };
       CurrentState().pushedClips.AppendElement(clip);
       mDT->PushClipRect(mRect);
     } else {
       EnsurePath();
       mDT->PushClip(mPath);
       AzureState::PushedClip clip = { mPath, Rect(), mTransform };
       CurrentState().pushedClips.AppendElement(clip);
@@ -1945,56 +1940,75 @@ gfxContext::EnsurePath()
   EnsurePathBuilder();
   mPath = mPathBuilder->Finish();
   mPathBuilder = NULL;
 }
 
 void
 gfxContext::EnsurePathBuilder()
 {
-  if (mPathBuilder) {
+  if (mPathBuilder && !mTransformChanged) {
     return;
   }
 
   if (mPath) {
-    mPathBuilder = mPath->CopyToBuilder(CurrentState().fillRule);
-    mPath = NULL;
+    if (!mTransformChanged) {
+      mPathBuilder = mPath->CopyToBuilder(CurrentState().fillRule);
+      mPath = NULL;
+    } else {
+      Matrix invTransform = mTransform;
+      invTransform.Invert();
+      Matrix toNewUS = mPathTransform * invTransform;
+      mPathBuilder = mPath->TransformedCopyToBuilder(toNewUS, CurrentState().fillRule);
+    }
+    return;
   }
 
-  mPathBuilder = mDT->CreatePathBuilder(CurrentState().fillRule);
+  DebugOnly<PathBuilder*> oldPath = mPathBuilder.get();
+
+  if (!mPathBuilder) {
+    mPathBuilder = mDT->CreatePathBuilder(CurrentState().fillRule);
+
+    if (mPathIsRect) {
+      mPathBuilder->MoveTo(mRect.TopLeft());
+      mPathBuilder->LineTo(mRect.TopRight());
+      mPathBuilder->LineTo(mRect.BottomRight());
+      mPathBuilder->LineTo(mRect.BottomLeft());
+      mPathBuilder->Close();
+    }
+  }
 
-  if (mPathIsRect && !mTransformChanged) {
-    mPathBuilder->MoveTo(mRect.TopLeft());
-    mPathBuilder->LineTo(mRect.TopRight());
-    mPathBuilder->LineTo(mRect.BottomRight());
-    mPathBuilder->LineTo(mRect.BottomLeft());
-    mPathBuilder->Close();
-  } else if (mPathIsRect) {
-    mTransformChanged = false;
-    Matrix mat = mTransform;
-    mat.Invert();
-    mat = mPathTransform * mat;
-    mPathBuilder->MoveTo(mat * mRect.TopLeft());
-    mPathBuilder->LineTo(mat * mRect.TopRight());
-    mPathBuilder->LineTo(mat * mRect.BottomRight());
-    mPathBuilder->LineTo(mat * mRect.BottomLeft());
-    mPathBuilder->Close();
+  if (mTransformChanged) {
+    // This could be an else if since this should never happen when
+    // mPathBuilder is NULL and mPath is NULL. But this way we can assert
+    // if all the state is as expected.
+    MOZ_ASSERT(oldPath);
+    MOZ_ASSERT(!mPathIsRect);
+
+    Matrix invTransform = mTransform;
+    invTransform.Invert();
+    Matrix toNewUS = mPathTransform * invTransform;
+
+    RefPtr<Path> path = mPathBuilder->Finish();
+    mPathBuilder = path->TransformedCopyToBuilder(toNewUS, CurrentState().fillRule);
   }
 
   mPathIsRect = false;
 }
 
 void
 gfxContext::FillAzure(Float aOpacity)
 {
   AzureState &state = CurrentState();
 
   CompositionOp op = GetOp();
 
-  if (mPathIsRect && !mTransformChanged) {
+  if (mPathIsRect) {
+    MOZ_ASSERT(!mTransformChanged);
+
     if (state.opIsClear) {
       mDT->ClearRect(mRect);
     } else if (op == OP_SOURCE) {
       // Emulate cairo operator source which is bound by mask!
       mDT->ClearRect(mRect);
       mDT->FillRect(mRect, GeneralPattern(this), DrawOptions(aOpacity));
     } else {
       mDT->FillRect(mRect, GeneralPattern(this), DrawOptions(aOpacity, op, state.aaMode));
@@ -2064,55 +2078,62 @@ gfxContext::GetOp()
       return OP_SOURCE;
     }
   }
 }
 
 /* SVG font code can change the transform after having set the pattern on the
  * context. When the pattern is set it is in user space, if the transform is
  * changed after doing so the pattern needs to be converted back into userspace.
- * We just store the old pattern here so that we only do the work needed here
- * if the pattern is actually used.
+ * We just store the old pattern transform here so that we only do the work
+ * needed here if the pattern is actually used.
+ * We need to avoid doing this when this ChangeTransform comes from a restore,
+ * since the current pattern and the current transform are both part of the
+ * state we know the new CurrentState()'s values are valid. But if we assume
+ * a change they might become invalid since patternTransformChanged is part of
+ * the state and might be false for the restored AzureState.
  */
 void
-gfxContext::ChangeTransform(const Matrix &aNewMatrix)
+gfxContext::ChangeTransform(const Matrix &aNewMatrix, bool aUpdatePatternTransform)
 {
   AzureState &state = CurrentState();
 
-  if ((state.pattern || state.sourceSurface)
+  if (aUpdatePatternTransform && (state.pattern || state.sourceSurface)
       && !state.patternTransformChanged) {
     state.patternTransform = mTransform;
     state.patternTransformChanged = true;
   }
 
-  if (mPathBuilder || mPathIsRect) {
+  if (mPathIsRect) {
     Matrix invMatrix = aNewMatrix;
     
     invMatrix.Invert();
 
     Matrix toNewUS = mTransform * invMatrix;
 
-    if (toNewUS.IsRectilinear() && mPathIsRect) {
+    if (toNewUS.IsRectilinear()) {
       mRect = toNewUS.TransformBounds(mRect);
       mRect.NudgeToIntegers();
-    } else if (mPathIsRect) {
+    } else {
       mPathBuilder = mDT->CreatePathBuilder(CurrentState().fillRule);
       
       mPathBuilder->MoveTo(toNewUS * mRect.TopLeft());
       mPathBuilder->LineTo(toNewUS * mRect.TopRight());
       mPathBuilder->LineTo(toNewUS * mRect.BottomRight());
       mPathBuilder->LineTo(toNewUS * mRect.BottomLeft());
       mPathBuilder->Close();
-    } else {
-      RefPtr<Path> path = mPathBuilder->Finish();
-      // Create path in device space.
-      mPathBuilder = path->TransformedCopyToBuilder(toNewUS);
+
+      mPathIsRect = false;
     }
+
     // No need to consider the transform changed now!
     mTransformChanged = false;
+  } else if ((mPath || mPathBuilder) && !mTransformChanged) {
+    mTransformChanged = true;
+    mPathTransform = mTransform;
   }
 
   mTransform = aNewMatrix;
 
   mDT->SetTransform(GetDTTransform());
 }
 
 Rect
--- a/gfx/thebes/gfxContext.h
+++ b/gfx/thebes/gfxContext.h
@@ -760,17 +760,17 @@ private:
 
   // This ensures mPath contains a valid path (in user space!)
   void EnsurePath();
   // This ensures mPathBuilder contains a valid PathBuilder (in user space!)
   void EnsurePathBuilder();
   void FillAzure(mozilla::gfx::Float aOpacity);
   void PushClipsToDT(mozilla::gfx::DrawTarget *aDT);
   CompositionOp GetOp();
-  void ChangeTransform(const mozilla::gfx::Matrix &aNewMatrix);
+  void ChangeTransform(const mozilla::gfx::Matrix &aNewMatrix, bool aUpdatePatternTransform = true);
   Rect GetAzureDeviceSpaceClipBounds();
   Matrix GetDeviceTransform() const;
   Matrix GetDTTransform() const;
   void PushNewDT(gfxASurface::gfxContentType content);
 
   bool mPathIsRect;
   bool mTransformChanged;
   Matrix mPathTransform;