Workaround sfml image loading thread race condition

See: https://github.com/SFML/SFML/issues/1711

Also some other smaller changes

5 files changed, 79 insertions, 41 deletions

diff --git a/src/AsyncImageLoader.cpp b/src/AsyncImageLoader.cpp
index ec9ae9a..98c7fee 100644
--- a/src/AsyncImageLoader.cpp
+++ b/src/AsyncImageLoader.cpp
@@ -2,23 +2,14 @@
 #include "../include/DownloadUtils.hpp"
 #include "../include/ImageUtils.hpp"
 #include "../include/Scale.hpp"
+#include "../include/SfmlFixes.hpp"
 #include "../external/hash-library/sha256.h"
 #include <assert.h>
 
 namespace QuickMedia {
-    static sf::Vector2f to_vec2f(const sf::Vector2u &vec) {
-        return sf::Vector2f(vec.x, vec.y);
-    }
-
-    static sf::Vector2f to_vec2f(const sf::Vector2i &vec) {
-        return sf::Vector2f(vec.x, vec.y);
-    }
-
-    static sf::Vector2u to_vec2u(const sf::Vector2f &vec) {
-        return sf::Vector2u(vec.x, vec.y);
-    }
-
     // Linear interpolation
+    // TODO: Is this implementation ok? it always uses +1 offset for interpolation but if we were to resize an image with near 1x1 scaling
+    // then it would be slightly blurry
     static void copy_resize(const sf::Image &source, sf::Image &destination, sf::Vector2u destination_size) {
         const sf::Vector2u source_size = source.getSize();
         if(source_size.x == 0 || source_size.y == 0 || destination_size.x == 0 || destination_size.y == 0)
@@ -120,14 +111,14 @@ namespace QuickMedia {
                 }
 
                 thumbnail_load_data.thumbnail_data->image = std::make_unique<sf::Image>();
-                if(thumbnail_load_data.thumbnail_data->image->loadFromFile(thumbnail_load_data.thumbnail_path.data)) {
+                if(load_image_from_file(*thumbnail_load_data.thumbnail_data->image, thumbnail_load_data.thumbnail_path.data)) {
                     fprintf(stderr, "Loaded %s from thumbnail cache\n", thumbnail_load_data.path.data.c_str());
                     thumbnail_load_data.thumbnail_data->loading_state = LoadingState::FINISHED_LOADING;
                     continue;
                 }
 
                 if(thumbnail_load_data.local) {
-                    if(thumbnail_load_data.thumbnail_data->image->loadFromFile(thumbnail_load_data.path.data)
+                    if(load_image_from_file(*thumbnail_load_data.thumbnail_data->image, thumbnail_load_data.path.data)
                         && thumbnail_load_data.resize_target_size.x != 0 && thumbnail_load_data.resize_target_size.y != 0)
                     {
                         create_thumbnail_if_thumbnail_smaller_than_image(thumbnail_load_data.path.data, thumbnail_load_data.thumbnail_path, thumbnail_load_data.thumbnail_data.get(), thumbnail_load_data.resize_target_size);
@@ -194,21 +185,21 @@ namespace QuickMedia {
         // TODO: Keep the thread running and use conditional variable instead to sleep until a new image should be loaded. Same in ImageViewer.
         download_image_thread[free_index] = std::thread([this, free_index, thumbnail_path, url, local, resize_target_size, thumbnail_data, use_tor]() mutable {
             thumbnail_data->image = std::make_unique<sf::Image>();
-            if(thumbnail_data->image->loadFromFile(thumbnail_path.data)) {
+            if(load_image_from_file(*thumbnail_data->image, thumbnail_path.data)) {
                 fprintf(stderr, "Loaded %s from thumbnail cache\n", url.c_str());
                 thumbnail_data->loading_state = LoadingState::FINISHED_LOADING;
                 loading_image[free_index] = false;
                 return;
             } else {
                 if(local) {
-                    if(!thumbnail_data->image->loadFromFile(url)) {
+                    if(!load_image_from_file(*thumbnail_data->image, url)) {
                         thumbnail_data->loading_state = LoadingState::FINISHED_LOADING;
                         loading_image[free_index] = false;
                         return;
                     }
                 } else {
                     // Use the same path as the thumbnail path, since it wont be overwritten if the image is smaller than the thumbnail target size
-                    if(download_to_file(url, thumbnail_path.data, {}, use_tor, true) != DownloadResult::OK || !thumbnail_data->image->loadFromFile(thumbnail_path.data)) {
+                    if(download_to_file(url, thumbnail_path.data, {}, use_tor, true) != DownloadResult::OK || !load_image_from_file(*thumbnail_data->image, thumbnail_path.data)) {
                         thumbnail_data->loading_state = LoadingState::FINISHED_LOADING;
                         loading_image[free_index] = false;
                         return;
diff --git a/src/Body.cpp b/src/Body.cpp
index fd29d17..6b7826d 100644
--- a/src/Body.cpp
+++ b/src/Body.cpp
@@ -59,8 +59,8 @@ namespace QuickMedia {
     {
         progress_text.setFillColor(sf::Color::White);
         replies_text.setFillColor(sf::Color(129, 162, 190));
-        thumbnail_resize_target_size.x = 250;
-        thumbnail_resize_target_size.y = 141;
+        thumbnail_max_size.x = 250;
+        thumbnail_max_size.y = 141;
         item_background.setFillColor(sf::Color(55, 60, 68));
         sf::Vector2f loading_icon_size(loading_icon.getTexture()->getSize().x, loading_icon.getTexture()->getSize().y);
         loading_icon.setOrigin(loading_icon_size.x * 0.5f, loading_icon_size.y * 0.5f);
@@ -387,8 +387,8 @@ namespace QuickMedia {
             }
 
             update_dirty_state(item.get(), size);
-            float item_height = get_item_height(item.get());
             item->last_drawn_time = elapsed_time_sec;
+            float item_height = get_item_height(item.get());
 
             // This is needed here rather than above the loop, since update_dirty_text cant be called inside scissor because it corrupts the text for some reason
             glEnable(GL_SCISSOR_TEST);
@@ -509,9 +509,9 @@ namespace QuickMedia {
     sf::Vector2i Body::get_item_thumbnail_size(BodyItem *item) const {
         sf::Vector2i content_size;
         if(item->thumbnail_size.x > 0 && item->thumbnail_size.y > 0)
-            content_size = clamp_to_size(item->thumbnail_size, thumbnail_resize_target_size);
+            content_size = clamp_to_size(item->thumbnail_size, thumbnail_max_size);
         else
-            content_size = thumbnail_resize_target_size;
+            content_size = thumbnail_max_size;
         return content_size;
     }
 
diff --git a/src/ImageViewer.cpp b/src/ImageViewer.cpp
index fc7b758..c704485 100644
--- a/src/ImageViewer.cpp
+++ b/src/ImageViewer.cpp
@@ -1,6 +1,7 @@
 #include "../include/ImageViewer.hpp"
 #include "../include/Notification.hpp"
 #include "../include/Storage.hpp"
+#include "../include/SfmlFixes.hpp"
 #include "../plugins/Manga.hpp"
 #include <cmath>
 #include <SFML/Window/Event.hpp>
@@ -50,7 +51,7 @@ namespace QuickMedia {
             image_loader_thread.join();
         image_loader_thread = std::thread([this, image_data, path]() mutable {
             auto image = std::make_unique<sf::Image>();
-            if(image->loadFromFile(path.data)) {
+            if(load_image_from_file(*image, path.data)) {
                 image_data->image = std::move(image);
                 image_data->image_status = ImageStatus::LOADED;
             } else {
diff --git a/src/QuickMedia.cpp b/src/QuickMedia.cpp
index f944e7d..48ef01b 100644
--- a/src/QuickMedia.cpp
+++ b/src/QuickMedia.cpp
@@ -19,6 +19,7 @@
 #include "../include/base64_url.hpp"
 #include "../include/Entry.hpp"
 #include "../include/NetUtils.hpp"
+#include "../include/SfmlFixes.hpp"
 
 #include <assert.h>
 #include <cmath>
@@ -926,7 +927,7 @@ namespace QuickMedia {
         }
 
         for(Tab &tab : tabs) {
-            tab.body->thumbnail_resize_target_size = tab.page->get_max_thumbnail_size();
+            tab.body->thumbnail_max_size = tab.page->get_thumbnail_max_size();
         }
 
         const Json::Value *json_chapters = &Json::Value::nullSingleton();
@@ -1837,7 +1838,7 @@ namespace QuickMedia {
 
         if(get_file_type(image_path) == FileType::REGULAR && upscaled_ok) {
             sf::Image image;
-            if(image.loadFromFile(image_path.data)) {
+            if(load_image_from_file(image, image_path.data)) {
                 if(image_texture.loadFromImage(image)) {
                     image_texture.setSmooth(true);
                     image_texture.generateMipmap();
@@ -2346,7 +2347,8 @@ namespace QuickMedia {
             DownloadResult download_image_result = download_to_string(challenge_info.payload_url, payload_image_data, {}, is_tor_enabled());
             if(download_image_result == DownloadResult::OK) {
                 std::lock_guard<std::mutex> lock(captcha_image_mutex);
-                if(captcha_texture.loadFromMemory(payload_image_data.data(), payload_image_data.size())) {
+                sf::Image captcha_image;
+                if(load_image_from_memory(captcha_image, payload_image_data.data(), payload_image_data.size()) && captcha_texture.loadFromImage(captcha_image)) {
                     captcha_texture.setSmooth(true);
                     captcha_sprite.setTexture(captcha_texture, true);
                     challenge_description_text.setString(challenge_info.description);
@@ -2704,7 +2706,8 @@ namespace QuickMedia {
                     downloading_image = false;
                     image_data = load_image_future.get();
                     
-                    if(attached_image_texture->loadFromMemory(image_data.data(), image_data.size())) {
+                    sf::Image attached_image;
+                    if(load_image_from_memory(attached_image, image_data.data(), image_data.size()) && attached_image_texture->loadFromImage(attached_image)) {
                         attached_image_texture->setSmooth(true);
                         //attached_image_texture->generateMipmap();
                         attached_image_sprite.setTexture(*attached_image_texture, true);
@@ -2943,7 +2946,7 @@ namespace QuickMedia {
         messages_tab.type = ChatTabType::MESSAGES;
         messages_tab.body = std::make_unique<Body>(this, font.get(), bold_font.get(), cjk_font.get(), loading_icon);
         messages_tab.body->draw_thumbnails = true;
-        messages_tab.body->thumbnail_resize_target_size = CHAT_MESSAGE_THUMBNAIL_MAX_SIZE;
+        messages_tab.body->thumbnail_max_size = CHAT_MESSAGE_THUMBNAIL_MAX_SIZE;
         messages_tab.body->thumbnail_mask_shader = &circle_mask_shader;
         //messages_tab.body->line_separator_color = sf::Color::Transparent;
         messages_tab.text = sf::Text("Messages", *font, tab_text_size);
@@ -2967,8 +2970,28 @@ namespace QuickMedia {
         RoomData *current_room = nullptr;
         bool is_window_focused = window.hasFocus();
 
+        // Returns -1 if no rooms or no unread rooms
+        auto find_top_body_position_for_unread_room = [&tabs](BodyItem *item_to_swap, int start_index) {
+            for(int i = start_index; i < (int)tabs[ROOMS_TAB_INDEX].body->items.size(); ++i) {
+                const auto &body_item = tabs[ROOMS_TAB_INDEX].body->items[i];
+                if(static_cast<RoomData*>(body_item->userdata)->last_message_read || body_item.get() == item_to_swap)
+                    return i;
+            }
+            return -1;
+        };
+
+        // Returns -1 if no rooms or all rooms have unread mentions
+        auto find_top_body_position_for_mentioned_room = [&tabs](BodyItem *item_to_swap, int start_index) {
+            for(int i = start_index; i < (int)tabs[ROOMS_TAB_INDEX].body->items.size(); ++i) {
+                const auto &body_item = tabs[ROOMS_TAB_INDEX].body->items[i];
+                if(!static_cast<RoomData*>(body_item->userdata)->has_unread_mention || body_item.get() == item_to_swap)
+                    return i;
+            }
+            return -1;
+        };
+
         auto process_new_room_messages = 
-            [this, &selected_tab, &current_room, &is_window_focused, &tabs]
+            [this, &selected_tab, &current_room, &is_window_focused, &tabs, &find_top_body_position_for_unread_room, &find_top_body_position_for_mentioned_room]
             (RoomSyncMessages &room_sync_messages, bool is_first_sync) mutable
         {
             for(auto &[room, messages] : room_sync_messages) {
@@ -2982,7 +3005,6 @@ namespace QuickMedia {
                 }
             }
 
-            bool has_unread_messages = false;
             for(auto &[room, messages] : room_sync_messages) {
                 if(messages.empty())
                     continue;
@@ -3010,13 +3032,32 @@ namespace QuickMedia {
                 assert(room_body_item);
 
                 if(last_unread_message) {
-                    has_unread_messages = true;
                     std::string room_desc = "Unread: " + matrix->message_get_author_displayname(last_unread_message) + ": " + extract_first_line(last_unread_message->body, 150);
                     if(room->has_unread_mention)
                         room_desc += "\n** You were mentioned **"; // TODO: Better notification?
                     room_body_item->set_description(std::move(room_desc));
                     room_body_item->set_title_color(sf::Color(255, 100, 100));
                     room->last_message_read = false;
+
+                    // Swap order of rooms in body list to put rooms with mentions at the top and then unread messages and then all the other rooms
+                    // TODO: Optimize with hash map instead of linear search? or cache the index
+                    Body *rooms_body = tabs[ROOMS_TAB_INDEX].body.get();
+                    int room_body_index = rooms_body->get_index_by_body_item(room_body_item);
+                    if(room_body_index != -1) {
+                        std::shared_ptr<BodyItem> body_item = rooms_body->items[room_body_index];
+                        int body_swap_index = -1;
+                        if(room->has_unread_mention)
+                            body_swap_index = find_top_body_position_for_mentioned_room(body_item.get(), 0);
+                        else if(!room->last_message_read)
+                            body_swap_index = find_top_body_position_for_unread_room(body_item.get(), 0);
+                        if(body_swap_index != -1 && body_swap_index != room_body_index) {
+                            rooms_body->items.erase(rooms_body->items.begin() + room_body_index);
+                            if(body_swap_index < room_body_index)
+                                rooms_body->items.insert(rooms_body->items.begin() + body_swap_index, std::move(body_item));
+                            else
+                                rooms_body->items.insert(rooms_body->items.begin() + (body_swap_index - 1), std::move(body_item));
+                        }
+                    }
                 } else if(is_first_sync) {
                     Message *last_unread_message = nullptr;
                     for(auto it = messages.rbegin(), end = messages.rend(); it != end; ++it) {
@@ -3029,16 +3070,6 @@ namespace QuickMedia {
                         room_body_item->set_description(matrix->message_get_author_displayname(last_unread_message) + ": " + extract_first_line(last_unread_message->body, 150));
                 }
             }
-
-            if(has_unread_messages) {
-                std::sort(tabs[ROOMS_TAB_INDEX].body->items.begin(), tabs[ROOMS_TAB_INDEX].body->items.end(), [](std::shared_ptr<BodyItem> &item1, std::shared_ptr<BodyItem> &item2) {
-                    RoomData *item1_room = static_cast<RoomData*>(item1->userdata);
-                    RoomData *item2_room = static_cast<RoomData*>(item2->userdata);
-                    int item1_presence_sum = (int)!item1_room->last_message_read + (int)item1_room->has_unread_mention;
-                    int item2_presence_sum = (int)!item2_room->last_message_read + (int)item2_room->has_unread_mention;
-                    return item1_presence_sum > item2_presence_sum;
-                });
-            }
         };
 
         enum class ChatState {
diff --git a/src/SfmlFixes.cpp b/src/SfmlFixes.cpp
new file mode 100644
index 0000000..d6d4b17
--- /dev/null
+++ b/src/SfmlFixes.cpp
@@ -0,0 +1,15 @@
+#include "../include/SfmlFixes.hpp"
+#include <mutex>
+
+static std::mutex mutex;
+namespace QuickMedia {
+    bool load_image_from_file(sf::Image &image, const std::string &filepath) {
+        std::lock_guard<std::mutex> lock(mutex);
+        return image.loadFromFile(filepath);
+    }
+
+    bool load_image_from_memory(sf::Image &image, const void *data, size_t size) {
+        std::lock_guard<std::mutex> lock(mutex);
+        return image.loadFromMemory(data, size);
+    }
+}
\ No newline at end of file