SkValue: implementation, unit test

src/core/SkValue.cpp

+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include <unordered_map>
+#include <vector>
+
+#include "SkData.h"
+#include "SkMatrix.h"
+#include "SkValue.h"
+
+class SkValue::Obj {
+public:
+    void set(SkValue::Key k, SkValue&& v) { fMap[k] = std::move(v);  }
+    const SkValue* get(SkValue::Key k) const {
+        auto it = fMap.find(k);
+        return it != fMap.end() ? &it->second : nullptr;
+    }
+    void foreach(std::function<void(Key, const SkValue&)> fn) const {
+        for (const auto& pair : fMap) {
+            fn(pair.first, pair.second);
+        }
+    }
+
+private:
+    std::unordered_map<SkValue::Key, SkValue> fMap;
+};
+
+class SkValue::Arr {
+public:
+    size_t length() const { return fVec.size(); }
+    void append(SkValue&& val) { fVec.emplace_back(std::move(val)); }
+    const SkValue& at(size_t index) const {
+        SkASSERT(index < fVec.size());
+        return fVec.at(index);

[mtklein, 2016/01/20 14:20:58]

vector::at does bounds checks.  I think you want fVec[index].

[hal.canary, 2016/01/20 15:14:35]

done

+    }
+
+private:
+    std::vector<SkValue> fVec;
+};
+
+SkValue::SkValue() : fType(Null) {}
+
+SkValue::SkValue(const SkValue& o) {
+    memcpy(this, &o, sizeof(o));
+    if (this->isData()) {
+        fBytes->ref();
+    } else if (this->isObject()) {
+        fObject = new Obj(*fObject);
+    } else if (Array == fType) {
+        fArray = new Arr(*fArray);
+    }
+}
+
+SkValue::SkValue(SkValue&& o) {
+    memcpy(this, &o, sizeof(o));
+    new (&o) SkValue();
+}
+
+SkValue& SkValue::operator=(const SkValue& o) {
+    if (this != &o) {
+        this->~SkValue();
+        new (this) SkValue(o);
+    }
+    return *this;
+}
+
+SkValue& SkValue::operator=(SkValue&& o) {
+    if (this != &o) {
+        this->~SkValue();
+        new (this) SkValue(std::move(o));
+    }
+    return *this;
+}
+
+SkValue::~SkValue() {
+    if (this->isData()) {
+        fBytes->unref();
+    } else if (this->isObject()) {
+        delete fObject;
+    } else if (Array == fType) {
+        delete fArray;
+    }
+}
+
+#define FN(NAME, LNAME, TYPE)             \
+    SkValue SkValue::From##NAME(TYPE x) { \
+        SkValue v;                        \
+        v.fType = NAME;                   \
+        v.f##NAME = x;                    \
+        return v;                         \
+    }                                     \
+    TYPE SkValue::LNAME() const {         \
+        SkASSERT(NAME == fType);          \
+        return f##NAME;                   \
+    }
+FN(S32, s32, int32_t)
+FN(U32, u32, uint32_t)
+FN(F32, f32, float)
+#undef FN
+
+SkValue SkValue::FromBytes(SkData* data) {
+    if (!data) {
+        return SkValue();
+    }
+    SkValue v;
+    v.fType = Bytes;
+    v.fBytes = SkRef(data);
+    return v;
+}
+
+SkValue SkValue::Object(SkValue::Type t) {
+    SkValue v;
+    v.fType = t;
+    SkASSERT(v.isObject());
+    v.fObject = new Obj;
+    return v;
+}
+
+SkValue SkValue::ValueArray() {
+    SkValue v;
+    v.fType = Array;
+    v.fArray = new Arr;
+    return v;
+}
+
+SkData* SkValue::bytes() const {
+    SkASSERT(this->isData());
+    return fBytes;
+}
+
+void SkValue::set(SkValue::Key k, SkValue v) {
+    SkASSERT(this->isObject());
+    fObject->set(k, std::move(v));
+}
+
+void SkValue::foreach(std::function<void(Key, const SkValue&)> fn) const {
+    SkASSERT(this->isObject());
+    fObject->foreach(fn);
+}
+
+size_t SkValue::length() const {
+    SkASSERT(Array == fType);
+    return fArray->length();
+}
+
+const SkValue& SkValue::at(size_t index) const {
+    SkASSERT(Array == fType);
+    return fArray->at(index);
+}
+
+void SkValue::append(SkValue val) {
+    SkASSERT(Array == fType);
+    fArray->append(std::move(val));
+}
+
+#define PTR_IS_ALIGNED_TO(PTR, TYPE) \
+    (0 == (reinterpret_cast<uintptr_t>(PTR) & (sizeof(TYPE) - 1)))
+
+#define FN(NAME, MNAME, TYPE)                              \

[mtklein, 2016/01/20 14:20:58]

Seems like we can get most of the concision by using templates instead of
macros?  I don't mind using macros when needed, but we're not #quoting any terms
or doing anything that really requires it here.

template <typename T>
/*private, static*/ SkValue SkValue::FromTs(SkValue::Type type, SkData* data) {
   SkValue val;
   val.fType = type;
   val.fBytes = SkRef(data);

   SkASSERT(val.isData());
   SkASSERT(0 == (reinterpret_cast<uintptr_t>(data->bytes()) & (sizeof(T)-1));
   return val;
}

SkValue SkValue::FromU16s(SkData* data) { return FromTs<uint16_t>(U16s, data); }
SkValue SkValue::FromU32s(SkData* data) { return FromTs<uint32_t>(U16s, data); }
SkValue SkValue::FromF32s(SkData* data) { return FromTs<   float>(F32s, data); }

template <typename T>
/*private*/ SkValue SkValue::ts(SkValue::Type type, int* count) const {
  SkASSERT(fType == type);
  if (count) { *count = fBytes->size() / sizeof(T); }
  return (const T*)fBytes->data();
}

const uint16_t* SkValue::u16s(int* count) const { return
this->ts<uint16_t>(U16s, count); }
const uint32_t* SkValue::u32s(int* count) const { return
this->ts<uint32_t>(U32s, count); }
const    float* SkValue::f32s(int* count) const { return this->ts<float>(F32s,
count); }

This could be even more concise if we make a compile time mapping back and forth
between type and Type.

[hal.canary, 2016/01/20 15:14:34]

DONE.  awesome.

+    SkValue SkValue::From##NAME(SkData* data) {            \
+        SkValue val;                                       \
+        val.fType = NAME;                                  \
+        SkASSERT(val.isData());                            \
+        val.fBytes = SkRef(data);                          \
+        SkASSERT(PTR_IS_ALIGNED_TO(data->bytes(), TYPE));  \
+        return val;                                        \
+    }                                                      \
+    const TYPE* SkValue::MNAME(int* count) const {         \
+        SkASSERT(NAME == fType);                           \
+        SkASSERT(count);                                   \
+        *count = fBytes->size() / sizeof(TYPE);            \
+        return static_cast<const TYPE*>(fBytes->data());   \
+    }
+FN(U16s, u16s, uint16_t)
+FN(U32s, u32s, uint32_t)
+FN(F32s, f32s, float)
+#undef FN

[mtklein, 2016/01/20 14:20:58]

#undef PTR_IS_ALIGNED_TO also, or inline it into FN?

[hal.canary, 2016/01/20 15:14:35]

done

+
+////////////////////////////////////////////////////////////////////////////////
+
+template<> SkValue SkValue::Encode<SkMatrix>(const SkMatrix& mat) {
+    auto val = SkValue::Object(SkValue::Matrix);
+    for (int i = 0; i < 9; ++i) {
+        if (mat[i] != SkMatrix::I()[i]) {
+            val.set((SkValue::Key)i, SkValue::FromF32(mat[i]));

[mtklein, 2016/01/20 14:20:59]

val.set(i, ...) ?  Isn't this cast implicit?

[hal.canary, 2016/01/20 15:14:34]

done.

+        }
+    }
+    return val;
+}
+
+template<> bool SkValue::Decode<SkMatrix>(const SkValue& val, SkMatrix* m){
+    SkASSERT(val.type() == SkValue::Matrix);
+    if (val.type() != SkValue::Matrix) {
+        return false;
+    }
+    *m = SkMatrix::I();
+    bool good = true;
+    val.foreach([&](SkValue::Key key, const SkValue& v) {
+            if (key < 9) {

[mtklein, 2016/01/20 14:20:59]

the indentation from 203-211 is pretty weird

[hal.canary, 2016/01/20 15:14:34]

let me write that simpler.

+                if (v.type() != SkValue::F32) {
+                    SkASSERT(false);
+                    good = false;
+                } else {
+                    (*m)[(int)key] = v.f32();

[mtklein, 2016/01/20 14:20:58]

(*m)[key] ?  Isn't this cast implicit?

[hal.canary, 2016/01/20 15:14:35]

done.

+                }
+            }

[mtklein, 2016/01/20 14:20:59]

I guess,

} else {
   good = false;
}

here too?

[hal.canary, 2016/01/20 15:14:35]

right.

+        });
+    return good;
+}