RemoteServer code cleanup and logcat cleanup.

This fixes two different logcat messages that would occur when scrubbing
a slider in the UI:

- "Discarding message, message queue overflow."
- "Downloading model..." would repeat itself needlessly

android/filament-utils-android/src/main/java/com/google/android/filament/utils/RemoteServer.java

@@ -43,10 +43,31 @@ public class RemoteServer {
         if (mNativeObject == 0) throw new IllegalStateException("Couldn't create RemoteServer");
     }
 
+    /**
+     * Checks if a download is currently in progress and returns its label.
+     * Returns null if nothing is being downloaded.
+     */
     public @Nullable String peekIncomingLabel() {
         return nPeekIncomingLabel(mNativeObject);
     }
 
+    /**
+     * Checks if a peeked message has JSON content.
+     */
+    public static boolean isJson(@Nullable String label) {
+        return label != null && label.endsWith(".json");
+    }
+
+    /**
+     * Checks if a peeked message has binary content.
+     */
+    public static boolean isBinary(@Nullable String label) {
+        return label != null && !label.endsWith(".json");
+    }
+
+    /**
+     * Pops a message off the incoming queue or returns null if there are no unread messages.
+     */
     public @Nullable ReceivedMessage acquireReceivedMessage() {
         int length = nPeekReceivedBufferLength(mNativeObject);
         if (length == 0) {

android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt

@@ -53,6 +53,7 @@ class MainActivity : Activity() {
     private var remoteServer: RemoteServer? = null
     private var statusToast: Toast? = null
     private var statusText: String? = null
+    private var latestDownload: String? = null
     private val automation = AutomationEngine()
 
     @SuppressLint("ClickableViewAccessibility")
@@ -258,15 +259,21 @@ class MainActivity : Activity() {
 
             modelViewer.render(frameTimeNanos)
 
-            val label = remoteServer?.peekIncomingLabel()
-            if (label != null && !label.endsWith(".json")) {
-                Log.i(TAG, "Downloading $label")
-                setStatusText("Downloading $label")
+            // Check if a new download is in progress. If so, let the user know with toast.
+            val currentDownload = remoteServer?.peekIncomingLabel()
+            if (RemoteServer.isBinary(currentDownload) && currentDownload != latestDownload) {
+                latestDownload = currentDownload
+                Log.i(TAG, "Downloading $currentDownload")
+                setStatusText("Downloading $currentDownload")
             }
 
+            // Check if a new message has been fully received from the client.
             val message = remoteServer?.acquireReceivedMessage()
             if (message != null) {
-                if (message.label.endsWith(".json")) {
+                if (message.label == latestDownload) {
+                    latestDownload = null
+                }
+                if (RemoteServer.isJson(message.label)) {
                     loadSettings(message)
                 } else {
                     loadModelData(message)

libs/viewer/include/viewer/RemoteServer.h

@@ -55,20 +55,37 @@ public:
     RemoteServer(int port = 8082);
     ~RemoteServer();
     bool isValid() const { return mMessageSender; }
+
+    /**
+     * Checks if a download is currently in progress and returns its label.
+     * Returns null if nothing is being downloaded.
+     */
     char const* peekIncomingLabel() const;
-    ReceivedMessage const* peekReceivedMessage() const;
+
+    /**
+     * Pops a message off the incoming queue or returns null if there are no unread messages.
+     *
+     * After examining its contents, users should free the message with releaseReceivedMessage.
+     */
     ReceivedMessage const* acquireReceivedMessage();
+
+    /**
+     * Frees the memory that holds the contents of a received message.
+     */
     void releaseReceivedMessage(ReceivedMessage const* message);
+
     void sendMessage(const Settings& settings);
     void sendMessage(const char* label, const char* buffer, size_t bufsize);
 
+    // For internal use (makes JNI simpler)
+    ReceivedMessage const* peekReceivedMessage() const;
+
 private:
     void enqueueReceivedMessage(ReceivedMessage* message);
     void setIncomingMessage(ReceivedMessage* message);
     MessageSender* mMessageSender = nullptr;
     MessageReceiver* mMessageReceiver = nullptr;
     size_t mNextMessageUid = 0;
-    size_t mOldestMessageUid = 0;
     static const size_t kMessageCapacity = 4;
     ReceivedMessage* mReceivedMessages[kMessageCapacity] = {};
     ReceivedMessage* mIncomingMessage = nullptr;

libs/viewer/src/RemoteServer.cpp

@@ -81,26 +81,36 @@ char const * RemoteServer::peekIncomingLabel() const {
 
 ReceivedMessage const * RemoteServer::peekReceivedMessage() const {
     std::lock_guard lock(mReceivedMessagesMutex);
-    const size_t oldest = mOldestMessageUid;
-    for (auto msg : mReceivedMessages) { if (msg && msg->messageUid == oldest) return msg; }
-    return nullptr;
+
+    // Find the oldest message in the queue by looking for the lowest id.
+    // Note that this queue is not a ring buffer, it's just a tiny sparse array.
+    ReceivedMessage const* oldest = nullptr;
+    for (auto msg : mReceivedMessages) {
+        if (msg && (!oldest || msg->messageUid < oldest->messageUid)) oldest = msg;
+    }
+
+    return oldest;
 }
 
 ReceivedMessage const * RemoteServer::acquireReceivedMessage() {
     std::lock_guard lock(mReceivedMessagesMutex);
-    const size_t oldest = mOldestMessageUid;
+
+    // Find the oldest message in the queue by looking for the lowest id.
+    ReceivedMessage** oldest = nullptr;
     for (auto& msg : mReceivedMessages) {
-        if (msg && msg->messageUid == oldest) {
-            if (msg == mIncomingMessage) {
-                mIncomingMessage = nullptr;
-            }
-            auto result = msg;
-            msg = nullptr;
-            ++mOldestMessageUid;
-            return result;
-        }
+        if (msg && (!oldest || msg->messageUid < (*oldest)->messageUid)) oldest = &msg;
     }
-    return nullptr;
+    if (!oldest) return nullptr;
+
+    // If this message is the most recent download, then mark the download as completed.
+    ReceivedMessage const * result = *oldest;
+    if (result == mIncomingMessage) {
+        mIncomingMessage = nullptr;
+    }
+
+    // Replace the message slot with null and return the message to the caller.
+    *oldest = nullptr;
+    return result;
 }
 
 void RemoteServer::setIncomingMessage(ReceivedMessage* message) {
@@ -110,13 +120,31 @@ void RemoteServer::setIncomingMessage(ReceivedMessage* message) {
 
 void RemoteServer::enqueueReceivedMessage(ReceivedMessage* message) {
     std::lock_guard lock(mReceivedMessagesMutex);
-    for (auto& msg : mReceivedMessages) {
-        if (!msg) {
+
+    // Check if any unread messages have the same label as the incoming message. If so, it is safe
+    // to discard the old message and snarf its slot.
+    ReceivedMessage** empty_slot = nullptr;
+    for (auto& old_message : mReceivedMessages) {
+        if (old_message == nullptr) {
+            empty_slot = &old_message;
+            continue;
+        }
+        if (!strcmp(old_message->label, message->label)) {
+            releaseReceivedMessage(old_message);
             message->messageUid = mNextMessageUid++;
-            msg = message;
+            old_message = message;
             return;
         }
     }
+
+    // Otherwise use any empty slot in the queue.
+    if (empty_slot) {
+        message->messageUid = mNextMessageUid++;
+        *empty_slot = message;
+        return;
+    }
+
+    // If there are no empty slots, then discard the message. This basically never happens.
     slog.e << "Discarding message, message queue overflow." << io::endl;
 }