From d1e134d988cbc2fb48b3a39b10438214ff91bb5b Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 19 Sep 2024 19:13:47 -0300 Subject: [PATCH] Don't read the magic number of all objects to show a backup description (#4610) Now we lazily initialize the description of each backup on each session. This means that only when we have to display the item on the screen (onPaint) we'll ask for the description/doc info (width/height/color mode, etc.). We've also removed the check of all magic numbers of every single object in the backup when we only need the doc description. --- src/app/crash/read_document.cpp | 7 ++++++- src/app/crash/session.cpp | 26 ++++++++++++++------------ src/app/crash/session.h | 4 ++-- src/app/ui/data_recovery_view.cpp | 10 +++++++++- 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/app/crash/read_document.cpp b/src/app/crash/read_document.cpp index 6dfbd957d..199e3b79e 100644 --- a/src/app/crash/read_document.cpp +++ b/src/app/crash/read_document.cpp @@ -91,7 +91,12 @@ public: if (!id || !ver) continue; // Error converting strings to ID/ver - if (!check_magic_number(base::join_path(m_dir, fn))) { + // Checking for the magic number of each file takes a long time, + // we can guess that all files are valid when there is no + // m_taskToken, i.e. when we have to just show the description + // of the doc in the list of backups. + if (m_taskToken && + !check_magic_number(base::join_path(m_dir, fn))) { RECO_TRACE("RECO: Ignoring invalid file %s (no magic number)\n", fn.c_str()); continue; } diff --git a/src/app/crash/session.cpp b/src/app/crash/session.cpp index 616bf3d7e..ae8afb8b7 100644 --- a/src/app/crash/session.cpp +++ b/src/app/crash/session.cpp @@ -44,22 +44,24 @@ static const char* kOpenFilename = "open"; // File that indicates if the documen Session::Backup::Backup(const std::string& dir) : m_dir(dir) { - DocumentInfo info; - read_document_info(dir, info); - - m_fn = info.filename; - m_desc = - fmt::format("{} Sprite {}x{}, {} {}", - info.mode == ColorMode::RGB ? "RGB": - info.mode == ColorMode::GRAYSCALE ? "Grayscale": - info.mode == ColorMode::INDEXED ? "Indexed": - info.mode == ColorMode::BITMAP ? "Bitmap": "Unknown", - info.width, info.height, info.frames, - info.frames == 1 ? "frame": "frames"); } std::string Session::Backup::description(const bool withFullPath) const { + // Lazy initialize description and filename. + if (m_desc.empty()) { + DocumentInfo info; + read_document_info(m_dir, info); + m_fn = info.filename; + m_desc = + fmt::format("{} Sprite {}x{}, {} {}", + info.mode == ColorMode::RGB ? "RGB": + info.mode == ColorMode::GRAYSCALE ? "Grayscale": + info.mode == ColorMode::INDEXED ? "Indexed": + info.mode == ColorMode::BITMAP ? "Bitmap": "Unknown", + info.width, info.height, info.frames, + info.frames == 1 ? "frame": "frames"); + } return fmt::format("{}: {}", m_desc, withFullPath ? m_fn: diff --git a/src/app/crash/session.h b/src/app/crash/session.h index 4ae1280d9..07a3c9a03 100644 --- a/src/app/crash/session.h +++ b/src/app/crash/session.h @@ -35,8 +35,8 @@ namespace crash { std::string description(const bool withFullPath) const; private: std::string m_dir; - std::string m_desc; - std::string m_fn; + mutable std::string m_desc; + mutable std::string m_fn; }; using BackupPtr = std::shared_ptr; using Backups = std::vector; diff --git a/src/app/ui/data_recovery_view.cpp b/src/app/ui/data_recovery_view.cpp index 56dd4c8f3..d85d5b1a0 100644 --- a/src/app/ui/data_recovery_view.cpp +++ b/src/app/ui/data_recovery_view.cpp @@ -58,7 +58,6 @@ public: : m_session(session) , m_backup(backup) , m_task(nullptr) { - updateText(); } crash::Session* session() const { return m_session; } @@ -150,6 +149,15 @@ public: } private: + void onPaint(PaintEvent& ev) override { + // The text is lazily initialized. So we read the backup data only + // when we have to show its information. + if (text().empty()) { + updateText(); + } + ListItem::onPaint(ev); + } + void onSizeHint(SizeHintEvent& ev) override { ListItem::onSizeHint(ev); gfx::Size sz = ev.sizeHint();