Add two pixel-based compositor unittests.

ui/gfx/compositor/layer_unittest.cc

Index: ui/gfx/compositor/layer_unittest.cc
diff --git a/ui/gfx/compositor/layer_unittest.cc b/ui/gfx/compositor/layer_unittest.cc
index 97bc387bddd1a49d63f701a4025db540184da11a..2ec0e5f8845e3db75746e9bf1ba97380eef45b91 100644
--- a/ui/gfx/compositor/layer_unittest.cc
+++ b/ui/gfx/compositor/layer_unittest.cc
@@ -15,13 +15,39 @@
 #include "ui/gfx/compositor/compositor_observer.h"
 #include "ui/gfx/compositor/layer.h"
 #include "ui/gfx/compositor/layer_animation_sequence.h"
-#include "ui/gfx/compositor/test_compositor.h"
-#include "ui/gfx/compositor/test_compositor_host.h"
+#include "ui/gfx/compositor/test/test_compositor.h"
+#include "ui/gfx/compositor/test/test_compositor_host.h"
+#include "ui/gfx/gfx_paths.h"
 
 namespace ui {
 
 namespace {
 
+// Encodes a bitmap into a PNG and write to disk. Returns true on success. The
+// parent directory does not have to exist.
+bool WritePNGFile(const SkBitmap& bitmap, const FilePath& file_path) {
+  std::vector<unsigned char> png_data;
+  if (gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, true, &png_data) &&
+      file_util::CreateDirectory(file_path.DirName())) {
+    int bytes_written = file_util::WriteFile(

[sky, 2011/11/14 21:44:26]

nit: make this one statement,
return file_util::WriteFile(..) == static_cast<int>(png_data.size());

+        file_path, reinterpret_cast<char*>(&png_data[0]), png_data.size());
+    if (bytes_written == static_cast<int>(png_data.size()))
+      return true;
+  }
+  return false;
+}
+
+// Reads and decodes a PNG image to a bitmap. Returns true on success. The PNG
+// should have been encoded using |gfx::PNGCodec::Encode|.
+bool ReadPNGFile(const FilePath& file_path, SkBitmap* bitmap) {
+  DCHECK(bitmap);
+  std::string png_data;
+  return file_util::ReadFileToString(file_path, &png_data) &&
+         gfx::PNGCodec::Decode(reinterpret_cast<unsigned char*>(&png_data[0]),
+                               png_data.length(),
+                               bitmap);
+}
+
 // There are three test classes in here that configure the Compositor and
 // Layer's slightly differently:
 // - LayerWithNullDelegateTest uses TestCompositor and NullLayerDelegate as the
@@ -53,7 +79,11 @@ class ColoredLayer : public Layer, public LayerDelegate {
 
 class LayerWithRealCompositorTest : public testing::Test {
  public:
-  LayerWithRealCompositorTest() {}
+  LayerWithRealCompositorTest() {
+    if (PathService::Get(gfx::DIR_TEST_DATA, &test_data_directory_)) {
+      test_data_directory_ = test_data_directory_.AppendASCII("compositor");
+    }
+  }
   virtual ~LayerWithRealCompositorTest() {}
 
   // Overridden from testing::Test:
@@ -105,9 +135,61 @@ class LayerWithRealCompositorTest : public testing::Test {
         gfx::Rect(0, 0, layer->bounds().width(), layer->bounds().height()));
   }
 
+  // Compares with a ping file on disk, and returns true if it is the same as

[jonathan.backer, 2011/11/14 20:22:40]

s/ping/PNG

+  // the given image.

[jonathan.backer, 2011/11/14 20:22:40]

Your call. Seems like something that belongs in the anon namespace at the top.
Just pass the file path instead of the file.

+  bool IsSameAsPNGFile(const SkBitmap& gen_bmp, const char* file) {

[sky, 2011/11/14 21:44:26]

Document what file is relative to.

+    SkBitmap ref_bmp;
+    bool should_compare = true;
+    FilePath ref_img_path = test_data_directory_.AppendASCII(file);
+    if (!ReadPNGFile(ref_img_path, &ref_bmp)) {
+      LOG(ERROR) << "Cannot read reference image: " << ref_img_path.value();
+      should_compare = false;

[sky, 2011/11/14 21:44:26]

Is there any point on continuing on if you can't read the file? Maybe all these
should be ASSERTs?

+    }
+
+    if (should_compare &&
+        (ref_bmp.width() != gen_bmp.width() ||
+         ref_bmp.height() != gen_bmp.height())) {
+      LOG(ERROR)
+          << "Dimensions do not match (Expected) vs (Actual):"
+          << "(" << ref_bmp.width() << "x" << ref_bmp.height()
+              << ") vs. "
+          << "(" << gen_bmp.width() << "x" << gen_bmp.height() << ")";
+      should_compare = false;

[sky, 2011/11/14 21:44:26]

Return here too?

+    }
+
+    // Compare pixels and create a simple diff image.
+    int diff_pixels_count = 0;
+    if (should_compare) {
+      SkAutoLockPixels lock_bmp(gen_bmp);
+      SkAutoLockPixels lock_ref_bmp(ref_bmp);
+      // The reference images were saved with no alpha channel. Use the mask to
+      // set alpha to 0.
+      uint32_t kAlphaMask = 0x00FFFFFF;
+      for (int x = 0; x < gen_bmp.width(); ++x) {
+        for (int y = 0; y < gen_bmp.height(); ++y) {
+          if ((*gen_bmp.getAddr32(x, y) & kAlphaMask) !=
+              (*ref_bmp.getAddr32(x, y) & kAlphaMask)) {
+            ++diff_pixels_count;
+          }
+        }
+      }
+    }
+
+    // Write the generated and diff images if the comparison failed.

[jonathan.backer, 2011/11/14 20:22:40]

Stale comment?

+    if (diff_pixels_count != 0) {
+      LOG(ERROR) << "Images differ by pixel count: " << diff_pixels_count;
+      return false;
+    }
+
+    return true;
+  }
+
  private:
   scoped_ptr<TestCompositorHost> window_;
 
+  // The root directory for test files.
+  FilePath test_data_directory_;
+
   DISALLOW_COPY_AND_ASSIGN(LayerWithRealCompositorTest);
 };
 
@@ -178,20 +260,6 @@ class NullLayerDelegate : public LayerDelegate {
   DISALLOW_COPY_AND_ASSIGN(NullLayerDelegate);
 };
 
-// Encodes a bitmap into a PNG and write to disk. Returns true on success. The
-// parent directory does not have to exist.
-bool WritePNGFile(const SkBitmap& bitmap, const FilePath& file_path) {
-  std::vector<unsigned char> png_data;
-  if (gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, true, &png_data) &&
-      file_util::CreateDirectory(file_path.DirName())) {
-    int bytes_written = file_util::WriteFile(
-        file_path, reinterpret_cast<char*>(&png_data[0]), png_data.size());
-    if (bytes_written == static_cast<int>(png_data.size()))
-      return true;
-  }
-  return false;
-}
-
 }
 
 #if defined(OS_WIN)
@@ -203,6 +271,8 @@ bool WritePNGFile(const SkBitmap& bitmap, const FilePath& file_path) {
 #define MAYBE_Hierarchy DISABLED_Hierarchy
 #define MAYBE_HierarchyNoTexture DISABLED_HierarchyNoTexture
 #define MAYBE_DrawPixels DISABLED_DrawPixels
+#define MAYBE_ModifyHierarchy DISABLED_ModifyHierarchy
+#define MAYBE_Opacity DISABLED_Opacity
 #else
 #define MAYBE_Delegate Delegate
 #define MAYBE_Draw Draw
@@ -210,6 +280,8 @@ bool WritePNGFile(const SkBitmap& bitmap, const FilePath& file_path) {
 #define MAYBE_Hierarchy Hierarchy
 #define MAYBE_HierarchyNoTexture HierarchyNoTexture
 #define MAYBE_DrawPixels DrawPixels
+#define MAYBE_ModifyHierarchy ModifyHierarchy
+#define MAYBE_Opacity Opacity
 #endif
 
 TEST_F(LayerWithRealCompositorTest, MAYBE_Draw) {
@@ -823,4 +895,102 @@ TEST_F(LayerWithRealCompositorTest, MAYBE_DrawPixels) {
   EXPECT_TRUE(is_all_red);
 }
 
+// Checks that modifying the hierarchy correctly affects final composite.
+TEST_F(LayerWithRealCompositorTest, MAYBE_ModifyHierarchy) {
+  GetCompositor()->WidgetSizeChanged(gfx::Size(50, 50));
+
+  // l0
+  //  +-l11
+  //  | +-l21
+  //  +-l12
+  scoped_ptr<Layer> l0(CreateColorLayer(SK_ColorRED,
+                                        gfx::Rect(0, 0, 50, 50)));
+  scoped_ptr<Layer> l11(CreateColorLayer(SK_ColorGREEN,
+                                         gfx::Rect(0, 0, 25, 25)));
+  scoped_ptr<Layer> l21(CreateColorLayer(SK_ColorMAGENTA,
+                                         gfx::Rect(0, 0, 15, 15)));
+  scoped_ptr<Layer> l12(CreateColorLayer(SK_ColorBLUE,
+                                         gfx::Rect(10, 10, 25, 25)));
+
+  l0->Add(l11.get());
+  l11->Add(l21.get());
+  l0->Add(l12.get());
+
+  DrawTree(l0.get());
+
+  SkBitmap bitmap;
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
+  ASSERT_FALSE(bitmap.empty());
+
+  //WritePNGFile(bitmap,
+  //             FilePath("ui/gfx/test/data/compositor/ModifyHierarchy1.png"));

[jonathan.backer, 2011/11/14 20:22:40]

This is probably OK. But I'm guessing that you need a space after //

+  ASSERT_TRUE(IsSameAsPNGFile(bitmap, "ModifyHierarchy1.png"));
+
+  l0->MoveToFront(l11.get());
+
+  DrawTree(l0.get());
+
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
+  ASSERT_FALSE(bitmap.empty());
+
+  //WritePNGFile(bitmap,
+  //             FilePath("ui/gfx/test/data/compositor/ModifyHierarchy2.png"));
+  ASSERT_TRUE(IsSameAsPNGFile(bitmap, "ModifyHierarchy2.png"));
+
+  // l11 is already at the front, should have no effect.
+  l0->MoveToFront(l11.get());
+
+  DrawTree(l0.get());
+
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
+  ASSERT_FALSE(bitmap.empty());
+
+  ASSERT_TRUE(IsSameAsPNGFile(bitmap, "ModifyHierarchy2.png"));
+
+  // l11 is already at the front, should have no effect.
+  l0->MoveAbove(l11.get(), l12.get());
+
+  DrawTree(l0.get());
+
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
+  ASSERT_FALSE(bitmap.empty());
+
+  ASSERT_TRUE(IsSameAsPNGFile(bitmap, "ModifyHierarchy2.png"));
+
+  // should restore to original configuration
+  l0->MoveAbove(l12.get(), l11.get());
+
+  DrawTree(l0.get());
+
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
+  ASSERT_FALSE(bitmap.empty());
+
+  ASSERT_TRUE(IsSameAsPNGFile(bitmap, "ModifyHierarchy1.png"));
+}
+
+// Opacity is rendered correctly.
+// Checks that modifying the hierarchy correctly affects final composite.
+TEST_F(LayerWithRealCompositorTest, MAYBE_Opacity) {
+  GetCompositor()->WidgetSizeChanged(gfx::Size(50, 50));
+
+  // l0
+  //  +-l11
+  scoped_ptr<Layer> l0(CreateColorLayer(SK_ColorRED,
+                                        gfx::Rect(0, 0, 50, 50)));
+  scoped_ptr<Layer> l11(CreateColorLayer(SK_ColorGREEN,
+                                         gfx::Rect(0, 0, 25, 25)));
+
+  l11->SetOpacity(0.75);
+
+  l0->Add(l11.get());
+  DrawTree(l0.get());
+
+  SkBitmap bitmap;
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
+  ASSERT_FALSE(bitmap.empty());
+
+  //WritePNGFile(bitmap, FilePath("ui/gfx/test/data/compositor/Opacity.png"));
+  ASSERT_TRUE(IsSameAsPNGFile(bitmap, "Opacity.png"));
+}
+
 } // namespace ui