From 9f219e5201efc12254f146ee3beb34a657de3307 Mon Sep 17 00:00:00 2001 From: 5428384822a-create <274500894+5428384822a-create@users.noreply.github.com> Date: Wed, 29 Apr 2026 03:25:52 +0800 Subject: [PATCH 1/3] fix: clean warnings and harden image decoding --- CMakeLists.txt | 1 + src/core/Application.cpp | 14 +- src/core/ImageDecoder.cpp | 210 +++++++++++++++++++++++------ src/core/ImagePipeline.cpp | 3 +- src/rendering/Direct2DRenderer.cpp | 2 +- src/ui/GestureHandler.cpp | 16 ++- src/ui/ThumbnailStrip.cpp | 3 - 7 files changed, 191 insertions(+), 58 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c8f5b69..12e0df9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,6 +45,7 @@ add_compile_options( /Zc:preprocessor /wd4100 /wd4201 + /wd4324 ) if(MSVC) diff --git a/src/core/Application.cpp b/src/core/Application.cpp index 5d5a2bb..ae4f5a2 100644 --- a/src/core/Application.cpp +++ b/src/core/Application.cpp @@ -26,7 +26,7 @@ namespace { void DebugLog(const char* msg) { static FILE* f = nullptr; if (!f) { - f = fopen("debug_log.txt", "w"); + fopen_s(&f, "debug_log.txt", "w"); } if (f) { fprintf(f, "%s\n", msg); @@ -1343,7 +1343,8 @@ void Application::SaveScanCache(const std::vector& results) // [24..31] reserved, already zero // Write entire buffer in one call - FILE* f = _wfopen(filePath.c_str(), L"wb"); + FILE* f = nullptr; + _wfopen_s(&f, filePath.c_str(), L"wb"); if (!f) return; fwrite(header, 1, kHeaderSize, f); @@ -1365,7 +1366,8 @@ std::vector Application::LoadScanCache() if (filePath.empty()) return results; try { - FILE* f = _wfopen(filePath.c_str(), L"rb"); + FILE* f = nullptr; + _wfopen_s(&f, filePath.c_str(), L"rb"); if (!f) return results; // Get file size @@ -1456,7 +1458,8 @@ void Application::LoadFolderProfiles() auto path = GetFolderProfilePath(); if (path.empty()) return; - FILE* f = _wfopen(path.c_str(), L"rb"); + FILE* f = nullptr; + _wfopen_s(&f, path.c_str(), L"rb"); if (!f) return; // Header: "FPROF" + version(4) + count(4) @@ -1497,7 +1500,8 @@ void Application::SaveFolderProfiles() std::lock_guard lock(scanMutex_); std::filesystem::create_directories(path.parent_path()); - FILE* f = _wfopen(path.c_str(), L"wb"); + FILE* f = nullptr; + _wfopen_s(&f, path.c_str(), L"wb"); if (!f) return; fwrite("FPROF", 1, 5, f); diff --git a/src/core/ImageDecoder.cpp b/src/core/ImageDecoder.cpp index e3c6fbe..ff05ab1 100644 --- a/src/core/ImageDecoder.cpp +++ b/src/core/ImageDecoder.cpp @@ -3,8 +3,45 @@ #include #include #include +#include +#include #include +namespace { +bool CalculateBgraLayout(uint32_t width, uint32_t height, size_t& dataSize, UINT& stride, UINT& bufferSize) +{ + if (width == 0 || height == 0) { + return false; + } + + constexpr uint64_t kBytesPerPixel = 4; + uint64_t stride64 = static_cast(width) * kBytesPerPixel; + uint64_t size64 = stride64 * static_cast(height); + + if (stride64 > std::numeric_limits::max() || + size64 > std::numeric_limits::max() || + size64 > std::numeric_limits::max()) { + return false; + } + + stride = static_cast(stride64); + bufferSize = static_cast(size64); + dataSize = static_cast(size64); + return true; +} + +uint32_t ScaleToMax(uint32_t value, uint32_t maxValue, uint32_t maxSize) +{ + if (maxValue == 0 || maxSize == 0) { + return 0; + } + + double scaled = (static_cast(value) / static_cast(maxValue)) * + static_cast(maxSize); + return std::max(1, static_cast(scaled)); +} +} // namespace + namespace UltraImageViewer { namespace Core { @@ -35,9 +72,12 @@ std::unique_ptr ImageDecoder::Decode( // Check if we should use memory mapping for large files if (HasFlag(flags, DecoderFlags::MemoryMapped)) { - uintmax_t fileSize = std::filesystem::file_size(filePath); - if (fileSize > 50 * 1024 * 1024) { // 50MB threshold - return DecodeMemoryMapped(filePath, flags); + std::error_code ec; + uintmax_t fileSize = std::filesystem::file_size(filePath, ec); + if (!ec && fileSize > 50 * 1024 * 1024) { // 50MB threshold + if (auto mapped = DecodeMemoryMapped(filePath, flags)) { + return mapped; + } } } @@ -58,7 +98,12 @@ void ImageDecoder::DecodeAsync( std::optional ImageDecoder::GetImageInfo(const std::filesystem::path& filePath) { - if (!std::filesystem::exists(filePath)) { + if (!wicFactory_) { + return std::nullopt; + } + + std::error_code ec; + if (!std::filesystem::exists(filePath, ec) || ec) { return std::nullopt; } @@ -71,28 +116,44 @@ std::optional ImageDecoder::GetImageInfo(const std::filesystem::path& &decoder ); - if (FAILED(hr)) { + if (FAILED(hr) || !decoder) { return std::nullopt; } Microsoft::WRL::ComPtr frame; hr = decoder->GetFrame(0, &frame); - if (FAILED(hr)) { + if (FAILED(hr) || !frame) { return std::nullopt; } ImageInfo info = {}; - frame->GetSize(&info.width, &info.height); - frame->GetPixelFormat(&info.pixelFormat); + hr = frame->GetSize(&info.width, &info.height); + if (FAILED(hr)) { + return std::nullopt; + } + + hr = frame->GetPixelFormat(&info.pixelFormat); + if (FAILED(hr)) { + return std::nullopt; + } // Get bits per pixel Microsoft::WRL::ComPtr componentInfo; - wicFactory_->CreateComponentInfo(info.pixelFormat, &componentInfo); + hr = wicFactory_->CreateComponentInfo(info.pixelFormat, &componentInfo); + if (FAILED(hr) || !componentInfo) { + return info; + } Microsoft::WRL::ComPtr formatInfo; - componentInfo->QueryInterface(IID_PPV_ARGS(&formatInfo)); + hr = componentInfo->QueryInterface(IID_PPV_ARGS(&formatInfo)); + if (FAILED(hr) || !formatInfo) { + return info; + } - formatInfo->GetBitsPerPixel(&info.bitsPerPixel); + hr = formatInfo->GetBitsPerPixel(&info.bitsPerPixel); + if (FAILED(hr)) { + info.bitsPerPixel = 0; + } return info; } @@ -101,6 +162,10 @@ std::unique_ptr ImageDecoder::GenerateThumbnail( const std::filesystem::path& filePath, uint32_t maxSize) { + if (!wicFactory_ || maxSize == 0) { + return nullptr; + } + Microsoft::WRL::ComPtr decoder; HRESULT hr = wicFactory_->CreateDecoderFromFilename( filePath.c_str(), @@ -110,46 +175,67 @@ std::unique_ptr ImageDecoder::GenerateThumbnail( &decoder ); - if (FAILED(hr)) { + if (FAILED(hr) || !decoder) { return nullptr; } Microsoft::WRL::ComPtr frame; hr = decoder->GetFrame(0, &frame); - if (FAILED(hr)) { + if (FAILED(hr) || !frame) { return nullptr; } // Get original dimensions - uint32_t width, height; - frame->GetSize(&width, &height); + uint32_t width = 0; + uint32_t height = 0; + hr = frame->GetSize(&width, &height); + if (FAILED(hr) || width == 0 || height == 0) { + return nullptr; + } // Calculate thumbnail size maintaining aspect ratio - uint32_t thumbWidth, thumbHeight; + uint32_t thumbWidth = 0; + uint32_t thumbHeight = 0; if (width > height) { thumbWidth = maxSize; - thumbHeight = static_cast((static_cast(height) / width) * maxSize); + thumbHeight = ScaleToMax(height, width, maxSize); } else { thumbHeight = maxSize; - thumbWidth = static_cast((static_cast(width) / height) * maxSize); + thumbWidth = ScaleToMax(width, height, maxSize); + } + + size_t dataSize = 0; + UINT stride = 0; + UINT bufferSize = 0; + if (!CalculateBgraLayout(thumbWidth, thumbHeight, dataSize, stride, bufferSize)) { + return nullptr; } // Create thumbnail Microsoft::WRL::ComPtr scaler; - wicFactory_->CreateBitmapScaler(&scaler); + hr = wicFactory_->CreateBitmapScaler(&scaler); + if (FAILED(hr) || !scaler) { + return nullptr; + } - scaler->Initialize( + hr = scaler->Initialize( frame.Get(), thumbWidth, thumbHeight, WICBitmapInterpolationModeFant ); + if (FAILED(hr)) { + return nullptr; + } // Convert to 32-bit BGRA Microsoft::WRL::ComPtr converter; - wicFactory_->CreateFormatConverter(&converter); + hr = wicFactory_->CreateFormatConverter(&converter); + if (FAILED(hr) || !converter) { + return nullptr; + } - converter->Initialize( + hr = converter->Initialize( scaler.Get(), GUID_WICPixelFormat32bppPBGRA, WICBitmapDitherTypeNone, @@ -157,22 +243,30 @@ std::unique_ptr ImageDecoder::GenerateThumbnail( 0.0, WICBitmapPaletteTypeCustom ); + if (FAILED(hr)) { + return nullptr; + } // Allocate buffer - auto image = std::make_unique(); - image->sourcePath = filePath; - image->info.width = thumbWidth; - image->info.height = thumbHeight; - image->info.pixelFormat = GUID_WICPixelFormat32bppPBGRA; - image->info.bitsPerPixel = 32; - image->info.dataSize = thumbWidth * thumbHeight * 4; - image->data = std::make_unique(image->info.dataSize); + std::unique_ptr image; + try { + image = std::make_unique(); + image->sourcePath = filePath; + image->info.width = thumbWidth; + image->info.height = thumbHeight; + image->info.pixelFormat = GUID_WICPixelFormat32bppPBGRA; + image->info.bitsPerPixel = 32; + image->info.dataSize = dataSize; + image->data = std::make_unique(image->info.dataSize); + } catch (const std::bad_alloc&) { + return nullptr; + } // Copy pixels hr = converter->CopyPixels( nullptr, - thumbWidth * 4, - image->info.dataSize, + stride, + bufferSize, image->data.get() ); @@ -208,6 +302,10 @@ std::unique_ptr ImageDecoder::DecodeWithWIC( const std::filesystem::path& filePath, DecoderFlags flags) { + if (!wicFactory_) { + return nullptr; + } + Microsoft::WRL::ComPtr decoder; Microsoft::WRL::ComPtr frame; @@ -219,28 +317,50 @@ std::unique_ptr ImageDecoder::DecodeWithWIC( &decoder ); - if (FAILED(hr)) { + if (FAILED(hr) || !decoder) { return nullptr; } hr = decoder->GetFrame(0, &frame); - if (FAILED(hr)) { + if (FAILED(hr) || !frame) { return nullptr; } // Get image info - auto image = std::make_unique(); - image->sourcePath = filePath; + std::unique_ptr image; + try { + image = std::make_unique(); + image->sourcePath = filePath; + } catch (const std::bad_alloc&) { + return nullptr; + } + + hr = frame->GetSize(&image->info.width, &image->info.height); + if (FAILED(hr) || image->info.width == 0 || image->info.height == 0) { + return nullptr; + } + + hr = frame->GetPixelFormat(&image->info.pixelFormat); + if (FAILED(hr)) { + return nullptr; + } - frame->GetSize(&image->info.width, &image->info.height); - frame->GetPixelFormat(&image->info.pixelFormat); + size_t dataSize = 0; + UINT stride = 0; + UINT bufferSize = 0; + if (!CalculateBgraLayout(image->info.width, image->info.height, dataSize, stride, bufferSize)) { + return nullptr; + } // Check if format conversion is needed WICPixelFormatGUID targetFormat = GUID_WICPixelFormat32bppPBGRA; // Convert to 32-bit BGRA for GPU compatibility Microsoft::WRL::ComPtr converter; - wicFactory_->CreateFormatConverter(&converter); + hr = wicFactory_->CreateFormatConverter(&converter); + if (FAILED(hr) || !converter) { + return nullptr; + } hr = converter->Initialize( frame.Get(), @@ -256,14 +376,18 @@ std::unique_ptr ImageDecoder::DecodeWithWIC( } // Allocate buffer - image->info.dataSize = image->info.width * image->info.height * 4; - image->data = std::make_unique(image->info.dataSize); + image->info.dataSize = dataSize; + try { + image->data = std::make_unique(image->info.dataSize); + } catch (const std::bad_alloc&) { + return nullptr; + } // Copy pixels hr = converter->CopyPixels( nullptr, - image->info.width * 4, - image->info.dataSize, + stride, + bufferSize, image->data.get() ); diff --git a/src/core/ImagePipeline.cpp b/src/core/ImagePipeline.cpp index 282ea44..e1345b1 100644 --- a/src/core/ImagePipeline.cpp +++ b/src/core/ImagePipeline.cpp @@ -1165,7 +1165,8 @@ void ImagePipeline::SavePersistentThumbs(const std::filesystem::path& cachePath) // Write to .tmp file auto tmpPath = cachePath.wstring() + L".tmp"; - FILE* f = _wfopen(tmpPath.c_str(), L"wb"); + FILE* f = nullptr; + _wfopen_s(&f, tmpPath.c_str(), L"wb"); if (!f) return; // Header diff --git a/src/rendering/Direct2DRenderer.cpp b/src/rendering/Direct2DRenderer.cpp index 2301231..d10420b 100644 --- a/src/rendering/Direct2DRenderer.cpp +++ b/src/rendering/Direct2DRenderer.cpp @@ -14,7 +14,7 @@ void D2DLog(const char* msg) { OutputDebugStringA(msg); OutputDebugStringA("\n"); static FILE* f = nullptr; - if (!f) f = fopen("debug_log.txt", "a"); + if (!f) fopen_s(&f, "debug_log.txt", "a"); if (f) { fprintf(f, " [D2D] %s\n", msg); fflush(f); } } void D2DLogHR(const char* msg, HRESULT hr) { diff --git a/src/ui/GestureHandler.cpp b/src/ui/GestureHandler.cpp index 0b19150..6b78add 100644 --- a/src/ui/GestureHandler.cpp +++ b/src/ui/GestureHandler.cpp @@ -44,13 +44,19 @@ bool GestureHandler::Initialize(HWND hwnd) // Configure Windows gesture support DWORD flags = GetGestureConfigFlags(); + auto wantFlag = [flags](DWORD flag) -> DWORD { + return (flags & flag) ? flag : 0u; + }; + auto blockFlag = [flags](DWORD flag) -> DWORD { + return (flags & flag) ? 0u : flag; + }; GESTURECONFIG config[] = { - { GID_ZOOM, flags & GC_ZOOM ? 0 : GC_ZOOM, flags & GC_ZOOM ? GC_ZOOM : 0 }, - { GID_PAN, flags & GC_PAN ? 0 : GC_PAN, flags & GC_PAN ? GC_PAN : 0 }, - { GID_ROTATE, flags & GC_ROTATE ? 0 : GC_ROTATE, flags & GC_ROTATE ? GC_ROTATE : 0 }, - { GID_TWOFINGERTAP, flags & GC_TWOFINGERTAP ? 0 : GC_TWOFINGERTAP, flags & GC_TWOFINGERTAP ? GC_TWOFINGERTAP : 0 }, - { GID_PRESSANDTAP, flags & GC_PRESSANDTAP ? 0 : GC_PRESSANDTAP, flags & GC_PRESSANDTAP ? GC_PRESSANDTAP : 0 } + { GID_ZOOM, wantFlag(GC_ZOOM), blockFlag(GC_ZOOM) }, + { GID_PAN, wantFlag(GC_PAN), blockFlag(GC_PAN) }, + { GID_ROTATE, wantFlag(GC_ROTATE), blockFlag(GC_ROTATE) }, + { GID_TWOFINGERTAP, wantFlag(GC_TWOFINGERTAP), blockFlag(GC_TWOFINGERTAP) }, + { GID_PRESSANDTAP, wantFlag(GC_PRESSANDTAP), blockFlag(GC_PRESSANDTAP) } }; BOOL result = SetGestureConfig( diff --git a/src/ui/ThumbnailStrip.cpp b/src/ui/ThumbnailStrip.cpp index ec975a4..f88edb6 100644 --- a/src/ui/ThumbnailStrip.cpp +++ b/src/ui/ThumbnailStrip.cpp @@ -405,9 +405,6 @@ void ThumbnailStrip::RenderLoadingIndicator(ID2D1DeviceContext* context, size_t float centerY = rect.top + rect.bottom / 2.0f; float radius = std::min(rect.right - rect.left, rect.bottom - rect.top) / 4.0f; - ULONGLONG time = GetTickCount64(); - float angle = static_cast(time % 1000) / 1000.0f * 360.0f; - // Simple loading circle D2D1_ELLIPSE ellipse = D2D1::Ellipse(D2D1::Point2F(centerX, centerY), radius, radius); From 9912c6f4ec00401dc937cb7ab34721a84e8f148b Mon Sep 17 00:00:00 2001 From: 5428384822a-create <274500894+5428384822a-create@users.noreply.github.com> Date: Wed, 29 Apr 2026 03:29:45 +0800 Subject: [PATCH 2/3] fix: avoid async decoder lifetime hazard --- src/core/ImageDecoder.cpp | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/core/ImageDecoder.cpp b/src/core/ImageDecoder.cpp index ff05ab1..6f53028 100644 --- a/src/core/ImageDecoder.cpp +++ b/src/core/ImageDecoder.cpp @@ -89,9 +89,29 @@ void ImageDecoder::DecodeAsync( std::function)> callback, DecoderFlags flags) { - // Launch async decoding in background thread - std::thread([this, filePath, callback, flags]() { - auto image = this->Decode(filePath, flags); + if (!callback) { + return; + } + + // Keep async decode independent from this object's lifetime. + std::thread([filePath, callback = std::move(callback), flags]() mutable { + std::unique_ptr image; + HRESULT hr = CoInitializeEx(nullptr, COINIT_MULTITHREADED); + bool shouldUninitializeCom = SUCCEEDED(hr); + + if (SUCCEEDED(hr)) { + try { + ImageDecoder decoder; + image = decoder.Decode(filePath, flags); + } catch (...) { + image.reset(); + } + } + + if (shouldUninitializeCom) { + CoUninitialize(); + } + callback(std::move(image)); }).detach(); } From e95aa8f290ad5453ae3f21a45a2948cda41272fb Mon Sep 17 00:00:00 2001 From: 5428384822a-create <274500894+5428384822a-create@users.noreply.github.com> Date: Wed, 29 Apr 2026 03:32:46 +0800 Subject: [PATCH 3/3] fix: manage async decoder task lifetime --- include/core/ImageDecoder.hpp | 14 +++++ src/core/ImageDecoder.cpp | 99 +++++++++++++++++++++++++++++------ 2 files changed, 98 insertions(+), 15 deletions(-) diff --git a/include/core/ImageDecoder.hpp b/include/core/ImageDecoder.hpp index bc0fd9a..3c49b85 100644 --- a/include/core/ImageDecoder.hpp +++ b/include/core/ImageDecoder.hpp @@ -6,6 +6,9 @@ #include #include #include +#include +#include +#include #include #include @@ -57,6 +60,9 @@ class ImageDecoder { ImageDecoder(); ~ImageDecoder(); + ImageDecoder(const ImageDecoder&) = delete; + ImageDecoder& operator=(const ImageDecoder&) = delete; + // Main decoding interface std::unique_ptr Decode( const std::filesystem::path& filePath, @@ -84,6 +90,13 @@ class ImageDecoder { static std::vector GetSupportedExtensions(); private: + struct AsyncDecodeState { + std::atomic shuttingDown{false}; + std::mutex mutex; + std::condition_variable idle; + size_t activeTasks = 0; + }; + // WIC decoder implementation std::unique_ptr DecodeWithWIC( const std::filesystem::path& filePath, @@ -103,6 +116,7 @@ class ImageDecoder { ); Microsoft::WRL::ComPtr wicFactory_; + std::shared_ptr asyncState_; }; } // namespace Core diff --git a/src/core/ImageDecoder.cpp b/src/core/ImageDecoder.cpp index 6f53028..be16d9f 100644 --- a/src/core/ImageDecoder.cpp +++ b/src/core/ImageDecoder.cpp @@ -8,6 +8,8 @@ #include namespace { +thread_local const void* g_currentAsyncDecoderState = nullptr; + bool CalculateBgraLayout(uint32_t width, uint32_t height, size_t& dataSize, UINT& stride, UINT& bufferSize) { if (width == 0 || height == 0) { @@ -46,6 +48,7 @@ namespace UltraImageViewer { namespace Core { ImageDecoder::ImageDecoder() + : asyncState_(std::make_shared()) { // Initialize WIC factory HRESULT hr = CoCreateInstance( @@ -60,7 +63,24 @@ ImageDecoder::ImageDecoder() } } -ImageDecoder::~ImageDecoder() = default; +ImageDecoder::~ImageDecoder() +{ + auto state = asyncState_; + if (!state) { + return; + } + + std::unique_lock lock(state->mutex); + state->shuttingDown.store(true, std::memory_order_release); + + if (g_currentAsyncDecoderState == state.get()) { + return; + } + + state->idle.wait(lock, [state]() { + return state->activeTasks == 0; + }); +} std::unique_ptr ImageDecoder::Decode( const std::filesystem::path& filePath, @@ -93,27 +113,76 @@ void ImageDecoder::DecodeAsync( return; } - // Keep async decode independent from this object's lifetime. - std::thread([filePath, callback = std::move(callback), flags]() mutable { - std::unique_ptr image; - HRESULT hr = CoInitializeEx(nullptr, COINIT_MULTITHREADED); - bool shouldUninitializeCom = SUCCEEDED(hr); + auto state = asyncState_; + if (!state) { + return; + } + + { + std::lock_guard lock(state->mutex); + if (state->shuttingDown.load(std::memory_order_acquire)) { + return; + } + ++state->activeTasks; + } + + try { + // Keep async decode independent from this object's lifetime. + std::thread([state, filePath, callback = std::move(callback), flags]() mutable { + g_currentAsyncDecoderState = state.get(); + std::unique_ptr image; - if (SUCCEEDED(hr)) { try { - ImageDecoder decoder; - image = decoder.Decode(filePath, flags); + HRESULT hr = CoInitializeEx(nullptr, COINIT_MULTITHREADED); + bool shouldUninitializeCom = SUCCEEDED(hr); + + if (SUCCEEDED(hr) && !state->shuttingDown.load(std::memory_order_acquire)) { + try { + ImageDecoder decoder; + image = decoder.Decode(filePath, flags); + } catch (...) { + image.reset(); + } + } + + if (shouldUninitializeCom) { + CoUninitialize(); + } + + if (!state->shuttingDown.load(std::memory_order_acquire)) { + try { + callback(std::move(image)); + } catch (...) { + } + } } catch (...) { - image.reset(); } - } - if (shouldUninitializeCom) { - CoUninitialize(); + { + std::lock_guard lock(state->mutex); + if (state->activeTasks > 0) { + --state->activeTasks; + } + } + state->idle.notify_all(); + g_currentAsyncDecoderState = nullptr; + }).detach(); + } catch (...) { + { + std::lock_guard lock(state->mutex); + if (state->activeTasks > 0) { + --state->activeTasks; + } } + state->idle.notify_all(); - callback(std::move(image)); - }).detach(); + if (!state->shuttingDown.load(std::memory_order_acquire)) { + try { + callback(nullptr); + } catch (...) { + } + } + } } std::optional ImageDecoder::GetImageInfo(const std::filesystem::path& filePath)