fix bug where we wrote uninitialized data to the flatten stream for shaders.

Both shader and gradient_shader write matrices to the flatten stream. However, they were
just calling write(&matrix, sizeof(SkMatrix)) and the matrix can contain lazily-computed
function ptrs as part of its internal cache. Thus two matrices that are logically the
same may write different bytes.

This is a problem because picture relies on flattening objects and then using the
flatten stream as a key into its cache. This matrix-write bug effectively kills the
effectiveness of the cache for shaders.

The fix is to write proper read/write functions for matrix (and region btw). These
call through to the existing low-level flatten routines (which just write into a
memory ptr).

git-svn-id: http://skia.googlecode.com/svn/trunk@1290 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/include/core/SkFlattenable.h b/include/core/SkFlattenable.h
index 553eb6c..03bcab8 100644 (file)
--- a/include/core/SkFlattenable.h
+++ b/include/core/SkFlattenable.h
@@ -70,6 +70,16 @@ protected:
     SkFlattenable(SkFlattenableReadBuffer&) {}
 };
 
+// helpers for matrix and region
+
+class SkMatrix;
+extern void SkReadMatrix(SkReader32*, SkMatrix*);
+extern void SkWriteMatrix(SkWriter32*, const SkMatrix&);
+
+class SkRegion;
+extern void SkReadRegion(SkReader32*, SkRegion*);
+extern void SkWriteRegion(SkWriter32*, const SkRegion&);
+
 ///////////////////////////////////////////////////////////////////////////////
 ///////////////////////////////////////////////////////////////////////////////
 

diff --git a/src/core/SkFlattenable.cpp b/src/core/SkFlattenable.cpp
index 7ad1472..99dd856 100644 (file)
--- a/src/core/SkFlattenable.cpp
+++ b/src/core/SkFlattenable.cpp
@@ -1,6 +1,35 @@
 #include "SkFlattenable.h"
 #include "SkTypeface.h"
 
+#include "SkMatrix.h"
+#include "SkRegion.h"
+
+void SkReadMatrix(SkReader32* reader, SkMatrix* matrix) {
+    size_t size = matrix->unflatten(reader->peek());
+    SkASSERT(SkAlign4(size) == size);
+    (void)reader->skip(size);
+}
+
+void SkWriteMatrix(SkWriter32* writer, const SkMatrix& matrix) {
+    size_t size = matrix.flatten(NULL);
+    SkASSERT(SkAlign4(size) == size);
+    matrix.flatten(writer->reserve(size));
+}
+
+void SkReadRegion(SkReader32* reader, SkRegion* rgn) {
+    size_t size = rgn->unflatten(reader->peek());
+    SkASSERT(SkAlign4(size) == size);
+    (void)reader->skip(size);
+}
+
+void SkWriteRegion(SkWriter32* writer, const SkRegion& rgn) {
+    size_t size = rgn.flatten(NULL);
+    SkASSERT(SkAlign4(size) == size);
+    rgn.flatten(writer->reserve(size));
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
 void SkFlattenable::flatten(SkFlattenableWriteBuffer&)
 {
     /*  we don't write anything at the moment, but this allows our subclasses

diff --git a/src/core/SkShader.cpp b/src/core/SkShader.cpp
index 8e469f2..7b46953 100644 (file)
--- a/src/core/SkShader.cpp
+++ b/src/core/SkShader.cpp
@@ -28,7 +28,7 @@ SkShader::SkShader(SkFlattenableReadBuffer& buffer)
         : INHERITED(buffer), fLocalMatrix(NULL) {
     if (buffer.readBool()) {
         SkMatrix matrix;
-        buffer.read(&matrix, sizeof(matrix));
+        SkReadMatrix(&buffer, &matrix);
         setLocalMatrix(matrix);
     }
     SkDEBUGCODE(fInSession = false;)
@@ -53,7 +53,7 @@ void SkShader::flatten(SkFlattenableWriteBuffer& buffer) {
     this->INHERITED::flatten(buffer);
     buffer.writeBool(fLocalMatrix != NULL);
     if (fLocalMatrix) {
-        buffer.writeMul4(fLocalMatrix, sizeof(SkMatrix));
+        SkWriteMatrix(&buffer, *fLocalMatrix);
     }
 }
 

diff --git a/src/effects/SkGradientShader.cpp b/src/effects/SkGradientShader.cpp
index 4eeeb68..cfe444e 100644 (file)
--- a/src/effects/SkGradientShader.cpp
+++ b/src/effects/SkGradientShader.cpp
@@ -342,7 +342,7 @@ Gradient_Shader::Gradient_Shader(SkFlattenableReadBuffer& buffer) :
             recs[i].fScale = buffer.readU32();
         }
     }
-    buffer.read(&fPtsToUnit, sizeof(SkMatrix));
+    SkReadMatrix(&buffer, &fPtsToUnit);
     fFlags = 0;
 }
 
@@ -370,7 +370,7 @@ void Gradient_Shader::flatten(SkFlattenableWriteBuffer& buffer) {
             buffer.write32(recs[i].fScale);
         }
     }
-    buffer.writeMul4(&fPtsToUnit, sizeof(SkMatrix));
+    SkWriteMatrix(&buffer, fPtsToUnit);
 }
 
 bool Gradient_Shader::setContext(const SkBitmap& device,