Bug 798849 - fix uninitialized values in WebGL element array cache... and reenable accidentally disabled canvas mochitests (!!!) - r=jgilbert
authorBenoit Jacob <bjacob@mozilla.com>
Thu, 11 Oct 2012 18:27:09 -0400
changeset 117924 4476b1668b33cb301cd3db8684998ca05a632c56
parent 117923 1c858930c3be5aafdc48db53c2e5d669c2d21bd1
child 117925 05567a9159c5ef3abea6dc57915bf7b296c5965a
push id1997
push userakeybl@mozilla.com
push dateMon, 07 Jan 2013 21:25:26 +0000
treeherdermozilla-beta@4baf45cdcf21 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgilbert
bugs798849, 732660
milestone19.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 798849 - fix uninitialized values in WebGL element array cache... and reenable accidentally disabled canvas mochitests (!!!) - r=jgilbert This fixes Valgrind errors while running TestWebGLElementArrayCache. This also fixes the way we do content/canvas/test in the Makefiles, which had been broken by my attempt to enable compiled-code tests there (bug 732660). Since I still don't know how to do compiled-code tests there, I've disabled them. At least we have the rest of content/canvas/test back. Let's hope that no regression happened.
content/canvas/Makefile.in
content/canvas/src/WebGLElementArrayCache.cpp
content/canvas/src/WebGLElementArrayCache.h
content/canvas/test/Makefile.in
content/canvas/test/compiled/TestWebGLElementArrayCache.cpp
--- a/content/canvas/Makefile.in
+++ b/content/canvas/Makefile.in
@@ -7,12 +7,12 @@ DEPTH		= @DEPTH@
 topsrcdir	= @top_srcdir@
 srcdir		= @srcdir@
 VPATH		= @srcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
 PARALLEL_DIRS	= public src
 
-TOOLS_DIRS += test
+TEST_DIRS += test
 
 include $(topsrcdir)/config/rules.mk
 
--- a/content/canvas/src/WebGLElementArrayCache.cpp
+++ b/content/canvas/src/WebGLElementArrayCache.cpp
@@ -141,63 +141,62 @@ public:
   WebGLElementArrayCacheTree(WebGLElementArrayCache& p)
     : mParent(p)
     , mNumLeaves(0)
     , mInvalidated(false)
     , mFirstInvalidatedLeaf(0)
     , mLastInvalidatedLeaf(0)
   {
     ResizeToParentSize();
-    Invalidate(0, mParent.ByteSize() - 1);
   }
 
   T GlobalMaximum() const {
     MOZ_ASSERT(!mInvalidated);
     return mTreeData[1];
   }
 
   // returns the index of the parent node; if treeIndex=1 (the root node),
   // the return value is 0.
   static size_t ParentNode(size_t treeIndex) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex >> 1;
   }
 
   static bool IsRightNode(size_t treeIndex) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex & 1;
   }
 
   static bool IsLeftNode(size_t treeIndex) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return !IsRightNode(treeIndex);
   }
 
   static size_t SiblingNode(size_t treeIndex) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex ^ 1;
   }
 
   static size_t LeftChildNode(size_t treeIndex) {
     MOZ_ASSERT(treeIndex);
     return treeIndex << 1;
   }
 
   static size_t RightChildNode(size_t treeIndex) {
     MOZ_ASSERT(treeIndex);
     return SiblingNode(LeftChildNode(treeIndex));
   }
 
   static size_t LeftNeighborNode(size_t treeIndex, size_t distance = 1) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex - distance;
   }
 
   static size_t RightNeighborNode(size_t treeIndex, size_t distance = 1) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex + distance;
   }
 
   size_t LeafForElement(size_t element) {
     size_t leaf = element / sElementsPerLeaf;
     MOZ_ASSERT(leaf < mNumLeaves);
     return leaf;
   }
@@ -282,20 +281,28 @@ public:
     return result;
   }
 
   bool ResizeToParentSize()
   {
     size_t numberOfElements = mParent.ByteSize() / sizeof(T);
     size_t requiredNumLeaves = (numberOfElements + sElementsPerLeaf - 1) / sElementsPerLeaf;
 
+    size_t oldNumLeaves = mNumLeaves;
     mNumLeaves = NextPowerOfTwo(requiredNumLeaves);
+    Invalidate(0, mParent.ByteSize() - 1);
 
     // see class comment for why we the tree storage size is 2 * mNumLeaves
-    return mTreeData.SetLength(2 * mNumLeaves);
+    if (!mTreeData.SetLength(2 * mNumLeaves)) {
+      return false;
+    }
+    if (mNumLeaves != oldNumLeaves) {
+      memset(mTreeData.Elements(), 0, mTreeData.Length() * sizeof(mTreeData[0]));
+    }
+    return true;
   }
 
   void Invalidate(size_t firstByte, size_t lastByte);
 
   void Update();
 
   size_t SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const
   {
@@ -375,33 +382,28 @@ void WebGLElementArrayCacheTree<T>::Upda
       }
       mTreeData[treeIndex] = m;
       treeIndex++;
     }
   }
 
   // Step #2: propagate the values up the tree. This is simply a matter of walking up
   // the tree and setting each node to the max of its two children.
-  while (true) {
-
-    // fast-exit case where only one node is invalidated at the current level
-    if (firstTreeIndex == lastTreeIndex) {
-      size_t firstTreeIndexParent = ParentNode(firstTreeIndex);
-      while (firstTreeIndexParent) {
-        mTreeData[firstTreeIndexParent] = NS_MAX(mTreeData[firstTreeIndex], mTreeData[SiblingNode(firstTreeIndex)]);
-        firstTreeIndex = firstTreeIndexParent;
-        firstTreeIndexParent = ParentNode(firstTreeIndex);
-      }
-      break;
-    }
+  while (firstTreeIndex > 1) {
 
     // move up 1 level
     firstTreeIndex = ParentNode(firstTreeIndex);
     lastTreeIndex = ParentNode(lastTreeIndex);
 
+    // fast-exit case where only one node is invalidated at the current level
+    if (firstTreeIndex == lastTreeIndex) {
+      mTreeData[firstTreeIndex] = NS_MAX(mTreeData[LeftChildNode(firstTreeIndex)], mTreeData[RightChildNode(firstTreeIndex)]);
+      continue;
+    }
+
     // initialize local iteration variables: child and parent.
     size_t child = LeftChildNode(firstTreeIndex);
     size_t parent = firstTreeIndex;
 
     // the unrolling makes this look more complicated than it is; the plain non-unrolled
     // version is in the second while loop below
     const int unrollSize = 8;
     while (RightNeighborNode(parent, unrollSize - 1) <= lastTreeIndex)
--- a/content/canvas/src/WebGLElementArrayCache.h
+++ b/content/canvas/src/WebGLElementArrayCache.h
@@ -60,17 +60,17 @@ private:
   template<typename T>
   const T* Elements() const { return static_cast<const T*>(mUntypedData); }
   template<typename T>
   T* Elements() { return static_cast<T*>(mUntypedData); }
 
   void InvalidateTrees(size_t firstByte, size_t lastByte);
 
   template<typename T>
-  friend class WebGLElementArrayCacheTree;
+  friend struct WebGLElementArrayCacheTree;
   template<typename T>
   friend struct TreeForType;
 
   void* mUntypedData;
   size_t mByteSize;
   WebGLElementArrayCacheTree<uint8_t>* mUint8Tree;
   WebGLElementArrayCacheTree<uint16_t>* mUint16Tree;
 };
--- a/content/canvas/test/Makefile.in
+++ b/content/canvas/test/Makefile.in
@@ -5,17 +5,17 @@
 
 DEPTH		= @DEPTH@
 topsrcdir	= @top_srcdir@
 srcdir		= @srcdir@
 VPATH		= @srcdir@
 relativesrcdir  = @relativesrcdir@
 DIRS		+= webgl crossorigin
 
-TOOLS_DIRS += compiled
+# TEST_DIRS += compiled
 
 include $(DEPTH)/config/autoconf.mk
 MOCHITEST_FILES = \
 	test_canvas.html \
 	image_transparent50.png \
 	image_redtransparent.png \
 	image_yellow.png \
 	image_anim-poster-gr.png \
--- a/content/canvas/test/compiled/TestWebGLElementArrayCache.cpp
+++ b/content/canvas/test/compiled/TestWebGLElementArrayCache.cpp
@@ -26,24 +26,21 @@ void VerifyImplFunction(bool condition, 
   }
 }
 
 #define VERIFY(condition) \
     VerifyImplFunction((condition), __FILE__, __LINE__)
 
 void MakeRandomVector(nsTArray<uint8_t>& a, size_t size) {
   a.SetLength(size);
-  // only the most-significant bits of rand() are reasonably random
-  // 16 here is arbitrary, may fail on platforms where RAND_MAX is low,
-  // but guarded by an assertion.
-  enum { bitsToIgnore = 16 };
-  MOZ_STATIC_ASSERT((unsigned int)(RAND_MAX) >> (8 + bitsToIgnore),
-                    "Didn't expect RAND_MAX to be so low");
+  // only the most-significant bits of rand() are reasonably random.
+  // RAND_MAX can be as low as 0x7fff, and we need 8 bits for the result, so we can only
+  // ignore the 7 least significant bits.
   for (size_t i = 0; i < size; i++)
-    a[i] = static_cast<uint8_t>((unsigned int)(rand()) >> bitsToIgnore);
+    a[i] = static_cast<uint8_t>((unsigned int)(rand()) >> 7);
 }
 
 template<typename T>
 T RandomInteger(T a, T b)
 {
   T result(a + rand() % (b - a + 1));
   return result;
 }