Proactive SkPixelRef invalidation

src/core/SkMessageBus.h

+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkMessageBus_DEFINED
+#define SkMessageBus_DEFINED
+
+#include "SkOnce.h"
+#include "SkTDArray.h"
+#include "SkThread.h"
+#include "SkTypes.h"
+
+template <typename Message>
+class SkMessageBus : SkNoncopyable {
+public:
+    // Post a message to be received by all Inboxes for this Message type.  Threadsafe.
+    static void post(const Message& m);
+
+    class Inbox {
+    public:
+        Inbox();
+        ~Inbox();

[scroggo, 2013/10/22 21:04:02]

Should this be virtual?

[mtklein, 2013/10/23 15:28:10]

Nope.  No virtual methods on Inbox (or SkMessageBus itself).

+
+        // Make messages contain all the messages we've received since the last call.  Threadsafe.

[scroggo, 2013/10/22 21:04:02]

I find this comment hard to read - it would help to differentiate messages the
object from messages the concept. It also doesn't quite describe the function.
messages (the object) will only contain the messages received since the last
call, and fMessages is cleared.

[bsalomon, 2013/10/23 13:52:57]

"The array messages"?

[mtklein, 2013/10/23 15:28:10]

Better?

[bsalomon, 2013/10/23 15:32:27]

ok by me

[scroggo, 2013/10/23 15:39:19]

Yes.

+        void poll(SkTDArray<Message>* messages);
+
+    private:
+        SkTDArray<Message> fMessages;
+        SkMutex            fMessagesMutex;
+
+        friend class SkMessageBus;
+        void receive(const Message& m);  // SkMessageBus is a friend only to call this.
+    };
+
+private:
+    SkMessageBus();
+    static SkMessageBus* Get();
+    static void New(SkMessageBus**);
+
+    SkTDArray<Inbox*> fInboxes;
+    SkMutex           fInboxesMutex;
+};
+
+//   ----------------------- Implementation of SkMessageBus::Inbox -----------------------
+
+template<typename Message>
+SkMessageBus<Message>::Inbox::Inbox() {
+    // Register ourselves with the corresponding message bus.
+    SkMessageBus<Message>* bus = SkMessageBus<Message>::Get();
+    SkAutoMutexAcquire lock(bus->fInboxesMutex);
+    bus->fInboxes.push(this);
+}
+
+template<typename Message>
+SkMessageBus<Message>::Inbox::~Inbox() {
+    // Remove ourselves from the corresponding message bus.
+    SkMessageBus<Message>* bus = SkMessageBus<Message>::Get();
+    SkAutoMutexAcquire lock(bus->fInboxesMutex);
+    // This is a cheaper fInboxes.remove(fInboxes.find(this)) when order doesn't matter.

[scroggo, 2013/10/22 21:04:02]

Clever. Should this be an option on SkTDArray?

[bsalomon, 2013/10/23 13:52:57]

It already is: removeShuffle

[mtklein, 2013/10/23 15:28:10]

Ah!  Missed that.  Updated.

+    for (int i = 0; i < bus->fInboxes.count(); i++) {
+        if (this == bus->fInboxes[i]) {
+            bus->fInboxes[i] = bus->fInboxes.top();
+            bus->fInboxes.pop();
+            break;
+        }
+    }
+}
+
+template<typename Message>
+void SkMessageBus<Message>::Inbox::receive(const Message& m) {
+    SkAutoMutexAcquire lock(fMessagesMutex);
+    fMessages.push(m);
+}
+
+template<typename Message>
+void SkMessageBus<Message>::Inbox::poll(SkTDArray<Message>* messages) {
+    messages->reset();

[scroggo, 2013/10/22 21:04:02]

Should we check for NULL or assert non NULL?

[mtklein, 2013/10/23 15:28:10]

Yes.  Added an assert.

+    SkAutoMutexAcquire lock(fMessagesMutex);
+    messages->swap(fMessages);
+}
+
+//   ----------------------- Implementation of SkMessageBus -----------------------
+
+template <typename Message>
+SkMessageBus<Message>::SkMessageBus() {}
+
+template <typename Message>
+/*static*/ void SkMessageBus<Message>::New(SkMessageBus<Message>** bus) {
+    *bus = new SkMessageBus<Message>();
+}
+
+template <typename Message>
+/*static*/ SkMessageBus<Message>* SkMessageBus<Message>::Get() {
+    // The first time this method is called, create the singleton bus for this message type.
+    static SkMessageBus<Message>* bus = NULL;
+    static bool initialized = false;
+    SK_DECLARE_STATIC_MUTEX(mutex);
+
+    sk_once(&initialized, &mutex, &New, &bus);
+
+    SkASSERT(bus != NULL);
+    return bus;
+}
+
+template <typename Message>
+/*static*/ void SkMessageBus<Message>::post(const Message& m) {

[bsalomon, 2013/10/23 13:52:57]

Is the Get()/post() pattern useful? Could we just have static Post() that
internally calls Get(). Or maybe you're trying to avoid hitting that mutex a lot
for callers that have multiple messages to post in succession?

[mtklein, 2013/10/23 15:28:10]

Ah, this may just be an issue with my tenuous grasp of Skia style.  This is
itself that very same static Post that internally calls Get.  Renamed.

[bsalomon, 2013/10/23 15:32:27]

Ha... I should have realized that from the impl.

+    SkMessageBus<Message>* bus = SkMessageBus<Message>::Get();
+    SkAutoMutexAcquire lock(bus->fInboxesMutex);
+    for (int i = 0; i < bus->fInboxes.count(); i++) {
+        bus->fInboxes[i]->receive(m);
+    }
+}
+
+#endif  // SkMessageBus_DEFINED