SkValue: to/from SkImage

src/effects/SkToFromValue.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 "SkArithmeticMode.h"
+#include "SkData.h"
+#include "SkImage.h"
 #include "SkLerpXfermode.h"
 #include "SkMatrix.h"
 #include "SkPixelXorXfermode.h"
 #include "SkToFromValue.h"
 #include "SkValueKeys.h"
 #include "SkXfermode.h"
 
 ////////////////////////////////////////////////////////////////////////////////
 
 #define REQUIRE(cond) do { if (!(cond)) { SkASSERT(false); return false; } } while (false)
@@ skipping 15 matching lines @@
     *x = val.s32();
     return true;
 }
 
 template<> bool SkFromValue<uint32_t>(const SkValue& val, uint32_t* x) {
     REQUIRE(val.type() == SkValue::U32);
     *x = val.u32();
     return true;
 }
 
+template<> bool SkFromValue<SkData*>(const SkValue& val, SkData** dst) {
+    REQUIRE(   val.type() == SkValue::Bytes

[mtklein, 2016/01/21 21:21:30]

Guess we'd better put isData() out in public for now.

[hal.canary, 2016/01/21 22:03:23]

done

+            || val.type() == SkValue::U16s
+            || val.type() == SkValue::U32s
+            || val.type() == SkValue::F32s);
+    *dst = val.bytes();  // no ref taken
+    return true;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 template<> SkValue SkToValue<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(i, SkValue::FromF32(mat[i]));
         }
     }
     return val;
@@ skipping 66 matching lines @@
         case SkValue::ArithmeticXfermode: return from_value_ArithmeticXfermode(val, dst);
         case SkValue::LerpXfermode:       return from_value_LerpXfermode(val, dst);
         case SkValue::PixelXorXfermode:   return from_value_PixelXorXfermode(val, dst);
         case SkValue::ProcCoeffXfermode:  return from_value_ProcCoeffXfermode(val, dst);
         default:                          REQUIRE(false);
     }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
+enum { kEncoded, kUniqueID };  // SkValue::Image

[mtklein, 2016/01/21 21:21:29]

?

[hal.canary, 2016/01/21 22:03:23]

moved.

+
+template<> SkValue SkToValue<SkImage>(const SkImage* image) {
+    SkAutoTUnref<SkData> encoded(image->encode());

[mtklein, 2016/01/22 14:09:18]

you know, this is probably the most controversial line of the whole CL.
it's not clear to me that we should be encoding these images.
that means,
   1) they must be encoded and decoded if you're going to use SkValues, which
may not be necessary
   2) the client gets no choice in how that encoding or decoding happens

Let's leave images for later to think more about.

+    if (!encoded || 0 == encoded->size()) {

[mtklein, 2016/01/21 21:21:30]

why 0 == encoded->size()?

[hal.canary, 2016/01/21 22:03:23]

removed.

+        return SkValue();
+    }
+    auto val = SkValue::Object(SkValue::Image);
+    val.set(kEncoded, SkValue::FromBytes(encoded));
+    val.set(kUniqueID, SkValue::FromU32(image->uniqueID()));

[mtklein, 2016/01/21 21:21:30]

This seems like a bad idea, especially if we ignore it in SkFromValue().

Those uniqueIDs are unique to the current process, but have nothing to do with
the image itself.

[hal.canary, 2016/01/21 22:03:23]

a serializer can use them to de-dup images.  Or anything else with a uniqueID.

[mtklein, 2016/01/22 14:09:18]

this unique ID is not part of the value of the image.  it does not belong in its
value.

+    return val;
+}
+
+template<> bool SkFromValue< SkAutoTUnref<SkImage> >(

[mtklein, 2016/01/21 21:21:30]

template<> bool SkFromValue<SkAutoTUnref<SkImage>>(const SkValue& val,
SkAutoTUnref<SkImage>* dst) {

is 100 columns.

[hal.canary, 2016/01/21 22:03:23]

yes but my *editor* doesn't know about how C++11 allows >> to sometimes not be a
shift operator.

[mtklein, 2016/01/22 14:09:18]

are you seriously proposing that we write code strangely to coddle the
eccentricities of the editor you have chosen to use?

+        const SkValue& val, SkAutoTUnref<SkImage>* dst) {
+    REQUIRE(val.type() == SkValue::Image);
+    SkData* encoded;
+    REQUIRE(getT(val, kEncoded, &encoded));
+    dst->reset(SkImage::NewFromEncoded(encoded));
+    return true;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
 #undef REQUIRE