Merge "Stop comparing Descriptions with memcmp" into nyc-dev

libs/hwui/PathCache.cpp

@@ -19,6 +19,7 @@
 #include <SkColor.h>
 #include <SkPaint.h>
 #include <SkPath.h>
+#include <SkPathEffect.h>
 #include <SkRect.h>
 
 #include <utils/JenkinsHash.h>
@@ -35,18 +36,34 @@
 namespace android {
 namespace uirenderer {
 
+template <class T>
+static bool compareWidthHeight(const T& lhs, const T& rhs) {
+    return (lhs.mWidth == rhs.mWidth) && (lhs.mHeight == rhs.mHeight);
+}
+
+static bool compareRoundRects(const PathDescription::Shape::RoundRect& lhs,
+        const PathDescription::Shape::RoundRect& rhs) {
+    return compareWidthHeight(lhs, rhs) && lhs.mRx == rhs.mRx && lhs.mRy == rhs.mRy;
+}
+
+static bool compareArcs(const PathDescription::Shape::Arc& lhs, const PathDescription::Shape::Arc& rhs) {
+    return compareWidthHeight(lhs, rhs) && lhs.mStartAngle == rhs.mStartAngle &&
+            lhs.mSweepAngle == rhs.mSweepAngle && lhs.mUseCenter == rhs.mUseCenter;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Cache entries
 ///////////////////////////////////////////////////////////////////////////////
 
 PathDescription::PathDescription()
-        : type(kShapeNone)
+        : type(ShapeType::None)
         , join(SkPaint::kDefault_Join)
         , cap(SkPaint::kDefault_Cap)
         , style(SkPaint::kFill_Style)
         , miter(4.0f)
         , strokeWidth(1.0f)
         , pathEffect(nullptr) {
+    // Shape bits should be set to zeroes, because they are used for hash calculation.
     memset(&shape, 0, sizeof(Shape));
 }
 
@@ -58,11 +75,12 @@ PathDescription::PathDescription(ShapeType type, const SkPaint* paint)
         , miter(paint->getStrokeMiter())
         , strokeWidth(paint->getStrokeWidth())
         , pathEffect(paint->getPathEffect()) {
+    // Shape bits should be set to zeroes, because they are used for hash calculation.
     memset(&shape, 0, sizeof(Shape));
 }
 
 hash_t PathDescription::hash() const {
-    uint32_t hash = JenkinsHashMix(0, type);
+    uint32_t hash = JenkinsHashMix(0, static_cast<int>(type));
     hash = JenkinsHashMix(hash, join);
     hash = JenkinsHashMix(hash, cap);
     hash = JenkinsHashMix(hash, style);
@@ -73,6 +91,32 @@ hash_t PathDescription::hash() const {
     return JenkinsHashWhiten(hash);
 }
 
+bool PathDescription::operator==(const PathDescription& rhs) const {
+    if (type != rhs.type) return false;
+    if (join != rhs.join) return false;
+    if (cap != rhs.cap) return false;
+    if (style != rhs.style) return false;
+    if (miter != rhs.miter) return false;
+    if (strokeWidth != rhs.strokeWidth) return false;
+    if (pathEffect != rhs.pathEffect) return false;
+    switch (type) {
+        case ShapeType::None:
+            return 0;
+        case ShapeType::Rect:
+            return compareWidthHeight(shape.rect, rhs.shape.rect);
+        case ShapeType::RoundRect:
+            return compareRoundRects(shape.roundRect, rhs.shape.roundRect);
+        case ShapeType::Circle:
+            return shape.circle.mRadius == rhs.shape.circle.mRadius;
+        case ShapeType::Oval:
+            return compareWidthHeight(shape.oval, rhs.shape.oval);
+        case ShapeType::Arc:
+            return compareArcs(shape.arc, rhs.shape.arc);
+        case ShapeType::Path:
+            return shape.path.mGenerationID == rhs.shape.path.mGenerationID;
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Utilities
 ///////////////////////////////////////////////////////////////////////////////
@@ -322,7 +366,7 @@ void PathCache::clearGarbage() {
             LruCache<PathDescription, PathTexture*>::Iterator iter(mCache);
             while (iter.next()) {
                 const PathDescription& key = iter.key();
-                if (key.type == kShapePath && key.shape.path.mGenerationID == generationID) {
+                if (key.type == ShapeType::Path && key.shape.path.mGenerationID == generationID) {
                     pathsToRemove.push(key);
                 }
             }
@@ -336,7 +380,7 @@ void PathCache::clearGarbage() {
 }
 
 PathTexture* PathCache::get(const SkPath* path, const SkPaint* paint) {
-    PathDescription entry(kShapePath, paint);
+    PathDescription entry(ShapeType::Path, paint);
     entry.shape.path.mGenerationID = path->getGenerationID();
 
     PathTexture* texture = mCache.get(entry);
@@ -366,9 +410,8 @@ PathTexture* PathCache::get(const SkPath* path, const SkPaint* paint) {
     return texture;
 }
 
-void PathCache::remove(const SkPath* path, const SkPaint* paint)
+void PathCache::remove(const SkPath* path, const SkPaint* paint) {
-{
+    PathDescription entry(ShapeType::Path, paint);
-    PathDescription entry(kShapePath, paint);
     entry.shape.path.mGenerationID = path->getGenerationID();
     mCache.remove(entry);
 }
@@ -378,7 +421,7 @@ void PathCache::precache(const SkPath* path, const SkPaint* paint) {
         return;
     }
 
-    PathDescription entry(kShapePath, paint);
+    PathDescription entry(ShapeType::Path, paint);
     entry.shape.path.mGenerationID = path->getGenerationID();
 
     PathTexture* texture = mCache.get(entry);
@@ -417,7 +460,7 @@ void PathCache::precache(const SkPath* path, const SkPaint* paint) {
 
 PathTexture* PathCache::getRoundRect(float width, float height,
         float rx, float ry, const SkPaint* paint) {
-    PathDescription entry(kShapeRoundRect, paint);
+    PathDescription entry(ShapeType::RoundRect, paint);
     entry.shape.roundRect.mWidth = width;
     entry.shape.roundRect.mHeight = height;
     entry.shape.roundRect.mRx = rx;
@@ -442,7 +485,7 @@ PathTexture* PathCache::getRoundRect(float width, float height,
 ///////////////////////////////////////////////////////////////////////////////
 
 PathTexture* PathCache::getCircle(float radius, const SkPaint* paint) {
-    PathDescription entry(kShapeCircle, paint);
+    PathDescription entry(ShapeType::Circle, paint);
     entry.shape.circle.mRadius = radius;
 
     PathTexture* texture = get(entry);
@@ -462,7 +505,7 @@ PathTexture* PathCache::getCircle(float radius, const SkPaint* paint) {
 ///////////////////////////////////////////////////////////////////////////////
 
 PathTexture* PathCache::getOval(float width, float height, const SkPaint* paint) {
-    PathDescription entry(kShapeOval, paint);
+    PathDescription entry(ShapeType::Oval, paint);
     entry.shape.oval.mWidth = width;
     entry.shape.oval.mHeight = height;
 
@@ -485,7 +528,7 @@ PathTexture* PathCache::getOval(float width, float height, const SkPaint* paint)
 ///////////////////////////////////////////////////////////////////////////////
 
 PathTexture* PathCache::getRect(float width, float height, const SkPaint* paint) {
-    PathDescription entry(kShapeRect, paint);
+    PathDescription entry(ShapeType::Rect, paint);
     entry.shape.rect.mWidth = width;
     entry.shape.rect.mHeight = height;
 
@@ -509,7 +552,7 @@ PathTexture* PathCache::getRect(float width, float height, const SkPaint* paint)
 
 PathTexture* PathCache::getArc(float width, float height,
         float startAngle, float sweepAngle, bool useCenter, const SkPaint* paint) {
-    PathDescription entry(kShapeArc, paint);
+    PathDescription entry(ShapeType::Arc, paint);
     entry.shape.arc.mWidth = width;
     entry.shape.arc.mHeight = height;
     entry.shape.arc.mStartAngle = startAngle;

libs/hwui/PathCache.h

@@ -25,6 +25,7 @@
 #include "utils/Pair.h"
 
 #include <GLES2/gl2.h>
+#include <SkPaint.h>
 #include <SkPath.h>
 #include <utils/LruCache.h>
 #include <utils/Mutex.h>
@@ -108,18 +109,18 @@ private:
     sp<Task<SkBitmap*> > mTask;
 }; // struct PathTexture
 
-enum ShapeType {
+enum class ShapeType {
-    kShapeNone,
+    None,
-    kShapeRect,
+    Rect,
-    kShapeRoundRect,
+    RoundRect,
-    kShapeCircle,
+    Circle,
-    kShapeOval,
+    Oval,
-    kShapeArc,
+    Arc,
-    kShapePath
+    Path
 };
 
 struct PathDescription {
-    DESCRIPTION_TYPE(PathDescription);
+    HASHABLE_TYPE(PathDescription);
     ShapeType type;
     SkPaint::Join join;
     SkPaint::Cap cap;
@@ -159,8 +160,6 @@ struct PathDescription {
 
     PathDescription();
     PathDescription(ShapeType shapeType, const SkPaint* paint);
-
-    hash_t hash() const;
 };
 
 /**

libs/hwui/TessellationCache.cpp

@@ -35,13 +35,14 @@ namespace uirenderer {
 ///////////////////////////////////////////////////////////////////////////////
 
 TessellationCache::Description::Description()
-        : type(kNone)
+        : type(Type::None)
         , scaleX(1.0f)
         , scaleY(1.0f)
         , aa(false)
         , cap(SkPaint::kDefault_Cap)
         , style(SkPaint::kFill_Style)
         , strokeWidth(1.0f) {
+    // Shape bits should be set to zeroes, because they are used for hash calculation.
     memset(&shape, 0, sizeof(Shape));
 }
 
@@ -52,11 +53,30 @@ TessellationCache::Description::Description(Type type, const Matrix4& transform,
         , style(paint.getStyle())
         , strokeWidth(paint.getStrokeWidth()) {
     PathTessellator::extractTessellationScales(transform, &scaleX, &scaleY);
+    // Shape bits should be set to zeroes, because they are used for hash calculation.
     memset(&shape, 0, sizeof(Shape));
 }
 
+bool TessellationCache::Description::operator==(const TessellationCache::Description& rhs) const {
+    if (type != rhs.type) return false;
+    if (scaleX != rhs.scaleX) return false;
+    if (scaleY != rhs.scaleY) return false;
+    if (aa != rhs.aa) return false;
+    if (cap != rhs.cap) return false;
+    if (style != rhs.style) return false;
+    if (strokeWidth != rhs.strokeWidth) return false;
+    if (type == Type::None) return true;
+    const Shape::RoundRect& lRect = shape.roundRect;
+    const Shape::RoundRect& rRect = rhs.shape.roundRect;
+
+    if (lRect.width != rRect.width) return false;
+    if (lRect.height != rRect.height) return false;
+    if (lRect.rx != rRect.rx) return false;
+    return lRect.ry == rRect.ry;
+}
+
 hash_t TessellationCache::Description::hash() const {
-    uint32_t hash = JenkinsHashMix(0, type);
+    uint32_t hash = JenkinsHashMix(0, static_cast<int>(type));
     hash = JenkinsHashMix(hash, aa);
     hash = JenkinsHashMix(hash, cap);
     hash = JenkinsHashMix(hash, style);
@@ -77,17 +97,23 @@ void TessellationCache::Description::setupMatrixAndPaint(Matrix4* matrix, SkPain
 
 TessellationCache::ShadowDescription::ShadowDescription()
         : nodeKey(nullptr) {
-    memset(&matrixData, 0, 16 * sizeof(float));
+    memset(&matrixData, 0, sizeof(matrixData));
 }
 
-TessellationCache::ShadowDescription::ShadowDescription(const void* nodeKey, const Matrix4* drawTransform)
+TessellationCache::ShadowDescription::ShadowDescription(const SkPath* nodeKey, const Matrix4* drawTransform)
         : nodeKey(nodeKey) {
-    memcpy(&matrixData, drawTransform->data, 16 * sizeof(float));
+    memcpy(&matrixData, drawTransform->data, sizeof(matrixData));
+}
+
+bool TessellationCache::ShadowDescription::operator==(
+        const TessellationCache::ShadowDescription& rhs) const {
+    return nodeKey == rhs.nodeKey
+            && memcmp(&matrixData, &rhs.matrixData, sizeof(matrixData)) == 0;
 }
 
 hash_t TessellationCache::ShadowDescription::hash() const {
     uint32_t hash = JenkinsHashMixBytes(0, (uint8_t*) &nodeKey, sizeof(const void*));
-    hash = JenkinsHashMixBytes(hash, (uint8_t*) &matrixData, 16 * sizeof(float));
+    hash = JenkinsHashMixBytes(hash, (uint8_t*) &matrixData, sizeof(matrixData));
     return JenkinsHashWhiten(hash);
 }
 
@@ -428,7 +454,7 @@ static VertexBuffer* tessellateRoundRect(const TessellationCache::Description& d
 TessellationCache::Buffer* TessellationCache::getRoundRectBuffer(
         const Matrix4& transform, const SkPaint& paint,
         float width, float height, float rx, float ry) {
-    Description entry(Description::kRoundRect, transform, paint);
+    Description entry(Description::Type::RoundRect, transform, paint);
     entry.shape.roundRect.width = width;
     entry.shape.roundRect.height = height;
     entry.shape.roundRect.rx = rx;

libs/hwui/TessellationCache.h

@@ -52,10 +52,10 @@ public:
     typedef Pair<VertexBuffer*, VertexBuffer*> vertexBuffer_pair_t;
 
     struct Description {
-        DESCRIPTION_TYPE(Description);
+        HASHABLE_TYPE(Description);
-        enum Type {
+        enum class Type {
-            kNone,
+            None,
-            kRoundRect,
+            RoundRect,
         };
 
         Type type;
@@ -76,18 +76,16 @@ public:
 
         Description();
         Description(Type type, const Matrix4& transform, const SkPaint& paint);
-        hash_t hash() const;
         void setupMatrixAndPaint(Matrix4* matrix, SkPaint* paint) const;
     };
 
     struct ShadowDescription {
-        DESCRIPTION_TYPE(ShadowDescription);
+        HASHABLE_TYPE(ShadowDescription);
-        const void* nodeKey;
+        const SkPath* nodeKey;
         float matrixData[16];
 
         ShadowDescription();
-        ShadowDescription(const void* nodeKey, const Matrix4* drawTransform);
+        ShadowDescription(const SkPath* nodeKey, const Matrix4* drawTransform);
-        hash_t hash() const;
     };
 
     class ShadowTask : public Task<vertexBuffer_pair_t> {

libs/hwui/utils/Macros.h

@@ -23,12 +23,10 @@
         Type(const Type&) = delete; \
         void operator=(const Type&) = delete
 
-#define DESCRIPTION_TYPE(Type) \
+#define HASHABLE_TYPE(Type) \
-        int compare(const Type& rhs) const { return memcmp(this, &rhs, sizeof(Type));} \
+        bool operator==(const Type& other) const; \
-        bool operator==(const Type& other) const { return compare(other) == 0; } \
+        hash_t hash() const; \
-        bool operator!=(const Type& other) const { return compare(other) != 0; } \
+        bool operator!=(const Type& other) const { return !(*this == other); } \
-        friend inline int strictly_order_type(const Type& lhs, const Type& rhs) { return lhs.compare(rhs) < 0; } \
-        friend inline int compare_type(const Type& lhs, const Type& rhs) { return lhs.compare(rhs); } \
         friend inline hash_t hash_type(const Type& entry) { return entry.hash(); }
 
 #define REQUIRE_COMPATIBLE_LAYOUT(Type) \