add image interface, remove lodepng from video_core/core, address more comments, fix comments

remove unnecessary conversion

src/citra/CMakeLists.txt

@@ -8,13 +8,15 @@ add_executable(citra
     default_ini.h
     emu_window/emu_window_sdl2.cpp
     emu_window/emu_window_sdl2.h
+    generic_image_interface.cpp
+    generic_image_interface.h
     resource.h
 )
 
 create_target_directory_groups(citra)
 
 target_link_libraries(citra PRIVATE common core input_common network)
-target_link_libraries(citra PRIVATE inih glad)
+target_link_libraries(citra PRIVATE inih glad lodepng)
 if (MSVC)
     target_link_libraries(citra PRIVATE getopt)
 endif()

src/citra/citra.cpp

@@ -40,6 +40,7 @@
 #include "core/loader/loader.h"
 #include "core/movie.h"
 #include "core/settings.h"
+#include "generic_image_interface.h"
 #include "network/network.h"
 #include "video_core/video_core.h"
 
@@ -342,6 +343,9 @@ int main(int argc, char** argv) {
     // Register frontend applets
     Frontend::RegisterDefaultApplets();
 
+    // Register generic image interface
+    Core::System::GetInstance().RegisterImageInterface(std::make_shared<GenericImageInterface>());
+
     std::unique_ptr<EmuWindow_SDL2> emu_window{std::make_unique<EmuWindow_SDL2>(fullscreen)};
 
     Core::System& system{Core::System::GetInstance()};

src/citra/generic_image_interface.cpp

@@ -0,0 +1,29 @@
+// Copyright 2019 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#include <lodepng.h>
+#include "common/logging/log.h"
+#include "generic_image_interface.h"
+
+bool GenericImageInterface::DecodePNG(std::vector<u8>& dst, u32& width, u32& height,
+                                      const std::string& path) {
+    u32 lodepng_ret = lodepng::decode(dst, width, height, path);
+    if (lodepng_ret) {
+        LOG_CRITICAL(Frontend, "Failed to decode {} because {}", path,
+                     lodepng_error_text(lodepng_ret));
+        return false;
+    }
+    return true;
+}
+
+bool GenericImageInterface::EncodePNG(const std::string& path, const std::vector<u8>& src,
+                                      u32 width, u32 height) {
+    u32 lodepng_ret = lodepng::encode(path, src, width, height);
+    if (lodepng_ret) {
+        LOG_CRITICAL(Frontend, "Failed to encode {} because {}", path,
+                     lodepng_error_text(lodepng_ret));
+        return false;
+    }
+    return true;
+}

src/citra/generic_image_interface.h

@@ -0,0 +1,14 @@
+// Copyright 2019 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#pragma once
+
+#include "core/frontend/image_interface.h"
+
+class GenericImageInterface final : public Frontend::ImageInterface {
+public:
+    bool DecodePNG(std::vector<u8>& dst, u32& width, u32& height, const std::string& path) override;
+    bool EncodePNG(const std::string& path, const std::vector<u8>& src, u32 width,
+                   u32 height) override;
+};

src/citra_qt/CMakeLists.txt

@@ -142,6 +142,8 @@ add_executable(citra-qt
     multiplayer/validation.h
     uisettings.cpp
     uisettings.h
+    qt_image_interface.cpp
+    qt_image_interface.h
     updater/updater.cpp
     updater/updater.h
     updater/updater_p.h

src/citra_qt/main.cpp

@@ -51,6 +51,7 @@
 #include "citra_qt/main.h"
 #include "citra_qt/multiplayer/state.h"
 #include "citra_qt/uisettings.h"
+#include "citra_qt/qt_image_interface.h"
 #include "citra_qt/updater/updater.h"
 #include "citra_qt/util/clickable_label.h"
 #include "common/common_paths.h"
@@ -2059,6 +2060,9 @@ int main(int argc, char* argv[]) {
     Core::System::GetInstance().RegisterMiiSelector(std::make_shared<QtMiiSelector>(main_window));
     Core::System::GetInstance().RegisterSoftwareKeyboard(std::make_shared<QtKeyboard>(main_window));
 
+    // Register Qt image interface
+    Core::System::GetInstance().RegisterImageInterface(std::make_shared<QtImageInterface>());
+
     main_window.show();
 
     QObject::connect(&app, &QGuiApplication::applicationStateChanged, &main_window,

src/citra_qt/qt_image_interface.cpp

@@ -0,0 +1,46 @@
+// Copyright 2019 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#include <QBuffer>
+#include <QImage>
+#include <QString>
+#include "common/logging/log.h"
+#include "core/frontend/image_interface.h"
+#include "qt_image_interface.h"
+
+bool QtImageInterface::DecodePNG(std::vector<u8>& dst, u32& width, u32& height,
+                                 const std::string& path) {
+    QImage image(QString::fromStdString(path));
+
+    if (image.isNull()) {
+        LOG_ERROR(Frontend, "Failed to open {} for decoding", path);
+        return false;
+    }
+    width = image.width();
+    height = image.height();
+
+    // Write RGBA8 to vector
+    for (u32 y = 1; y < image.height() + 1; y++) {
+        for (u32 x = 1; x < image.width() + 1; x++) {
+            const QColor pixel(image.pixel(y, x));
+            dst.push_back(pixel.red());
+            dst.push_back(pixel.green());
+            dst.push_back(pixel.blue());
+            dst.push_back(pixel.alpha());
+        }
+    }
+
+    return true;
+}
+
+bool QtImageInterface::EncodePNG(const std::string& path, const std::vector<u8>& src, u32 width,
+                                 u32 height) {
+    QImage image(src.data(), width, height, QImage::Format_RGBA8888);
+
+    if (!image.save(QString::fromStdString(path))) {
+        LOG_ERROR(Frontend, "Failed to save {}", path);
+        return false;
+    }
+    return true;
+}

src/citra_qt/qt_image_interface.h

@@ -0,0 +1,14 @@
+// Copyright 2019 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#pragma once
+
+#include "core/frontend/image_interface.h"
+
+class QtImageInterface final : public Frontend::ImageInterface {
+public:
+    bool DecodePNG(std::vector<u8>& dst, u32& width, u32& height, const std::string& path) override;
+    bool EncodePNG(const std::string& path, const std::vector<u8>& src, u32 width,
+                   u32 height) override;
+};

src/core/CMakeLists.txt

@@ -102,6 +102,7 @@ add_library(core STATIC
     frontend/emu_window.h
     frontend/framebuffer_layout.cpp
     frontend/framebuffer_layout.h
+    frontend/image_interface.h
     frontend/input.h
     frontend/mic.h
     frontend/mic.cpp
@@ -460,7 +461,7 @@ endif()
 create_target_directory_groups(core)
 
 target_link_libraries(core PUBLIC common PRIVATE audio_core network video_core)
-target_link_libraries(core PUBLIC Boost::boost PRIVATE cryptopp fmt open_source_archives lodepng)
+target_link_libraries(core PUBLIC Boost::boost PRIVATE cryptopp fmt open_source_archives)
 if (ENABLE_WEB_SERVICE)
     target_compile_definitions(core PRIVATE -DENABLE_WEB_SERVICE)
     target_link_libraries(core PRIVATE web_service)

src/core/core.cpp

@@ -4,7 +4,6 @@
 
 #include <memory>
 #include <utility>
-#include <lodepng.h>
 #include "audio_core/dsp_interface.h"
 #include "audio_core/hle/hle.h"
 #include "audio_core/lle/lle.h"
@@ -126,15 +125,14 @@ void System::PreloadCustomTextures() {
                 u32 png_height;
                 std::vector<u8> decoded_png;
 
-                u32 lodepng_ret =
+                if (registered_image_interface->DecodePNG(decoded_png, png_width, png_height,
-                    lodepng::decode(decoded_png, png_width, png_height, file.physicalName);
+                                                          file.physicalName)) {
-                if (lodepng_ret) {
-                    LOG_CRITICAL(Render_OpenGL, "Failed to preload custom texture: {}",
-                                 lodepng_error_text(lodepng_ret));
-                } else {
                     LOG_INFO(Render_OpenGL, "Preloaded custom texture from {}", file.physicalName);
                     Common::FlipRGBA8Texture(decoded_png, png_width, png_height);
                     custom_tex_cache->CacheTexture(hash, decoded_png, png_width, png_height);
+                } else {
+                    // Error should be reported by frontend
+                    LOG_CRITICAL(Render_OpenGL, "Failed to preload custom texture");
                 }
             }
         }
@@ -404,6 +402,10 @@ void System::RegisterSoftwareKeyboard(std::shared_ptr<Frontend::SoftwareKeyboard
     registered_swkbd = std::move(swkbd);
 }
 
+void System::RegisterImageInterface(std::shared_ptr<Frontend::ImageInterface> image_interface) {
+    registered_image_interface = std::move(image_interface);
+}
+
 void System::Shutdown() {
     // Log last frame performance stats
     const auto perf_results = GetAndResetPerfStats();

src/core/core.h

@@ -10,6 +10,7 @@
 #include "core/custom_tex_cache.h"
 #include "core/frontend/applets/mii_selector.h"
 #include "core/frontend/applets/swkbd.h"
+#include "core/frontend/image_interface.h"
 #include "core/loader/loader.h"
 #include "core/memory.h"
 #include "core/perf_stats.h"
@@ -256,6 +257,14 @@ public:
         return registered_swkbd;
     }
 
+    /// Image interface
+
+    void RegisterImageInterface(std::shared_ptr<Frontend::ImageInterface> image_interface);
+
+    std::shared_ptr<Frontend::ImageInterface> GetImageInterface() const {
+        return registered_image_interface;
+    }
+
 private:
     /**
      * Initialize the emulated system.
@@ -300,6 +309,9 @@ private:
     /// Custom texture cache system
     std::unique_ptr<Core::CustomTexCache> custom_tex_cache;
 
+    /// Image interface
+    std::shared_ptr<Frontend::ImageInterface> registered_image_interface;
+
     /// RPC Server for scripting support
     std::unique_ptr<RPC::RPCServer> rpc_server;
 

src/core/custom_tex_cache.cpp

@@ -8,9 +8,9 @@
 #include "custom_tex_cache.h"
 
 namespace Core {
-CustomTexCache::CustomTexCache() {}
+CustomTexCache::CustomTexCache() = default;
 
-CustomTexCache::~CustomTexCache() {}
+CustomTexCache::~CustomTexCache() = default;
 
 bool CustomTexCache::IsTextureDumped(u64 hash) const {
     return dumped_textures.find(hash) != dumped_textures.end();

src/core/frontend/image_interface.h

@@ -0,0 +1,28 @@
+// Copyright 2019 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#pragma once
+
+#include <string>
+#include <vector>
+#include "common/common_types.h"
+#include "common/logging/log.h"
+
+namespace Frontend {
+
+class ImageInterface {
+public:
+    // Error logging should be handled by the frontend
+    virtual bool DecodePNG(std::vector<u8>& dst, u32& width, u32& height, const std::string& path) {
+        LOG_CRITICAL(Frontend, "Attempted to decode PNG without an image interface!");
+        return false;
+    };
+    virtual bool EncodePNG(const std::string& path, const std::vector<u8>& src, u32 width,
+                           u32 height) {
+        LOG_CRITICAL(Frontend, "Attempted to encode PNG without an image interface!");
+        return false;
+    };
+};
+
+} // namespace Frontend

src/video_core/CMakeLists.txt

@@ -92,7 +92,7 @@ endif()
 create_target_directory_groups(video_core)
 
 target_link_libraries(video_core PUBLIC common core)
-target_link_libraries(video_core PRIVATE glad nihstro-headers lodepng)
+target_link_libraries(video_core PRIVATE glad nihstro-headers)
 
 if (ARCHITECTURE_x86_64)
     target_link_libraries(video_core PUBLIC xbyak)

src/video_core/renderer_opengl/gl_rasterizer_cache.cpp

@@ -14,7 +14,6 @@
 #include <vector>
 #include <boost/range/iterator_range.hpp>
 #include <glad/glad.h>
-#include <lodepng.h>
 #include "common/alignment.h"
 #include "common/bit_field.h"
 #include "common/color.h"
@@ -856,10 +855,11 @@ void CachedSurface::FlushGLBuffer(PAddr flush_start, PAddr flush_end) {
     }
 }
 
-bool CachedSurface::LoadCustomTextures(u64 tex_hash, Core::CustomTexInfo& tex_info,
+bool CachedSurface::LoadCustomTexture(u64 tex_hash, Core::CustomTexInfo& tex_info,
-                                       Common::Rectangle<u32>& custom_rect) {
+                                      Common::Rectangle<u32>& custom_rect) {
     bool result = false;
     auto& custom_tex_cache = Core::System::GetInstance().CustomTexCache();
+    const auto& image_interface = Core::System::GetInstance().GetImageInterface();
     const std::string load_path =
         fmt::format("{}textures/{:016X}/tex1_{}x{}_{:016X}_{}.png",
                     FileUtil::GetUserPath(FileUtil::UserPath::LoadDir),
@@ -868,17 +868,15 @@ bool CachedSurface::LoadCustomTextures(u64 tex_hash, Core::CustomTexInfo& tex_in
 
     if (!custom_tex_cache.IsTextureCached(tex_hash)) {
         if (FileUtil::Exists(load_path)) {
-            u32 lodepng_ret =
+            if (image_interface->DecodePNG(tex_info.tex, tex_info.width, tex_info.height,
-                lodepng::decode(tex_info.tex, tex_info.width, tex_info.height, load_path);
+                                           load_path)) {
-            if (lodepng_ret) {
-                LOG_CRITICAL(Render_OpenGL, "Failed to load custom texture: {}",
-                             lodepng_error_text(lodepng_ret));
-            } else {
                 LOG_INFO(Render_OpenGL, "Loaded custom texture from {}", load_path);
                 Common::FlipRGBA8Texture(tex_info.tex, tex_info.width, tex_info.height);
                 custom_tex_cache.CacheTexture(tex_hash, tex_info.tex, tex_info.width,
                                               tex_info.height);
                 result = true;
+            } else {
+                LOG_CRITICAL(Render_OpenGL, "Failed to load custom texture");
             }
         }
     } else {
@@ -896,27 +894,30 @@ bool CachedSurface::LoadCustomTextures(u64 tex_hash, Core::CustomTexInfo& tex_in
     return result;
 }
 
-bool CachedSurface::GetDumpPath(u64 tex_hash, std::string& path) {
+std::optional<std::string> CachedSurface::GetDumpPath(u64 tex_hash) {
     auto& custom_tex_cache = Core::System::GetInstance().CustomTexCache();
+    std::string path;
+
     path =
         fmt::format("{}textures/{:016X}/", FileUtil::GetUserPath(FileUtil::UserPath::DumpDir),
                     Core::System::GetInstance().Kernel().GetCurrentProcess()->codeset->program_id);
     if (!FileUtil::CreateFullPath(path)) {
         LOG_ERROR(Render, "Unable to create {}", path);
-        return false;
+        return {};
     }
 
     path += fmt::format("tex1_{}x{}_{:016X}_{}.png", width, height, tex_hash,
                         static_cast<u32>(pixel_format));
     if (!custom_tex_cache.IsTextureDumped(tex_hash) && !FileUtil::Exists(path)) {
         custom_tex_cache.SetTextureDumped(tex_hash);
-        return true;
+        return path;
     }
-    return false;
+    return {};
 }
 
 void CachedSurface::DumpTexture(GLuint target_tex, const std::string& dump_path) {
     // Dump texture to RGBA8 and encode as PNG
+    const auto& image_interface = Core::System::GetInstance().GetImageInterface();
     LOG_INFO(Render_OpenGL, "Dumping texture to {}", dump_path);
     std::vector<u8> decoded_texture;
     decoded_texture.resize(width * height * 4);
@@ -924,11 +925,8 @@ void CachedSurface::DumpTexture(GLuint target_tex, const std::string& dump_path)
     glGetTexImage(GL_TEXTURE_2D, 0, GL_RGBA, GL_UNSIGNED_BYTE, &decoded_texture[0]);
     glBindTexture(GL_TEXTURE_2D, 0);
     Common::FlipRGBA8Texture(decoded_texture, width, height);
-    u32 png_error = lodepng::encode(dump_path, decoded_texture, width, height);
+    if (image_interface->EncodePNG(dump_path, decoded_texture, width, height))
-    if (png_error) {
+        LOG_CRITICAL(Render_OpenGL, "Failed to save decoded texture");
-        LOG_CRITICAL(Render_OpenGL, "Failed to save decoded texture! {}",
-                     lodepng_error_text(png_error));
-    }
 }
 
 MICROPROFILE_DEFINE(OpenGL_TextureUL, "OpenGL", "Texture Upload", MP_RGB(128, 192, 64));
@@ -955,10 +953,13 @@ void CachedSurface::UploadGLTexture(const Common::Rectangle<u32>& rect, GLuint r
         tex_hash = Common::ComputeHash64(gl_buffer.get(), gl_buffer_size);
 
     if (Settings::values.custom_textures)
-        use_custom_tex = LoadCustomTextures(tex_hash, custom_tex_info, custom_rect);
+        use_custom_tex = LoadCustomTexture(tex_hash, custom_tex_info, custom_rect);
 
     if (Settings::values.dump_textures && !use_custom_tex)
-        dump_tex = GetDumpPath(tex_hash, dump_path);
+        if (auto temp_dump_path = GetDumpPath(tex_hash)) {
+            dump_path = temp_dump_path.value();
+            dump_tex = true;
+        }
 
     // Load data from memory to the surface
     GLint x0 = static_cast<GLint>(custom_rect.left);
@@ -1168,8 +1169,7 @@ Surface FindMatch(const SurfaceCache& surface_cache, const SurfaceParams& params
                     surface->type != SurfaceType::Fill)
                     return;
 
-                // Found a match, update only if this is better than the previous
+                // Found a match, update only if this is better than the previous one
-                // one
                 auto UpdateMatch = [&] {
                     match_surface = surface;
                     match_valid = is_valid;
@@ -1368,8 +1368,8 @@ Surface RasterizerCacheOpenGL::GetSurface(const SurfaceParams& params, ScaleMatc
     if (surface == nullptr) {
         u16 target_res_scale = params.res_scale;
         if (match_res_scale != ScaleMatch::Exact) {
-            // This surface may have a subrect of another surface with a higher
+            // This surface may have a subrect of another surface with a higher res_scale, find it
-            // res_scale, find it to adjust our params
+            // to adjust our params
             SurfaceParams find_params = params;
             Surface expandable = FindMatch<MatchFlags::Expand | MatchFlags::Invalid>(
                 surface_cache, find_params, match_res_scale);
@@ -1470,8 +1470,7 @@ SurfaceRect_Tuple RasterizerCacheOpenGL::GetSurfaceSubRect(const SurfaceParams&
         // Can't have gaps in a surface
         new_params.width = aligned_params.stride;
         new_params.UpdateParams();
-        // GetSurface will create the new surface and possibly adjust res_scale if
+        // GetSurface will create the new surface and possibly adjust res_scale if necessary
-        // necessary
         surface = GetSurface(new_params, match_res_scale, load_if_create);
     } else if (load_if_create) {
         ValidateSurface(surface, aligned_params.addr, aligned_params.size);
@@ -1628,10 +1627,9 @@ const CachedTextureCube& RasterizerCacheOpenGL::GetTextureCube(const TextureCube
             if (surface) {
                 face.watcher = surface->CreateWatcher();
             } else {
-                // Can occur when texture address is invalid. We mark the watcher
+                // Can occur when texture address is invalid. We mark the watcher with nullptr in
-                // with nullptr in this case and the content of the face wouldn't
+                // this case and the content of the face wouldn't get updated. These are usually
-                // get updated. These are usually leftover setup in the texture unit
+                // leftover setup in the texture unit and games are not supposed to draw using them.
-                // and games are not supposed to draw using them.
                 face.watcher = nullptr;
             }
         }
@@ -1731,8 +1729,7 @@ SurfaceSurfaceRect_Tuple RasterizerCacheOpenGL::GetFramebufferSurfaces(
     auto color_vp_interval = color_params.GetSubRectInterval(viewport_clamped);
     auto depth_vp_interval = depth_params.GetSubRectInterval(viewport_clamped);
 
-    // Make sure that framebuffers don't overlap if both color and depth are being
+    // Make sure that framebuffers don't overlap if both color and depth are being used
-    // used
     if (using_color_fb && using_depth_fb &&
         boost::icl::length(color_vp_interval & depth_vp_interval)) {
         LOG_CRITICAL(Render_OpenGL, "Color and depth framebuffer memory regions overlap; "
@@ -1920,10 +1917,9 @@ void RasterizerCacheOpenGL::FlushRegion(PAddr addr, u32 size, Surface flush_surf
     SurfaceRegions flushed_intervals;
 
     for (auto& pair : RangeFromInterval(dirty_regions, flush_interval)) {
-        // small sizes imply that this most likely comes from the cpu, flush the
+        // small sizes imply that this most likely comes from the cpu, flush the entire region
-        // entire region the point is to avoid thousands of small writes every frame
+        // the point is to avoid thousands of small writes every frame if the cpu decides to access
-        // if the cpu decides to access that region, anything higher than 8 you're
+        // that region, anything higher than 8 you're guaranteed it comes from a service
-        // guaranteed it comes from a service
         const auto interval = size <= 8 ? pair.first : pair.first & flush_interval;
         auto& surface = pair.second;
 
@@ -1980,8 +1976,7 @@ void RasterizerCacheOpenGL::InvalidateRegion(PAddr addr, u32 size, const Surface
             cached_surface->invalid_regions.insert(interval);
             cached_surface->InvalidateAllWatcher();
 
-            // Remove only "empty" fill surfaces to avoid destroying and recreating
+            // Remove only "empty" fill surfaces to avoid destroying and recreating OGL textures
-            // OGL textures
             if (cached_surface->type == SurfaceType::Fill &&
                 cached_surface->IsSurfaceFullyInvalid()) {
                 remove_surfaces.emplace(cached_surface);
@@ -2050,8 +2045,8 @@ void RasterizerCacheOpenGL::UpdatePagesCachedCount(PAddr addr, u32 size, int del
     const u32 page_start = addr >> Memory::PAGE_BITS;
     const u32 page_end = page_start + num_pages;
 
-    // Interval maps will erase segments if count reaches 0, so if delta is negative
+    // Interval maps will erase segments if count reaches 0, so if delta is negative we have to
-    // we have to subtract after iterating
+    // subtract after iterating
     const auto pages_interval = PageMap::interval_type::right_open(page_start, page_end);
     if (delta > 0)
         cached_pages.add({pages_interval, delta});

src/video_core/renderer_opengl/gl_rasterizer_cache.h

@@ -379,9 +379,9 @@ struct CachedSurface : SurfaceParams, std::enable_shared_from_this<CachedSurface
     void FlushGLBuffer(PAddr flush_start, PAddr flush_end);
 
     // Custom texture loading and dumping
-    bool LoadCustomTextures(u64 tex_hash, Core::CustomTexInfo& tex_info,
+    bool LoadCustomTexture(u64 tex_hash, Core::CustomTexInfo& tex_info,
-                            Common::Rectangle<u32>& custom_rect);
+                           Common::Rectangle<u32>& custom_rect);
-    bool GetDumpPath(u64 tex_hash, std::string& path);
+    std::optional<std::string> GetDumpPath(u64 tex_hash);
     void DumpTexture(GLuint target_tex, const std::string& dump_path);
 
     // Upload/Download data in gl_buffer in/to this surface's texture