From b3c241311548701c2cdfa8ff7e1fde9cd768c212 Mon Sep 17 00:00:00 2001 From: Juuso Piippo Date: Mon, 8 Jun 2026 23:44:13 +0300 Subject: [PATCH 1/5] Introduce arena, use permanent memory storage Introduced arenas to control the permanent memory storage as well as scratch memory. We now just preallocate a large enough buffer for all paths in AppState and UIJob. Arenas are meant to handle that memory more cleanly as well as help debugging memory usage. Tests still use stack space with regards to paths but that will probably change --- src/arena.h | 69 ++++++ src/compressor.h | 5 + src/compressor_tests.cpp | 476 ++++++++++++++++++++++----------------- src/win32_compressor.cpp | 376 ++++++++++++++++++++++--------- src/win32_compressor.h | 55 +++-- 5 files changed, 644 insertions(+), 337 deletions(-) create mode 100644 src/arena.h diff --git a/src/arena.h b/src/arena.h new file mode 100644 index 0000000..fba5632 --- /dev/null +++ b/src/arena.h @@ -0,0 +1,69 @@ +#pragma once + +struct Arena { + u8* base; + u64 used; + u64 size; + + // Debug + u64 totalUsed; // Accumulated used +}; + +static void +InitArena(Arena* arena, void* base, u64 size) { + arena->base = static_cast(base); + arena->used = 0; + arena->size = size; + + arena->totalUsed = 0; +} + +static void* +ArenaPush(Arena* arena, u64 size) { + ASSERT(arena->used + size <= arena->size); + //if (arena->used + size > arena->size) { + //DEBUG_PRINT("ArenaPush: Out of memory!\n"); + //PrintArena(arena); nocheckin + //ExitProcess(1); + //} + + void* result = arena->base + arena->used; + arena->used += size; + arena->totalUsed += size; + return result; +} + +static void* +ArenaPushZero(Arena* arena, u64 size) { + void* result = ArenaPush(arena, size); + ZeroMemory(result, size); + return result; +} + +static void +ArenaPop(Arena* arena, u64 size) { + ASSERT(arena->used >= size); + arena->used -= size; +} + +static void +ArenaClear(Arena* arena) { + arena->used = 0; +} + +static u64 +ArenaGetPos(Arena* arena) { + return arena->used; +} + +static void +ArenaSetPos(Arena* arena, u64 pos) { + // It would be a programmer error to set equal to size + ASSERT(pos >= 0 && pos < arena->size); + arena->used = pos; +} + +#define PushArray(arena, type, count) (type*)ArenaPush((arena), sizeof(type) * (count)) +#define PushArrayZero(arena, type, count) (type*)ArenaPushZero((arena), sizeof(type) * (count)) +#define PushStruct(arena, type) PushArray((arena), type, 1) +#define PushStructZero(arena, type) PushArrayZero((arena), type, 1) diff --git a/src/compressor.h b/src/compressor.h index 974bc14..9cbe3c7 100644 --- a/src/compressor.h +++ b/src/compressor.h @@ -39,6 +39,11 @@ typedef wchar_t wchar; // clang-format on +#define KB(x) (x * 1024ULL) +#define MB(x) (KB(x) * 1024ULL) +#define GB(x) (MB(x) * 1024ULL) +#define TB(x) (GB(x) * 1024ULL) + /** * Returns number of bytes, NOT the number of "real" characters */ diff --git a/src/compressor_tests.cpp b/src/compressor_tests.cpp index 5841dd6..58d1aff 100644 --- a/src/compressor_tests.cpp +++ b/src/compressor_tests.cpp @@ -20,15 +20,67 @@ CANDIDATES TO TEST: - GetExeDirectory */ -struct AddJobAppStateFixture { - AppState appState = {}; +// Now that we use a fixed permanent storage, we have to clear manually +static void +ResetAppState(AppState* appState) { + appState->jobCount = 0; + for (i32 i = 0; i < ARR_COUNT(appState->jobs); ++i) { + appState->jobs[i].input[0] = '\0'; + appState->jobs[i].output[0] = '\0'; + } + + appState->defaultCodec = Codec::NONE; + appState->defaultTargetSize = 0.0f; + ZeroMemory(appState->targetSizes, sizeof(appState->targetSizes)); + appState->outputFolder[0] = '\0'; + + // The functions use the scratch arena so we do this manually here + ArenaClear(&appState->scratchArena); +} + +struct AppStateFixture { + AppState* appState = nullptr; + + AppStateFixture() { + static AppState staticAppState = {}; + static bool32 initialized = false; + if (!initialized) { + InitAppState(&staticAppState); + initialized = true; + } + + appState = &staticAppState; + } + + ~AppStateFixture() { ResetAppState(appState); } + + bool32 + IsZeroed() { + bool32 targetSizesZeroed = true; + for (i32 i = 0; i < ARR_COUNT(appState->targetSizes); ++i) { + if (appState->targetSizes[i] != 0.0f) { + targetSizesZeroed = false; + break; + } + } + + bool32 defaultTargetSizeZeroed = appState->defaultTargetSize == 0.0f; + bool32 codecZeroed = appState->defaultCodec == Codec::NONE; + bool32 outputFolderZeroed = appState->outputFolder[0] == '\0'; + return targetSizesZeroed && defaultTargetSizeZeroed && codecZeroed && outputFolderZeroed; + } +}; + +struct AddJobAppStateFixture : AppStateFixture { AddJobAppStateFixture() { // Other stuff? - snprintf(appState.outputFolder, ARR_COUNT(appState.outputFolder), "C:\\output"); + _snprintf_s(appState->outputFolder, MAX_PATH_COUNT, _TRUNCATE, "C:\\output"); } }; +TEST_SUITE_BEGIN("String stuff"); + TEST_CASE("StrEqual works correctly") { struct TC { const char* a; @@ -72,6 +124,8 @@ TEST_CASE("UTF16 round trip") { CHECK(StrEqual(str, str2)); } +TEST_SUITE_END(); + TEST_CASE_FIXTURE(AddJobAppStateFixture, "ConstructPathsForJob works correctly") { struct TC { const wchar* input; @@ -89,7 +143,11 @@ TEST_CASE_FIXTURE(AddJobAppStateFixture, "ConstructPathsForJob works correctly") TC{ L"K:\\input\\요 세계.hgfh.mp4", "C:\\output\\요 세계.hgfh.mp4" }); UIJob j = {}; - ConstructPathsForJob(&appState, &j, tc.input); + j.input = appState->jobs[0].input; + j.output = appState->jobs[0].output; + j.input[0] = '\0'; + j.output[0] = '\0'; + ConstructPathsForJob(appState, &j, tc.input); char inputUtf8[MAX_PATH_COUNT]; UTF16To8(tc.input, inputUtf8); @@ -113,20 +171,20 @@ TEST_CASE_FIXTURE(AddJobAppStateFixture, "IsPathFromOutputFolder works correctly char inputUtf8[MAX_PATH_COUNT]; UTF16To8(tc.input, inputUtf8); INFO("input: ", inputUtf8); - INFO("appstate output: ", appState.outputFolder); + INFO("appstate output: ", appState->outputFolder); INFO("expected: ", tc.exp); - CHECK_EQ(IsPathFromOutputFolder(&appState, tc.input), tc.exp); + CHECK_EQ(IsPathFromOutputFolder(appState, tc.input), tc.exp); } // Temp files for AddJob -struct TempFileFixture { +struct TempFilesFixture { // TODO: test paths greater than MAX_PATH: 260 and current MAX_PATH_COUNT wchar dir[MAX_PATH_COUNT]; wchar path[MAX_PATH_COUNT]; wchar path2[MAX_PATH_COUNT]; wchar path3[MAX_PATH_COUNT]; - TempFileFixture() { + TempFilesFixture() { i32 val = GetTempPathW(ARR_COUNT(dir), dir); REQUIRE(val != 0); dir[val - 1] = '\0'; // Remove trailing slash... @@ -141,6 +199,12 @@ struct TempFileFixture { CreateTestFile(path3); } + ~TempFilesFixture() { + DeleteFileW(path); + DeleteFileW(path2); + DeleteFileW(path3); + } + void CreateTestFile(const wchar* pathW) { HANDLE file = CreateFileW(pathW, GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, @@ -155,56 +219,52 @@ struct TempFileFixture { CloseHandle(file); } - - ~TempFileFixture() { - DeleteFileW(path); - DeleteFileW(path2); - DeleteFileW(path3); - } }; -struct AddJobFixture : AddJobAppStateFixture, TempFileFixture {}; +TEST_SUITE_BEGIN("AddJob"); + +struct AddJobFixture : AddJobAppStateFixture, TempFilesFixture {}; TEST_CASE_FIXTURE(AddJobFixture, "AddJob reads file size correctly") { CAPTURE(appState); - CHECK(AddJob(&appState, path) == AddJobResult::SUCCESS); - CHECK(appState.jobCount == 1); - CHECK(appState.jobs[0].inputFileSize == doctest::Approx(4096 / (1024.0f * 1024.0f))); + CHECK(AddJob(appState, path) == AddJobResult::SUCCESS); + CHECK(appState->jobCount == 1); + CHECK(appState->jobs[0].inputFileSize == doctest::Approx(4096 / (1024.0f * 1024.0f))); } TEST_CASE_FIXTURE(AddJobFixture, "AddJob fails at MAX_JOBS") { - appState.jobCount = MAX_JOBS; - CHECK(AddJob(&appState, path) == AddJobResult::JOBS_FULL); - CHECK(appState.jobCount == MAX_JOBS); + appState->jobCount = MAX_JOBS; + CHECK(AddJob(appState, path) == AddJobResult::JOBS_FULL); + CHECK(appState->jobCount == MAX_JOBS); } TEST_CASE_FIXTURE(AddJobFixture, "AddJob succeeds at MAX_JOBS - 1") { - appState.jobCount = MAX_JOBS - 1; - CHECK(AddJob(&appState, path) == AddJobResult::SUCCESS); - CHECK(appState.jobCount == MAX_JOBS); + appState->jobCount = MAX_JOBS - 1; + CHECK(AddJob(appState, path) == AddJobResult::SUCCESS); + CHECK(appState->jobCount == MAX_JOBS); } TEST_CASE_FIXTURE(AddJobFixture, "AddJob increments job list correctly") { - CHECK(AddJob(&appState, path) == AddJobResult::SUCCESS); - CHECK(AddJob(&appState, path2) == AddJobResult::SUCCESS); - CHECK(AddJob(&appState, path3) == AddJobResult::SUCCESS); - CHECK(appState.jobCount == 3); + CHECK(AddJob(appState, path) == AddJobResult::SUCCESS); + CHECK(AddJob(appState, path2) == AddJobResult::SUCCESS); + CHECK(AddJob(appState, path3) == AddJobResult::SUCCESS); + CHECK(appState->jobCount == 3); for (i32 i = 0; i < 3; ++i) { - CHECK(appState.jobs[i].status == JobStatus::QUEUED); + CHECK(appState->jobs[i].status == JobStatus::QUEUED); } } TEST_CASE_FIXTURE(AddJobFixture, "AddJob rejects duplicate input") { - CHECK(AddJob(&appState, path) == AddJobResult::SUCCESS); + CHECK(AddJob(appState, path) == AddJobResult::SUCCESS); - CHECK(AddJob(&appState, path) == AddJobResult::DUPLICATE_JOB); - CHECK(AddJob(&appState, path) == AddJobResult::DUPLICATE_JOB); - CHECK(appState.jobCount == 1); + CHECK(AddJob(appState, path) == AddJobResult::DUPLICATE_JOB); + CHECK(AddJob(appState, path) == AddJobResult::DUPLICATE_JOB); + CHECK(appState->jobCount == 1); - CHECK(AddJob(&appState, path2) == AddJobResult::SUCCESS); - CHECK(AddJob(&appState, path2) == AddJobResult::DUPLICATE_JOB); - CHECK(appState.jobCount == 2); + CHECK(AddJob(appState, path2) == AddJobResult::SUCCESS); + CHECK(AddJob(appState, path2) == AddJobResult::DUPLICATE_JOB); + CHECK(appState->jobCount == 2); } TEST_CASE_FIXTURE(AddJobFixture, "AddJob rejects input from output folder") { @@ -214,18 +274,20 @@ TEST_CASE_FIXTURE(AddJobFixture, "AddJob rejects input from output folder") { UTF16To8(dir, dirUtf8); INFO(pathUtf8); - snprintf(appState.outputFolder, ARR_COUNT(appState.outputFolder), "%s", dirUtf8); - INFO(appState.outputFolder); + snprintf(appState->outputFolder, MAX_PATH_COUNT, "%s", dirUtf8); + INFO(appState->outputFolder); - CHECK(AddJob(&appState, path) == AddJobResult::JOB_FROM_OUTPUT); - CHECK(appState.jobCount == 0); + CHECK(AddJob(appState, path) == AddJobResult::JOB_FROM_OUTPUT); + CHECK(appState->jobCount == 0); } +TEST_SUITE_END(); + TEST_CASE_FIXTURE(AddJobFixture, "MoveJob works correctly") { - REQUIRE(AddJob(&appState, path) == AddJobResult::SUCCESS); - REQUIRE(AddJob(&appState, path2) == AddJobResult::SUCCESS); - REQUIRE(AddJob(&appState, path3) == AddJobResult::SUCCESS); - REQUIRE(appState.jobCount == 3); + REQUIRE(AddJob(appState, path) == AddJobResult::SUCCESS); + REQUIRE(AddJob(appState, path2) == AddJobResult::SUCCESS); + REQUIRE(AddJob(appState, path3) == AddJobResult::SUCCESS); + REQUIRE(appState->jobCount == 3); char pathUtf8[MAX_PATH_COUNT]; char path2Utf8[MAX_PATH_COUNT]; @@ -235,41 +297,41 @@ TEST_CASE_FIXTURE(AddJobFixture, "MoveJob works correctly") { UTF16To8(path3, path3Utf8); // Normal operations - CHECK(MoveJob(&appState, 0, 2)); - CHECK(MoveJob(&appState, 2, 0)); - CHECK(MoveJob(&appState, 1, 2)); - CHECK(StrEqual(appState.jobs[1].input, path3Utf8)); - CHECK(StrEqual(appState.jobs[2].input, path2Utf8)); + CHECK(MoveJob(appState, 0, 2)); + CHECK(MoveJob(appState, 2, 0)); + CHECK(MoveJob(appState, 1, 2)); + CHECK(StrEqual(appState->jobs[1].input, path3Utf8)); + CHECK(StrEqual(appState->jobs[2].input, path2Utf8)); - CHECK(MoveJob(&appState, 2, 0)); - CHECK(StrEqual(appState.jobs[0].input, path2Utf8)); - CHECK(StrEqual(appState.jobs[1].input, pathUtf8)); - CHECK(StrEqual(appState.jobs[2].input, path3Utf8)); + CHECK(MoveJob(appState, 2, 0)); + CHECK(StrEqual(appState->jobs[0].input, path2Utf8)); + CHECK(StrEqual(appState->jobs[1].input, pathUtf8)); + CHECK(StrEqual(appState->jobs[2].input, path3Utf8)); // Invalid - CHECK(!MoveJob(&appState, 3, 2)); - CHECK(!MoveJob(&appState, -2, 2)); - CHECK(StrEqual(appState.jobs[0].input, path2Utf8)); - CHECK(StrEqual(appState.jobs[1].input, pathUtf8)); - CHECK(StrEqual(appState.jobs[2].input, path3Utf8)); + CHECK(!MoveJob(appState, 3, 2)); + CHECK(!MoveJob(appState, -2, 2)); + CHECK(StrEqual(appState->jobs[0].input, path2Utf8)); + CHECK(StrEqual(appState->jobs[1].input, pathUtf8)); + CHECK(StrEqual(appState->jobs[2].input, path3Utf8)); // Highest running index - CHECK(!MoveJob(&appState, 0, 2, 1)); - CHECK(StrEqual(appState.jobs[1].input, pathUtf8)); - CHECK(StrEqual(appState.jobs[2].input, path3Utf8)); - CHECK(MoveJob(&appState, 2, 1, 0)); - CHECK(StrEqual(appState.jobs[1].input, path3Utf8)); - CHECK(StrEqual(appState.jobs[2].input, pathUtf8)); - CHECK(!MoveJob(&appState, 1, 2, 2)); - CHECK(StrEqual(appState.jobs[1].input, path3Utf8)); - CHECK(StrEqual(appState.jobs[2].input, pathUtf8)); + CHECK(!MoveJob(appState, 0, 2, 1)); + CHECK(StrEqual(appState->jobs[1].input, pathUtf8)); + CHECK(StrEqual(appState->jobs[2].input, path3Utf8)); + CHECK(MoveJob(appState, 2, 1, 0)); + CHECK(StrEqual(appState->jobs[1].input, path3Utf8)); + CHECK(StrEqual(appState->jobs[2].input, pathUtf8)); + CHECK(!MoveJob(appState, 1, 2, 2)); + CHECK(StrEqual(appState->jobs[1].input, path3Utf8)); + CHECK(StrEqual(appState->jobs[2].input, pathUtf8)); } TEST_CASE_FIXTURE(AddJobFixture, "RemoveJob works correctly") { - REQUIRE(AddJob(&appState, path) == AddJobResult::SUCCESS); - REQUIRE(AddJob(&appState, path2) == AddJobResult::SUCCESS); - REQUIRE(AddJob(&appState, path3) == AddJobResult::SUCCESS); - REQUIRE(appState.jobCount == 3); + REQUIRE(AddJob(appState, path) == AddJobResult::SUCCESS); + REQUIRE(AddJob(appState, path2) == AddJobResult::SUCCESS); + REQUIRE(AddJob(appState, path3) == AddJobResult::SUCCESS); + REQUIRE(appState->jobCount == 3); char pathUtf8[MAX_PATH_COUNT]; char path2Utf8[MAX_PATH_COUNT]; @@ -278,44 +340,44 @@ TEST_CASE_FIXTURE(AddJobFixture, "RemoveJob works correctly") { UTF16To8(path2, path2Utf8); UTF16To8(path3, path3Utf8); - CHECK(RemoveJob(&appState, 1)); - CHECK(appState.jobCount == 2); - CHECK(StrEqual(appState.jobs[0].input, pathUtf8)); - CHECK(StrEqual(appState.jobs[1].input, path3Utf8)); + CHECK(RemoveJob(appState, 1)); + CHECK(appState->jobCount == 2); + CHECK(StrEqual(appState->jobs[0].input, pathUtf8)); + CHECK(StrEqual(appState->jobs[1].input, path3Utf8)); // Remove from end - CHECK(RemoveJob(&appState, 1)); - CHECK(appState.jobCount == 1); - CHECK(StrEqual(appState.jobs[0].input, pathUtf8)); + CHECK(RemoveJob(appState, 1)); + CHECK(appState->jobCount == 1); + CHECK(StrEqual(appState->jobs[0].input, pathUtf8)); // Tests adding after removal - REQUIRE(AddJob(&appState, path3) == AddJobResult::SUCCESS); - REQUIRE(AddJob(&appState, path2) == AddJobResult::SUCCESS); + REQUIRE(AddJob(appState, path3) == AddJobResult::SUCCESS); + REQUIRE(AddJob(appState, path2) == AddJobResult::SUCCESS); // Invalid - CHECK(!RemoveJob(&appState, -1)); - CHECK(appState.jobCount == 3); - CHECK(!RemoveJob(&appState, 4)); - CHECK(appState.jobCount == 3); - CHECK(!RemoveJob(&appState, 3)); - CHECK(appState.jobCount == 3); - CHECK(StrEqual(appState.jobs[0].input, pathUtf8)); - CHECK(StrEqual(appState.jobs[1].input, path3Utf8)); - CHECK(StrEqual(appState.jobs[2].input, path2Utf8)); - - CHECK(RemoveJob(&appState, 0)); - CHECK(appState.jobCount == 2); - CHECK(StrEqual(appState.jobs[0].input, path3Utf8)); - CHECK(StrEqual(appState.jobs[1].input, path2Utf8)); - - CHECK(RemoveJob(&appState, 1)); - CHECK(appState.jobCount == 1); - CHECK(StrEqual(appState.jobs[0].input, path3Utf8)); - CHECK(RemoveJob(&appState, 0)); - CHECK(appState.jobCount == 0); - - CHECK(!RemoveJob(&appState, 0)); - CHECK(appState.jobCount == 0); + CHECK(!RemoveJob(appState, -1)); + CHECK(appState->jobCount == 3); + CHECK(!RemoveJob(appState, 4)); + CHECK(appState->jobCount == 3); + CHECK(!RemoveJob(appState, 3)); + CHECK(appState->jobCount == 3); + CHECK(StrEqual(appState->jobs[0].input, pathUtf8)); + CHECK(StrEqual(appState->jobs[1].input, path3Utf8)); + CHECK(StrEqual(appState->jobs[2].input, path2Utf8)); + + CHECK(RemoveJob(appState, 0)); + CHECK(appState->jobCount == 2); + CHECK(StrEqual(appState->jobs[0].input, path3Utf8)); + CHECK(StrEqual(appState->jobs[1].input, path2Utf8)); + + CHECK(RemoveJob(appState, 1)); + CHECK(appState->jobCount == 1); + CHECK(StrEqual(appState->jobs[0].input, path3Utf8)); + CHECK(RemoveJob(appState, 0)); + CHECK(appState->jobCount == 0); + + CHECK(!RemoveJob(appState, 0)); + CHECK(appState->jobCount == 0); } TEST_CASE_FIXTURE(AddJobFixture, "AddJob, RemoveJob and MoveJob work correctly") { @@ -326,65 +388,67 @@ TEST_CASE_FIXTURE(AddJobFixture, "AddJob, RemoveJob and MoveJob work correctly") UTF16To8(path2, path2Utf8); UTF16To8(path3, path3Utf8); - REQUIRE(AddJob(&appState, path) == AddJobResult::SUCCESS); - REQUIRE(AddJob(&appState, path2) == AddJobResult::SUCCESS); - REQUIRE(AddJob(&appState, path3) == AddJobResult::SUCCESS); - REQUIRE(appState.jobCount == 3); + REQUIRE(AddJob(appState, path) == AddJobResult::SUCCESS); + REQUIRE(AddJob(appState, path2) == AddJobResult::SUCCESS); + REQUIRE(AddJob(appState, path3) == AddJobResult::SUCCESS); + REQUIRE(appState->jobCount == 3); // [path, path2, path3] // Move then remove the result - CHECK(MoveJob(&appState, 0, 2)); + CHECK(MoveJob(appState, 0, 2)); // [path2, path3, path] - CHECK(RemoveJob(&appState, 2)); - CHECK(appState.jobCount == 2); - CHECK(StrEqual(appState.jobs[0].input, path2Utf8)); - CHECK(StrEqual(appState.jobs[1].input, path3Utf8)); + CHECK(RemoveJob(appState, 2)); + CHECK(appState->jobCount == 2); + CHECK(StrEqual(appState->jobs[0].input, path2Utf8)); + CHECK(StrEqual(appState->jobs[1].input, path3Utf8)); // Re-add the removed job, check it lands at the end - REQUIRE(AddJob(&appState, path) == AddJobResult::SUCCESS); - CHECK(appState.jobCount == 3); - CHECK(StrEqual(appState.jobs[2].input, pathUtf8)); + REQUIRE(AddJob(appState, path) == AddJobResult::SUCCESS); + CHECK(appState->jobCount == 3); + CHECK(StrEqual(appState->jobs[2].input, pathUtf8)); // [path2, path3, path] // Remove from front - CHECK(RemoveJob(&appState, 0)); - CHECK(appState.jobCount == 2); - CHECK(StrEqual(appState.jobs[0].input, path3Utf8)); - CHECK(StrEqual(appState.jobs[1].input, pathUtf8)); + CHECK(RemoveJob(appState, 0)); + CHECK(appState->jobCount == 2); + CHECK(StrEqual(appState->jobs[0].input, path3Utf8)); + CHECK(StrEqual(appState->jobs[1].input, pathUtf8)); // [path3, path] // Re-add path2, move it to front - REQUIRE(AddJob(&appState, path2) == AddJobResult::SUCCESS); - CHECK(appState.jobCount == 3); + REQUIRE(AddJob(appState, path2) == AddJobResult::SUCCESS); + CHECK(appState->jobCount == 3); // [path3, path, path2] - CHECK(MoveJob(&appState, 2, 0)); + CHECK(MoveJob(appState, 2, 0)); // [path2, path3, path] - CHECK(StrEqual(appState.jobs[0].input, path2Utf8)); - CHECK(StrEqual(appState.jobs[1].input, path3Utf8)); - CHECK(StrEqual(appState.jobs[2].input, pathUtf8)); + CHECK(StrEqual(appState->jobs[0].input, path2Utf8)); + CHECK(StrEqual(appState->jobs[1].input, path3Utf8)); + CHECK(StrEqual(appState->jobs[2].input, pathUtf8)); // Remove middle, move remaining - CHECK(RemoveJob(&appState, 1)); - CHECK(appState.jobCount == 2); + CHECK(RemoveJob(appState, 1)); + CHECK(appState->jobCount == 2); // [path2, path] - CHECK(MoveJob(&appState, 1, 0)); + CHECK(MoveJob(appState, 1, 0)); // [path, path2] - CHECK(StrEqual(appState.jobs[0].input, pathUtf8)); - CHECK(StrEqual(appState.jobs[1].input, path2Utf8)); + CHECK(StrEqual(appState->jobs[0].input, pathUtf8)); + CHECK(StrEqual(appState->jobs[1].input, path2Utf8)); // Remove down to empty - CHECK(RemoveJob(&appState, 0)); - CHECK(RemoveJob(&appState, 0)); - CHECK(appState.jobCount == 0); - CHECK(!RemoveJob(&appState, 0)); - CHECK(appState.jobCount == 0); + CHECK(RemoveJob(appState, 0)); + CHECK(RemoveJob(appState, 0)); + CHECK(appState->jobCount == 0); + CHECK(!RemoveJob(appState, 0)); + CHECK(appState->jobCount == 0); // Re-add after empty - REQUIRE(AddJob(&appState, path3) == AddJobResult::SUCCESS); - CHECK(appState.jobCount == 1); - CHECK(StrEqual(appState.jobs[0].input, path3Utf8)); + REQUIRE(AddJob(appState, path3) == AddJobResult::SUCCESS); + CHECK(appState->jobCount == 1); + CHECK(StrEqual(appState->jobs[0].input, path3Utf8)); } +TEST_SUITE_BEGIN("Adding files"); + TEST_CASE_FIXTURE(AddJobFixture, "HandleOpenFileNameBuffer handles single file") { wchar buff[MAX_PATH_COUNT]; _snwprintf_s(buff, ARR_COUNT(buff), L"%ls", this->path); @@ -393,9 +457,9 @@ TEST_CASE_FIXTURE(AddJobFixture, "HandleOpenFileNameBuffer handles single file") AddJobResult lastError; // fileOffset points to the filename after the last backslash UINT fileOffset = static_cast((StrLengthW(this->dir) + 1)); // dir + '\' + fil)ename - i32 rejected = HandleOpenFileNameBuffer(&appState, buff, fileOffset, &lastError); + i32 rejected = HandleOpenFileNameBuffer(appState, buff, fileOffset, &lastError); CHECK(rejected == 0); - CHECK(appState.jobCount == 1); + CHECK(appState->jobCount == 1); } TEST_CASE_FIXTURE(AddJobFixture, "HandleOpenFileNameBuffer handles multiple files") { @@ -415,9 +479,9 @@ TEST_CASE_FIXTURE(AddJobFixture, "HandleOpenFileNameBuffer handles multiple file AddJobResult lastError; UINT fileOffset = dirLen + 1; - i32 rejected = HandleOpenFileNameBuffer(&appState, buff, fileOffset, &lastError); + i32 rejected = HandleOpenFileNameBuffer(appState, buff, fileOffset, &lastError); CHECK(rejected == 0); - CHECK(appState.jobCount == 3); + CHECK(appState->jobCount == 3); } TEST_CASE_FIXTURE(AddJobFixture, "HandleOpenFileNameBuffer rejects truncated path") { @@ -439,11 +503,13 @@ TEST_CASE_FIXTURE(AddJobFixture, "HandleOpenFileNameBuffer rejects truncated pat AddJobResult lastError; UINT fileOffset = dirLen + 1; - i32 rejected = HandleOpenFileNameBuffer(&appState, buff, fileOffset, &lastError); + i32 rejected = HandleOpenFileNameBuffer(appState, buff, fileOffset, &lastError); CHECK(rejected == 1); - CHECK(appState.jobCount == 0); + CHECK(appState->jobCount == 0); } +TEST_SUITE_END(); + /// ----------------------------------------------------------------------------- /// Config file stuff /// ----------------------------------------------------------------------------- @@ -475,6 +541,8 @@ struct TempConfigFileFixture { ~TempConfigFileFixture() { DeleteFileW(path); } }; +TEST_SUITE_BEGIN("Parsing config"); + // This tests file I/O as well TEST_CASE_FIXTURE(TempConfigFileFixture, "ReadConfigFile works correctly") { struct TC { @@ -489,27 +557,6 @@ TEST_CASE_FIXTURE(TempConfigFileFixture, "ReadConfigFile works correctly") { CHECK_EQ(ok, tc.exp); } -struct AppStateFixture { - AppState appState = {}; - - bool32 - IsZeroed() { - bool32 targetSizesZeroed = true; - for (i32 i = 0; i < ARR_COUNT(appState.targetSizes); ++i) { - if (appState.targetSizes[i] != 0.0f) { - targetSizesZeroed = false; - break; - } - } - - bool32 defaultTargetSizeZeroed = appState.defaultTargetSize == 0.0f; - bool32 codecZeroed = appState.defaultCodec == Codec::NONE; - bool32 outputFolderZeroed = appState.outputFolder[0] == '\0'; - - return targetSizesZeroed && defaultTargetSizeZeroed && codecZeroed && outputFolderZeroed; - } -}; - // Here we don't care about the file I/O anymore, we can just use the raw content as test data TEST_CASE_FIXTURE(AppStateFixture, "ParseConfigBuffer skips comments") { struct TC { @@ -520,7 +567,7 @@ TEST_CASE_FIXTURE(AppStateFixture, "ParseConfigBuffer skips comments") { TC{ "# THis isffect anything\n" "#ASDasd" "test characters### dфывьн - --" }); - ParseConfigBuffer(&appState, tc.content); + ParseConfigBuffer(appState, tc.content); CHECK(this->IsZeroed()); } @@ -538,19 +585,15 @@ TEST_CASE_FIXTURE(AppStateFixture, "ParseConfigBuffer skips garbage input") { "### comment\n1234567890" "#comment\n !!!! #### ????" "" }); - ParseConfigBuffer(&appState, tc.content); + ParseConfigBuffer(appState, tc.content); CHECK(this->IsZeroed()); } -struct ParseConfigAppStateFixture { - AppState appState = {}; -}; - // TODO: this creates folders and deletes them -TEST_CASE_FIXTURE(ParseConfigAppStateFixture, "ParseConfigBuffer works correctly") { +TEST_CASE_FIXTURE(AppStateFixture, "ParseConfigBuffer works correctly") { struct TC { const char* content; - f32 expSizes[ARR_COUNT(appState.targetSizes)]; + f32 expSizes[ARR_COUNT(appState->targetSizes)]; Codec expCodec; const char* expOutput; f32 expDefaultSize; @@ -607,18 +650,18 @@ TEST_CASE_FIXTURE(ParseConfigAppStateFixture, "ParseConfigBuffer works correctly "" }); // TODO: this creates the folder for output path - ParseConfigBuffer(&appState, tc.content); + ParseConfigBuffer(appState, tc.content); INFO(tc.content); - for (i32 i = 0; i < ARR_COUNT(appState.targetSizes); ++i) { - CHECK(appState.targetSizes[i] == doctest::Approx(tc.expSizes[i])); + for (i32 i = 0; i < ARR_COUNT(appState->targetSizes); ++i) { + CHECK(appState->targetSizes[i] == doctest::Approx(tc.expSizes[i])); } - CHECK(appState.defaultTargetSize == doctest::Approx(tc.expDefaultSize)); - CHECK_EQ(appState.defaultCodec, tc.expCodec); + CHECK(appState->defaultTargetSize == doctest::Approx(tc.expDefaultSize)); + CHECK_EQ(appState->defaultCodec, tc.expCodec); - INFO(appState.outputFolder); + INFO(appState->outputFolder); INFO(tc.expOutput); - CHECK(StrEqual(appState.outputFolder, tc.expOutput)); + CHECK(StrEqual(appState->outputFolder, tc.expOutput)); // Cleanup wchar outputW[MAX_PATH_COUNT]; @@ -628,10 +671,11 @@ TEST_CASE_FIXTURE(ParseConfigAppStateFixture, "ParseConfigBuffer works correctly } } +TEST_SUITE_END(); + /// LoadConfigFile -struct LoadConfigFileFixture { - AppState appState = {}; +struct LoadConfigFileFixture : AppStateFixture { wchar dir[MAX_PATH_COUNT]; wchar path[MAX_PATH_COUNT]; @@ -659,34 +703,36 @@ struct LoadConfigFileFixture { void CleanupCreatedOutputFolder() { wchar outputW[MAX_PATH_COUNT]; - UTF8To16(appState.outputFolder, outputW); + UTF8To16(appState->outputFolder, outputW); if (PathFileExistsW(outputW)) { RemoveDirectoryW(outputW); } } }; +TEST_SUITE_BEGIN("Loading config"); + // These just test that the fallbacks work correctly // ParseConfigBuffer does the heavy lifting TEST_CASE_FIXTURE(LoadConfigFileFixture, "LoadConfigFile fails correctly") { - CHECK(!LoadConfigFile(nullptr, &appState, L"invalid/path/file.cfg")); + CHECK(!LoadConfigFile(nullptr, appState, L"invalid/path/file.cfg")); // No sizes is the only content failure this->WriteConfig("[Codecs]\nh264\n"); - CHECK(!LoadConfigFile(nullptr, &appState, path)); - CHECK(appState.defaultCodec == Codec::H264); - INFO(appState.outputFolder); + CHECK(!LoadConfigFile(nullptr, appState, path)); + CHECK(appState->defaultCodec == Codec::H264); + INFO(appState->outputFolder); // TODO: we assign to User/Documents/EasyCompressor // hard to test so we just check this - CHECK(appState.outputFolder[0] != '\0'); + CHECK(appState->outputFolder[0] != '\0'); } TEST_CASE_FIXTURE(LoadConfigFileFixture, "LoadConfigFile applies fallbacks correctly") { this->WriteConfig("[Sizes]\n2.0\n5.0\n"); - REQUIRE(LoadConfigFile(nullptr, &appState, path)); - CHECK(appState.defaultCodec == Codec::H264); - CHECK(appState.defaultTargetSize == doctest::Approx(2.0f)); - CHECK(appState.outputFolder[0] != '\0'); + REQUIRE(LoadConfigFile(nullptr, appState, path)); + CHECK(appState->defaultCodec == Codec::H264); + CHECK(appState->defaultTargetSize == doctest::Approx(2.0f)); + CHECK(appState->outputFolder[0] != '\0'); this->CleanupCreatedOutputFolder(); //appState = {}; @@ -706,40 +752,48 @@ TEST_CASE_FIXTURE(LoadConfigFileFixture, "LoadConfigFile applies fallbacks corre cl!DllGetObjHandler()+0x180fbf Already had this happen before inside AddJob: - UIJob* j = &appState->jobs[appState->jobCount]; + UIJob* j = appState->jobs[appState->jobCount]; *j = {}; I guess the compiler tries to do some magic but crashes in that attempt. Using ZeroMemory manually has fixed the issue every time */ - ZeroMemory(&appState, sizeof(appState)); + //appState = {}; + //ZeroMemory(appState, sizeof(appState)); + // Manual reset as now we use arenas with fixed permanent storage + ResetAppState(appState); this->WriteConfig("[Sizes]\n2.0\n5.0 !\n[Codecs]\nh265 !\n[OutputPath]\nC:\\EasyCompTemp"); - REQUIRE(LoadConfigFile(nullptr, &appState, path)); - CHECK(appState.defaultTargetSize == doctest::Approx(5.0f)); - CHECK(appState.defaultCodec == Codec::H265); - CHECK(StrEqual(appState.outputFolder, "C:\\EasyCompTemp")); + REQUIRE(LoadConfigFile(nullptr, appState, path)); + CHECK(appState->defaultTargetSize == doctest::Approx(5.0f)); + CHECK(appState->defaultCodec == Codec::H265); + CHECK(StrEqual(appState->outputFolder, "C:\\EasyCompTemp")); this->CleanupCreatedOutputFolder(); - //appState = {}; - ZeroMemory(&appState, sizeof(appState)); + ResetAppState(appState); this->WriteConfig("[Sizes]\n7.5\n 6.42\n"); - REQUIRE(LoadConfigFile(nullptr, &appState, path)); - CHECK(appState.defaultTargetSize == doctest::Approx(7.5f)); + REQUIRE(LoadConfigFile(nullptr, appState, path)); + CHECK(appState->defaultTargetSize == doctest::Approx(7.5f)); this->CleanupCreatedOutputFolder(); - //appState = {}; - ZeroMemory(&appState, sizeof(appState)); + ResetAppState(appState); this->WriteConfig("[Sizes]\n2.0\n5.0 !\n4.0\n"); - REQUIRE(LoadConfigFile(nullptr, &appState, path)); - CHECK(appState.defaultTargetSize == doctest::Approx(5.0f)); + REQUIRE(LoadConfigFile(nullptr, appState, path)); + CHECK(appState->defaultTargetSize == doctest::Approx(5.0f)); this->CleanupCreatedOutputFolder(); - //appState = {}; - ZeroMemory(&appState, sizeof(appState)); + ResetAppState(appState); this->WriteConfig("[Sizes]\n2.\n[Codecs]"); - REQUIRE(LoadConfigFile(nullptr, &appState, path)); - CHECK(appState.defaultTargetSize == doctest::Approx(2.0f)); - CHECK(appState.defaultCodec == Codec::H264); + REQUIRE(LoadConfigFile(nullptr, appState, path)); + CHECK(appState->defaultTargetSize == doctest::Approx(2.0f)); + CHECK(appState->defaultCodec == Codec::H264); this->CleanupCreatedOutputFolder(); } + +TEST_SUITE_END(); + +/// ----------------------------------------------------------------------------- +/// Arena +/// ----------------------------------------------------------------------------- + +// TODO: diff --git a/src/win32_compressor.cpp b/src/win32_compressor.cpp index 47b47d2..3858e9e 100644 --- a/src/win32_compressor.cpp +++ b/src/win32_compressor.cpp @@ -59,6 +59,7 @@ #include "compressor.h" +#include "arena.h" #include "win32_compressor.h" // 😈, works :) @@ -243,7 +244,8 @@ ConstructPathsForJob(AppState* appState, UIJob* j, const wchar* path) { // TODO: this probably suffices as we don't have to do any string processing really // Just always use the output folder specified and remove the checkbox if (lastSlash) { - _snprintf_s(j->output, ARR_COUNT(j->output), "%s\\%s", appState->outputFolder, lastSlash); + _snprintf_s(j->output, MAX_PATH_COUNT, _TRUNCATE, "%s\\%s", appState->outputFolder, + lastSlash); } else { INVALID_CODE_PATH; } @@ -286,12 +288,14 @@ static bool32 IsPathFromOutputFolder(AppState* appState, const wchar* path) { i32 amount = StrLengthW(path); ASSERT(amount >= 0 && amount < MAX_PATH_COUNT); - wchar pathW[MAX_PATH_COUNT]; + //wchar pathW[MAX_PATH_COUNT]; + wchar* pathW = PushArray(&appState->scratchArena, wchar, MAX_PATH_COUNT); // Include null term and as it's wide we have to multiply by the size CopyMemory(pathW, path, (amount + 1) * sizeof(wchar)); // This removes the file or a folder, so any last "element" of the path - PathCchRemoveFileSpec(pathW, ARR_COUNT(pathW)); - char copy[MAX_PATH_COUNT]; + PathCchRemoveFileSpec(pathW, MAX_PATH_COUNT); + //char copy[MAX_PATH_COUNT]; + char* copy = PushArray(&appState->scratchArena, char, MAX_PATH_COUNT); UTF16To8(pathW, copy); return StrEqual(copy, appState->outputFolder); } @@ -305,9 +309,13 @@ AddJob(AppState* appState, const wchar* path) { for (i32 i = 0; i < appState->jobCount; ++i) { UIJob* job = &appState->jobs[i]; - char buff[ARR_COUNT(job->input)]; - UTF16To8(path, buff, ARR_COUNT(buff)); - if (StrEqual(job->input, buff)) { + //char buff[ARR_COUNT(job->input)]; + char* buff = PushArray(&appState->scratchArena, char, MAX_PATH_COUNT); + //UTF16To8(path, buff, ARR_COUNT(buff)); + UTF16To8(path, buff, MAX_PATH_COUNT); + bool32 equal = StrEqual(job->input, buff); + ArenaPop(&appState->scratchArena, MAX_PATH_COUNT); + if (equal) { return AddJobResult::DUPLICATE_JOB; } } @@ -321,7 +329,14 @@ AddJob(AppState* appState, const wchar* path) { UIJob* j = &appState->jobs[appState->jobCount]; //*j = {}; + + // We have to save the permanent storage locations + char* inputAddress = j->input; + char* outputAddress = j->output; ZeroMemory(j, sizeof(*j)); + j->input = inputAddress; + j->output = outputAddress; + ASSERT(j->input != nullptr && j->output != nullptr); j->status = JobStatus::QUEUED; j->targetSizeMb = appState->defaultTargetSize; @@ -357,7 +372,16 @@ RemoveJob(AppState* appState, i32 index) { i32 newJobCount = appState->jobCount - 1; ASSERT(newJobCount >= 0); for (i32 i = index; i < newJobCount; ++i) { + char* inputAddress = appState->jobs[i].input; + char* outputAddress = appState->jobs[i].output; + appState->jobs[i] = appState->jobs[i + 1]; + // Copy string contents + _snprintf_s(inputAddress, MAX_PATH_COUNT, _TRUNCATE, "%s", appState->jobs[i + 1].input); + _snprintf_s(outputAddress, MAX_PATH_COUNT, _TRUNCATE, "%s", appState->jobs[i + 1].output); + + appState->jobs[i].input = inputAddress; + appState->jobs[i].output = outputAddress; } _InterlockedExchange(&appState->jobCount, newJobCount); @@ -969,8 +993,10 @@ HandleAddJobResults(AppState* appState, i32 rejected, AddJobResult lastErrorResu static void GetExeDirectory(AppState* appState) { - wchar exeDir[ARR_COUNT(appState->exeDir)]; - GetModuleFileNameW(nullptr, exeDir, MAX_PATH_COUNT); + u64 size = MAX_PATH_COUNT; + wchar* exeDir = PushArray(&appState->scratchArena, wchar, size); + //wchar exeDir[ARR_COUNT(appState->exeDir)]; + GetModuleFileNameW(nullptr, exeDir, static_cast(size)); //UTF16To8Fixed(exeDir, appState->exeDir, ARR_COUNT(appState->exeDir)); UTF16To8(exeDir, appState->exeDir); @@ -986,27 +1012,35 @@ GetExeDirectory(AppState* appState) { if (lastSlashIndex >= 0) { appState->exeDir[lastSlashIndex] = '\0'; } + + ArenaPop(&appState->scratchArena, size); } static void -SelectInExplorer(HWND hWnd, const char* path) { - wchar pathW[MAX_PATH_COUNT]; +SelectInExplorer(HWND hWnd, Arena* arena, const char* path) { + // TODO: scratch arena needed? + wchar* pathW = PushArray(arena, wchar, MAX_PATH_COUNT); + //wchar pathW[MAX_PATH_COUNT]; UTF8To16(path, pathW); - wchar cmd[MAX_PATH_COUNT]; - _snwprintf_s(cmd, ARR_COUNT(cmd), L"/select,\"%ls\"", pathW); + //wchar cmd[MAX_PATH_COUNT]; + wchar* cmd = PushArray(arena, wchar, MAX_PATH_COUNT); + _snwprintf_s(cmd, MAX_PATH_COUNT, _TRUNCATE, L"/select,\"%ls\"", pathW); ShellExecuteW(hWnd, L"open", L"explorer.exe", cmd, nullptr, SW_SHOWNORMAL); } static void -OpenInExplorer(HWND hWnd, const char* path) { - wchar pathW[MAX_PATH_COUNT]; +OpenInExplorer(HWND hWnd, Arena* arena, const char* path) { + // TODO: scratch arena needed? + wchar* pathW = PushArray(arena, wchar, MAX_PATH_COUNT); UTF8To16(path, pathW); ShellExecuteW(hWnd, L"open", L"explorer.exe", pathW, nullptr, SW_SHOWNORMAL); } static void -PickOutputFile(HINSTANCE hInstance, HWND hWnd, char* outPath) { - wchar buffer[MAX_PATH_COUNT] = {}; +PickOutputFile(HINSTANCE hInstance, HWND hWnd, Arena* arena, char* outPath) { + // TODO: scratch arena needed? + //wchar buff[MAX_PATH_COUNT] = {}; + wchar* buff = PushArrayZero(arena, wchar, MAX_PATH_COUNT); OPENFILENAMEW ofn = {}; ofn.lStructSize = sizeof(ofn); @@ -1020,9 +1054,9 @@ PickOutputFile(HINSTANCE hInstance, HWND hWnd, char* outPath) { "*.mp4;*.mov;*.mkv;*.avi;*.webm\0" "All Files (*.*)\0" "*.*\0"; - ofn.lpstrFile = buffer; + ofn.lpstrFile = buff; ofn.lpstrTitle = L"Select output file"; - ofn.nMaxFile = ARR_COUNT(buffer); + ofn.nMaxFile = MAX_PATH_COUNT; // NOCHANGE_DIR to prevent generating imgui.ini ofn.Flags = OFN_EXPLORER | OFN_PATHMUSTEXIST | OFN_OVERWRITEPROMPT | OFN_NOCHANGEDIR; ofn.lpstrDefExt = L"mp4"; @@ -1058,10 +1092,12 @@ PickOutputFile(HINSTANCE hInstance, HWND hWnd, char* outPath) { */ static void SaveOutputPathToConfig(AppState* appState, const char* path) { - wchar configFilePath[ARR_COUNT(appState->appData) + 32]; - UTF8To16(appState->appData, configFilePath); - _snwprintf_s(configFilePath, ARR_COUNT(configFilePath), L"%ls\\easycompressor.cfg", - configFilePath); + //wchar configFilePath[ARR_COUNT(appState->appData) + 32]; + wchar* configFilePath = PushArray(&appState->scratchArena, wchar, MAX_PATH_COUNT); + //UTF8To16(appState->appData, configFilePath); + //_snwprintf_s(configFilePath, ARR_COUNT(configFilePath), L"%ls\\easycompressor.cfg", + // configFilePath); + UTF8To16(appState->configFilePath, configFilePath); HANDLE file = CreateFileW(configFilePath, GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); if (file == INVALID_HANDLE_VALUE) { @@ -1069,10 +1105,12 @@ SaveOutputPathToConfig(AppState* appState, const char* path) { return; } - char buff[MAX_PATH_COUNT + 512]; + //char buff[MAX_PATH_COUNT + 512]; + //char* buff = PushArray(&appState->scratchArena, char, MAX_PATH_COUNT + 512); + char* buff = PushArray(&appState->scratchArena, char, CONFIG_FILE_MAX_SIZE); DWORD read = 0; - ReadFile(file, buff, ARR_COUNT(buff), &read, nullptr); + ReadFile(file, buff, CONFIG_FILE_MAX_SIZE, &read, nullptr); CloseHandle(file); buff[read] = '\0'; @@ -1119,10 +1157,11 @@ PickoutputFolder(HWND hWnd, AppState* appState) { LPITEMIDLIST result = SHBrowseForFolderW(&bi); if (result) { - wchar buffer[MAX_PATH_COUNT] = {}; - if (SHGetPathFromIDListW(result, buffer)) { + //wchar buff[MAX_PATH_COUNT] = {}; + wchar* buff = PushArrayZero(&appState->scratchArena, wchar, MAX_PATH_COUNT); + if (SHGetPathFromIDListW(result, buff)) { //CopyMemory(appState->outputFolder, buffer, MAX_PATH_COUNT); - UTF16To8(buffer, appState->outputFolder); + UTF16To8(buff, appState->outputFolder); // This overwrites the selected value always // I think it's the most expected approach as if the user selects a new output path it // should be automatically be activated @@ -1160,14 +1199,17 @@ HandleOpenFileNameBuffer(AppState* appState, const wchar* buffer, UINT fileOffse } else { // Multiple files while (*file) { - wchar fullW[MAX_PATH_COUNT]; + Arena* scratch = &appState->scratchArena; + u64 startPos = ArenaGetPos(scratch); + //wchar fullW[MAX_PATH_COUNT]; + wchar* fullW = PushArray(scratch, wchar, MAX_PATH_COUNT); #if !COMPRESSOR_TESTS - i32 written = _snwprintf_s(fullW, ARR_COUNT(fullW), L"%ls\\%ls", dir, file); + i32 written = _snwprintf_s(fullW, MAX_PATH_COUNT, _TRUNCATE, L"%ls\\%ls", dir, file); #else // To prevent crash when truncation would happen due to test data - i32 written = _snwprintf_s(fullW, ARR_COUNT(fullW), _TRUNCATE, L"%ls\\%ls", dir, file); + i32 written = _snwprintf_s(fullW, MAX_PATH_COUNT, _TRUNCATE, L"%ls\\%ls", dir, file); #endif - if (written < 0 || written >= ARR_COUNT(fullW)) { + if (written < 0 || written >= MAX_PATH_COUNT) { ++rejected; file += StrLengthW(file) + 1; continue; @@ -1180,6 +1222,8 @@ HandleOpenFileNameBuffer(AppState* appState, const wchar* buffer, UINT fileOffse } file += StrLengthW(file) + 1; + + ArenaSetPos(scratch, startPos); } } @@ -1189,7 +1233,9 @@ HandleOpenFileNameBuffer(AppState* appState, const wchar* buffer, UINT fileOffse static void PickInputFiles(HINSTANCE hInstance, HWND hWnd, AppState* appState) { // TODO: Take care of stack size if MAX_PATH_COUNT is changed - wchar buff[MAX_PATH_COUNT * ARR_COUNT(appState->jobs)] = {}; + //wchar buff[MAX_PATH_COUNT * ARR_COUNT(appState->jobs)] = {}; + u64 size = MAX_PATH_COUNT * ARR_COUNT(appState->jobs); + wchar* buff = PushArrayZero(&appState->scratchArena, wchar, size); OPENFILENAME ofn = {}; ofn.lStructSize = sizeof(ofn); @@ -1204,7 +1250,8 @@ PickInputFiles(HINSTANCE hInstance, HWND hWnd, AppState* appState) { "*.*\0"; ofn.lpstrFile = buff; ofn.lpstrTitle = L"Select input files"; - ofn.nMaxFile = ARR_COUNT(buff); + //ofn.nMaxFile = ARR_COUNT(buff); + ofn.nMaxFile = static_cast(size); ofn.Flags = OFN_EXPLORER | OFN_FILEMUSTEXIST | OFN_ALLOWMULTISELECT; ofn.lpstrDefExt = L"mp4"; @@ -1218,6 +1265,8 @@ PickInputFiles(HINSTANCE hInstance, HWND hWnd, AppState* appState) { i32 rejected = HandleOpenFileNameBuffer(appState, buff, ofn.nFileOffset, &lastErrorResult); HandleAddJobResults(appState, rejected, lastErrorResult); + + ArenaPop(&appState->scratchArena, size); } static const char* @@ -1512,8 +1561,7 @@ ParseConfigBuffer(AppState* appState, const char* buff) { if (valid) { DEBUG_PRINTF("Using output path: %s\n", outputPath); - _snprintf_s(appState->outputFolder, ARR_COUNT(appState->outputFolder), - outputPath); + _snprintf_s(appState->outputFolder, MAX_PATH_COUNT, _TRUNCATE, outputPath); } else { DEBUG_PRINTF("Invalid output path: %s\n", outputPath); } @@ -1529,25 +1577,30 @@ ParseConfigBuffer(AppState* appState, const char* buff) { static bool32 LoadConfigFile(HWND hWnd, AppState* appState, const wchar* path) { - char buff[CONFIG_FILE_MAX_SIZE]; - bool32 ok = ReadConfigFile(path, buff, ARR_COUNT(buff)); + //char buff[CONFIG_FILE_MAX_SIZE]; + u64 size = CONFIG_FILE_MAX_SIZE; + char* buff = PushArrayZero(&appState->scratchArena, char, size); + //bool32 ok = ReadConfigFile(path, buff, ARR_COUNT(buff)); + bool32 ok = ReadConfigFile(path, buff, static_cast(size)); if (!ok) { return false; } ParseConfigBuffer(appState, buff); + ArenaPop(&appState->scratchArena, size); + /// Fallbacks // Having no output path is NOT considered a failure // We just use the default User/Documents/EasyCompressor if (appState->outputFolder[0] == '\0') { + // TODO: scratch wchar outputFolder[MAX_PATH_COUNT]; if (!SUCCEEDED(SHGetFolderPathW(hWnd, CSIDL_MYDOCUMENTS, nullptr, 0, outputFolder))) { DEBUG_PRINT("Couldn't get user documents folder, using exe dir as working dir...\n"); // Use exe dir as working dir... - _snprintf_s(appState->outputFolder, ARR_COUNT(appState->outputFolder), "%s", - appState->exeDir); + _snprintf_s(appState->outputFolder, MAX_PATH_COUNT, _TRUNCATE, "%s", appState->exeDir); } else { DEBUG_PRINTF("Found documents %ls\n", outputFolder); _snwprintf_s(outputFolder, ARR_COUNT(outputFolder), L"%ls\\EasyCompressor", @@ -1562,7 +1615,7 @@ LoadConfigFile(HWND hWnd, AppState* appState, const wchar* path) { INVALID_CODE_PATH; DEBUG_PRINTF("SHGetFolderPathA returned %s but couldn't create the directory " "there. Using exe dir...\n"); - _snprintf_s(appState->outputFolder, ARR_COUNT(appState->outputFolder), "%s", + _snprintf_s(appState->outputFolder, MAX_PATH_COUNT, _TRUNCATE, "%s", appState->exeDir); } } @@ -1619,7 +1672,10 @@ ClearJobs(AppState* appState) { static void OverwriteAndLoadNewConfig(HWND hWnd, AppState* appState) { - wchar configFilePathW[MAX_PATH_COUNT]; + //wchar configFilePathW[MAX_PATH_COUNT]; + u64 size = MAX_PATH_COUNT; + wchar* configFilePathW = PushArrayZero(&appState->scratchArena, wchar, size); + UTF8To16(appState->configFilePath, configFilePathW); if (DeleteFileW(configFilePathW)) { if (CreateDefaultConfigFile(configFilePathW)) { @@ -1635,6 +1691,67 @@ OverwriteAndLoadNewConfig(HWND hWnd, AppState* appState) { } else { SetErrorMsg(appState, "Couldn't delete old config"); } + + ArenaPop(&appState->scratchArena, size); +} + +static bool32 +InitAppState(AppState* appState) { + // 64 bit +#if COMPRESSOR_DEV + LPVOID baseAddress = reinterpret_cast(TB(2)); +#else + LPVOID baseAddress = nullptr; +#endif + + u64 permanentSize = (MAX_PATH_COUNT * 7) + (MAX_PATH_COUNT * 2 * MAX_JOBS); + u64 scratchSize = MB(4); + u64 totalSize = permanentSize + scratchSize; + void* memory = VirtualAlloc(baseAddress, totalSize, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); + if (!memory) { + DEBUG_PRINT("VirtualAlloc failed!\n"); + return false; + } + + f32 totalSizeKB = static_cast(totalSize) / 1024.0f; + DEBUG_PRINTF("Allocated %llu bytes, KB: %.2f\n", totalSize, totalSizeKB); + + appState->permanentMemory = memory; + appState->permanentMemorySize = permanentSize; + appState->scratchMemory = static_cast(memory) + permanentSize; + appState->scratchMemorySize = scratchSize; + + // Assign permanent storage locations + { + Arena* permanent = &appState->permanentArena; + InitArena(permanent, appState->permanentMemory, appState->permanentMemorySize); + DEBUG_PRINTF("Permanent arena base: %llu, size: %llu\n", appState->permanentMemory, + appState->permanentMemorySize); + + u64 size = MAX_PATH_COUNT; + + appState->outputFolder = PushArray(permanent, char, size); + appState->exeDir = PushArray(permanent, char, size); + appState->tempDir = PushArray(permanent, char, size); + appState->appData = PushArray(permanent, char, size); + appState->imguiIniPath = PushArray(permanent, char, size); + appState->configFilePath = PushArray(permanent, char, size); + appState->ffmpegPath = PushArray(permanent, char, size); + + for (i32 i = 0; i < ARR_COUNT(appState->jobs); ++i) { + appState->jobs[i].input = PushArray(permanent, char, size); + appState->jobs[i].output = PushArray(permanent, char, size); + } + + ASSERT(permanent->used == permanentSize); + } + + Arena* scratch = &appState->scratchArena; + InitArena(scratch, appState->scratchMemory, appState->scratchMemorySize); + DEBUG_PRINTF("Scratch arena base: %llu, size: %llu\n", appState->scratchMemory, + appState->scratchMemorySize); + + return true; } // Tests don't need this stuff @@ -1679,7 +1796,7 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) } if (ImGui::MenuItem("Open config folder...")) { - OpenInExplorer(hWnd, appState->appData); + //OpenInExplorer(hWnd, &appState->scratchArena, appState->appData); } ImGui::EndMenu(); @@ -1699,6 +1816,21 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) # if COMPRESSOR_DEV ImGui::TextDisabled("DEV: compressing: %d, cancelled: %d", compressing, cancelled); + Arena* permanent = &appState->permanentArena; + Arena* scratch = &appState->scratchArena; + + f32 toMB = 1 / (1024.0f * 1024.0f); + f32 scratchUsedMB = scratch->used * toMB; + f32 scratchTotalUsedMB = scratch->totalUsed * toMB; + f32 permUsedMB = permanent->used * toMB; + f32 permTotalUsedMB = permanent->totalUsed * toMB; + + ImGui::TextDisabled("Permanent size: %llu, used: %llu (%.2f MB), total used: %llu (%.2f MB)", + permanent->size, permanent->used, permUsedMB, permanent->totalUsed, + permTotalUsedMB); + ImGui::TextDisabled("Scratch size: %llu, used: %llu (%.2f MB), total used: %llu (%.2f MB)", + scratch->size, scratch->used, scratchUsedMB, scratch->totalUsed, + scratchTotalUsedMB); # endif const f32 sliderWidth = 190 * scale; @@ -1794,10 +1926,10 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) PickoutputFolder(hWnd, appState); ImGui::GetIO().ClearInputMouse(); } else if (ImGui::IsItemClicked(ImGuiMouseButton_Middle)) { - OpenInExplorer(hWnd, appState->outputFolder); + OpenInExplorer(hWnd, &appState->scratchArena, appState->outputFolder); } else if (ImGui::BeginPopupContextItem("default_folder_menu")) { if (ImGui::MenuItem("Open folder in explorer...")) { - OpenInExplorer(hWnd, appState->outputFolder); + OpenInExplorer(hWnd, &appState->scratchArena, appState->outputFolder); } ImGui::EndPopup(); @@ -1960,14 +2092,14 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) } if (ImGui::IsItemClicked(ImGuiMouseButton_Left)) { - PickOutputFile(hInstance, hWnd, job->output); + PickOutputFile(hInstance, hWnd, &appState->scratchArena, job->output); // This is done to reset broken hover state after the dialog closes ImGui::GetIO().ClearInputMouse(); } else if (ImGui::IsItemClicked(ImGuiMouseButton_Middle)) { - SelectInExplorer(hWnd, job->input); + SelectInExplorer(hWnd, &appState->scratchArena, job->input); } else if (ImGui::BeginPopupContextItem("job_context_menu")) { if (ImGui::MenuItem("Open input in explorer...")) { - SelectInExplorer(hWnd, job->input); + SelectInExplorer(hWnd, &appState->scratchArena, job->input); } // TODO: more? @@ -2076,7 +2208,7 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) ImGui::CalcTextSize(label).x + ImGui::GetStyle().FramePadding.x * 2.0f; ImGui::SetCursorPosX(ImGui::GetCursorPosX() + avail - buttonWidth); if (ImGui::SmallButton(label)) { - SelectInExplorer(hWnd, job->output); + SelectInExplorer(hWnd, &appState->scratchArena, job->output); } ImGui::PopStyleColor(2); @@ -2261,7 +2393,7 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) ImGui::SameLine(0, gap); if (ImGui::Button(btn3)) { - OpenInExplorer(hWnd, appState->appData); + OpenInExplorer(hWnd, &appState->scratchArena, appState->appData); } ImGui::EndPopup(); @@ -2390,19 +2522,22 @@ WndProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) { AddJobResult lastErrorResult = AddJobResult::SUCCESS; for (UINT i = 0; i < fileCount && i < MAX_JOBS; ++i) { - wchar path[MAX_PATH_COUNT]; + //wchar path[MAX_PATH_COUNT]; + ASSERT(gAppState); + wchar* path = PushArray(&gAppState->scratchArena, wchar, MAX_PATH_COUNT); + // Query the required character amount first, not including null terminator // If we don't query we have no way of deducing if the path was truncated or it's // exactly MAX_PATH_COUNT long UINT required = DragQueryFileW(drop, i, nullptr, 0); // Get the path and get the copied amount, not including null terminator - UINT copied = DragQueryFileW(drop, i, path, ARR_COUNT(path)); + UINT copied = DragQueryFileW(drop, i, path, MAX_PATH_COUNT); DEBUG_PRINTF("Required: %u, copied %u (both not including null terminator)\n", required, copied); // required > copied would work as well - if (required >= ARR_COUNT(path)) { + if (required >= MAX_PATH_COUNT) { DEBUG_PRINTF("Path was truncated, didn't add job! Max length: %d\nPath would have " "been %s\n", MAX_PATH_COUNT, path); @@ -2410,7 +2545,6 @@ WndProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) { continue; } - ASSERT(gAppState); auto result = AddJob(gAppState, path); if (result != AddJobResult::SUCCESS) { lastErrorResult = result; @@ -2528,9 +2662,18 @@ WinMain(HINSTANCE hInstance, HINSTANCE /*unused*/, LPSTR /*unused*/, int /*unuse io.ConfigDpiScaleFonts = true; // Automatically scales fonts for docking branch io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard; + /// Memory + AppState appState = {}; + if (!InitAppState(&appState)) { + DEBUG_PRINT("InitAppState failed!"); + return 0; + } + gAppState = &appState; + Arena* scratch = &appState.scratchArena; + GetExeDirectory(&appState); DEBUG_PRINTF("Exe dir: %s\n", appState.exeDir); @@ -2538,9 +2681,12 @@ WinMain(HINSTANCE hInstance, HINSTANCE /*unused*/, LPSTR /*unused*/, int /*unuse { # if 1 - char buff[MAX_PATH_COUNT + 64]; + //char buff[MAX_PATH_COUNT + 64]; + + u64 size = MAX_PATH_COUNT + 64; + char* buff = PushArray(scratch, char, size); // This should show basically all Unicode characters - _snprintf_s(buff, ARR_COUNT(buff), "%s\\vendor\\NotoSans-Regular.ttf", appState.exeDir); + _snprintf_s(buff, size, _TRUNCATE, "%s\\vendor\\NotoSans-Regular.ttf", appState.exeDir); ImFontConfig fontConfig = {}; // TODO: Flags is not yet exposed to be externally modified as of v1.92.7 // See ImFontConfig declaration @@ -2552,6 +2698,8 @@ WinMain(HINSTANCE hInstance, HINSTANCE /*unused*/, LPSTR /*unused*/, int /*unuse if (!font) { DEBUG_PRINTF("Couldn't load font file: %s\n", buff); } + + ArenaPop(scratch, size); # else io.Fonts->AddFontFromFileTTF("C:\\Windows\\Fonts\\segoeui.ttf", 18.0f, nullptr, io.Fonts->GetGlyphRangesDefault()); @@ -2567,42 +2715,47 @@ WinMain(HINSTANCE hInstance, HINSTANCE /*unused*/, LPSTR /*unused*/, int /*unuse /// Temp dir { - wchar tempDir[MAX_PATH_COUNT]; - DWORD result = GetTempPathW(MAX_PATH_COUNT, tempDir); + //wchar tempDir[MAX_PATH_COUNT]; + u64 size = MAX_PATH_COUNT; + wchar* tempDir = PushArray(scratch, wchar, size); + DWORD result = GetTempPathW(static_cast(size), tempDir); if (!result) { DEBUG_PRINT("Couldn't get temp folder, using exe dir as temp dir...\n"); // Use exe dir as working dir... - _snprintf_s(appState.tempDir, ARR_COUNT(appState.tempDir), "%s", appState.exeDir); + _snprintf_s(appState.tempDir, size, _TRUNCATE, "%s", appState.exeDir); } // No space! - else if (result >= ARR_COUNT(appState.tempDir)) { + else if (result >= size) { DEBUG_PRINT("No space for temp dir, using exe dir...\n"); - _snprintf_s(appState.tempDir, ARR_COUNT(appState.tempDir), "%s", appState.exeDir); + _snprintf_s(appState.tempDir, size, _TRUNCATE, "%s", appState.exeDir); } else { // Of course this API is different from SHGetFolderPathA, which doesn't have a backslash - ASSERT(result > 0 && result < ARR_COUNT(appState.tempDir)); - if (result > 0 && result < ARR_COUNT(appState.tempDir)) { + ASSERT(result > 0 && result < size); + if (result > 0 && result < size) { UTF16To8(tempDir, appState.tempDir); appState.tempDir[result - 1] = '\0'; } else { - _snprintf_s(appState.tempDir, ARR_COUNT(appState.tempDir), "%s", appState.exeDir); + _snprintf_s(appState.tempDir, size, _TRUNCATE, "%s", appState.exeDir); } } + + ArenaPop(scratch, size); } /// Config file // TODO: exact same code as above { - wchar appData[MAX_PATH_COUNT]; + u64 size = MAX_PATH_COUNT; + wchar* appData = PushArray(scratch, wchar, size); if (!SUCCEEDED(SHGetFolderPathW(hWnd, CSIDL_LOCAL_APPDATA, nullptr, 0, appData))) { DEBUG_PRINT("Couldn't get user appdata folder, using exe dir as working dir...\n"); - _snprintf_s(appState.appData, ARR_COUNT(appState.appData), "%s", appState.exeDir); + _snprintf_s(appState.appData, size, _TRUNCATE, "%s", appState.exeDir); } else { // Also very cumbersome to print wide strings, have to use %ls // probably doesn't even work correctly for special characters... DEBUG_PRINTF("Found appdata %ls\n", appData); - _snwprintf_s(appData, ARR_COUNT(appData), L"%ls\\%ls", appData, COMPRESSOR_NAMEW); + _snwprintf_s(appData, size, _TRUNCATE, L"%ls\\%ls", appData, COMPRESSOR_NAMEW); BOOL created = CreateDirectoryW(appData, nullptr); // This is extremely annoying to do having to convert all the time UTF16To8(appData, appState.appData); @@ -2611,49 +2764,60 @@ WinMain(HINSTANCE hInstance, HINSTANCE /*unused*/, LPSTR /*unused*/, int /*unuse if (err != ERROR_ALREADY_EXISTS) { DEBUG_PRINTF("SHGetFolderPathA returned %s but couldn't create the directory " "there. Using exe dir...\n"); - _snprintf_s(appState.appData, ARR_COUNT(appState.appData), "%s", - appState.exeDir); + _snprintf_s(appState.appData, size, _TRUNCATE, "%s", appState.exeDir); INVALID_CODE_PATH; } } } + + ArenaPop(scratch, size); } - _snprintf_s(appState.configFilePath, ARR_COUNT(appState.configFilePath), - "%s\\easycompressor.cfg", appState.appData); + _snprintf_s(appState.configFilePath, MAX_PATH_COUNT, _TRUNCATE, "%s\\easycompressor.cfg", + appState.appData); - wchar configPathW[MAX_PATH_COUNT]; - UTF8To16(appState.configFilePath, configPathW); - bool32 ok = LoadConfigFile(hWnd, &appState, configPathW); - if (!ok) { - bool32 configExists = PathFileExistsW(configPathW); - if (configExists) { - SetPopupErrorMsg(&appState, - "Config is malformed, consider fixing or deleting it\n" - "The app will generate a new one upon startup after deletion"); - } else { - bool32 created = CreateDefaultConfigFile(configPathW); - if (created) { - DEBUG_PRINT("Loading just created config file...\n"); - LoadConfigFile(hWnd, &appState, configPathW); + { + u64 size = MAX_PATH_COUNT; + wchar* configPathW = PushArray(scratch, wchar, size); + UTF8To16(appState.configFilePath, configPathW); + bool32 ok = LoadConfigFile(hWnd, &appState, configPathW); + if (!ok) { + bool32 configExists = PathFileExistsW(configPathW); + if (configExists) { + SetPopupErrorMsg(&appState, + "Config is malformed, consider fixing or deleting it\n" + "The app will generate a new one upon startup after deletion"); } else { - DEBUG_PRINT("Fallback to using default target sizes...\n"); - f32 defaultTargetSizes[ARR_COUNT(appState.targetSizes)] = { 5.0f, 10.0f, 25.0, 50.0, - 100.0f }; - CopyMemory(appState.targetSizes, defaultTargetSizes, sizeof(appState.targetSizes)); - appState.defaultTargetSize = defaultTargetSizes[1]; + bool32 created = CreateDefaultConfigFile(configPathW); + if (created) { + DEBUG_PRINT("Loading just created config file...\n"); + LoadConfigFile(hWnd, &appState, configPathW); + } else { + DEBUG_PRINT("Fallback to using default target sizes...\n"); + f32 defaultTargetSizes[ARR_COUNT(appState.targetSizes)] = { 5.0f, 10.0f, 25.0, + 50.0, 100.0f }; + CopyMemory(appState.targetSizes, defaultTargetSizes, + sizeof(appState.targetSizes)); + appState.defaultTargetSize = defaultTargetSizes[1]; + } } } + + ArenaPop(scratch, size); } - // imgui.ini also to same path - char configPath[MAX_PATH_COUNT]; - _snprintf_s(configPath, ARR_COUNT(configPath), "%s\\imgui.ini", appState.appData); - io.IniFilename = configPath; + { + // imgui.ini also to same path + _snprintf_s(appState.imguiIniPath, MAX_PATH_COUNT, _TRUNCATE, "%s\\imgui.ini", + appState.appData); + io.IniFilename = appState.imguiIniPath; + + // TODO: support package managers so read from PATH + u64 size = MAX_PATH_COUNT; + _snprintf_s(appState.ffmpegPath, size, _TRUNCATE, "%s\\vendor\\ffmpeg", appState.exeDir); - // TODO: support package managers so read from PATH - _snprintf_s(appState.ffmpegPath, ARR_COUNT(appState.ffmpegPath), "%s\\vendor\\ffmpeg", - appState.exeDir); + ArenaPop(scratch, size); + } // Start worker thread HANDLE workerThread = CreateThread(nullptr, 0, WorkerThread, &appState, 0, nullptr); @@ -2668,16 +2832,17 @@ WinMain(HINSTANCE hInstance, HINSTANCE /*unused*/, LPSTR /*unused*/, int /*unuse # if COMPRESSOR_DEV DEBUG_PRINT("Adding test files at startup...\n"); - wchar testPath1[MAX_PATH_COUNT]; - wchar testPath2[MAX_PATH_COUNT]; - wchar testPath3[MAX_PATH_COUNT]; + u64 size = MAX_PATH_COUNT; + wchar* testPath1 = PushArray(scratch, wchar, size); + wchar* testPath2 = PushArray(scratch, wchar, size); + wchar* testPath3 = PushArray(scratch, wchar, size); - wchar exeDir[ARR_COUNT(appState.exeDir)]; + wchar* exeDir = PushArray(scratch, wchar, size); UTF8To16(appState.exeDir, exeDir); - _snwprintf_s(testPath1, ARR_COUNT(testPath1), L"%ls\\..\\test_file1_large.mp4", exeDir); - _snwprintf_s(testPath2, ARR_COUNT(testPath2), L"%ls\\..\\test_file2.mp4", exeDir); - _snwprintf_s(testPath3, ARR_COUNT(testPath3), L"%ls\\..\\testi_file_small.mp4", exeDir); + _snwprintf_s(testPath1, size, _TRUNCATE, L"%ls\\..\\test_file1_large.mp4", exeDir); + _snwprintf_s(testPath2, size, _TRUNCATE, L"%ls\\..\\test_file2.mp4", exeDir); + _snwprintf_s(testPath3, size, _TRUNCATE, L"%ls\\..\\testi_file_small.mp4", exeDir); AddJob(&appState, testPath1); AddJob(&appState, testPath2); @@ -2699,6 +2864,9 @@ WinMain(HINSTANCE hInstance, HINSTANCE /*unused*/, LPSTR /*unused*/, int /*unuse //ImVec4 clearColor = ImVec4(0.45f, 0.55f, 0.60f, 1.00f); while (running) { + // TODO: we would not have to pop at every moment if we just do this at the end + ArenaClear(scratch); + auto frameStart = GetWallClock(); // For the first frame delta is ~0.0f, which is fine f32 deltaMs = GetMsElapsed(currentCounter, frameStart); diff --git a/src/win32_compressor.h b/src/win32_compressor.h index 125f85a..1d75b98 100644 --- a/src/win32_compressor.h +++ b/src/win32_compressor.h @@ -30,11 +30,23 @@ enum class AddJobResult : u8 { // MAX_PATH is 260 defined by Windows // TODO: if we want to support wide paths (32 767 characters), would need to allocate heap memory // https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry -#define MAX_PATH_COUNT 512 +#define MAX_PATH_COUNT 32767 // 512 + +#define OPEN_FLASH_TIMER_START 5.0f + +#define MAX_JOBS 50 + +// All in MB +#define MIN_TARGET_SIZE 0.5f +#define MAX_TARGET_SIZE 5000.0f + +#define TARGET_SIZES_COUNT 5 + +#define CONFIG_FILE_MAX_SIZE (1024 + MAX_PATH_COUNT) struct UIJob { - char input[MAX_PATH_COUNT]; - char output[MAX_PATH_COUNT]; // No suffix anymore + char* input; + char* output; // No suffix anymore //bool32 hasValidFileExtension; Some file extension is required by ffmpeg pass 2 // We pass "-f mp4" as default if this is false @@ -54,18 +66,6 @@ struct UIJob { f32 openFlashTimer; // Open button flash }; -#define OPEN_FLASH_TIMER_START 5.0f - -#define MAX_JOBS 50 - -// All in MB -#define MIN_TARGET_SIZE 0.5f -#define MAX_TARGET_SIZE 5000.0f - -#define TARGET_SIZES_COUNT 5 - -#define CONFIG_FILE_MAX_SIZE (1024 + MAX_PATH_COUNT) - // Mainly for popup dialogs struct UIState { char errorMsg[256]; // Buffer for different error messages @@ -79,6 +79,15 @@ struct UIState { }; struct AppState { + void* permanentMemory; + u64 permanentMemorySize; + void* scratchMemory; + u64 scratchMemorySize; + + // Permanent arena is not necessary but helps the setup code so it's not so manual + Arena permanentArena; + Arena scratchArena; + UIJob jobs[MAX_JOBS]; volatile long jobCount; // If we were to use condition variables instead of spin lock @@ -95,14 +104,16 @@ struct AppState { // All UI state with ImGui UIState uiState; - char outputFolder[MAX_PATH_COUNT]; // User/Documents + char* outputFolder; // User/Documents //bool32 useOutputFolder; - char exeDir[MAX_PATH_COUNT]; // Absolute path to the exe directory - char tempDir[MAX_PATH_COUNT]; // Temp dir for ffmpeg logs + char* exeDir; // Absolute path to the exe directory + char* tempDir; // Temp dir for ffmpeg logs + + char* appData; // AppData/Local/EasyCompressor, where the config and imgui.ini + // are stored + char* imguiIniPath; // AppData/Local/EasyCompressor/imgui.ini + char* configFilePath; // AppData/Local/EasyCompressor/easycompressor.cfg - char appData[MAX_PATH_COUNT]; // AppData/Local/EasyCompressor, where the config and imgui.ini - // are stored - char configFilePath[MAX_PATH_COUNT]; // AppData/Local/EasyCompressor/easycompressor.cfg - char ffmpegPath[MAX_PATH_COUNT]; // Relative to working directory OR TODO: path? + char* ffmpegPath; // Relative to working directory OR TODO: path? }; From 0e404a0ad7ba28269749340c9ff852678d2e8ec4 Mon Sep 17 00:00:00 2001 From: Juuso Piippo Date: Tue, 9 Jun 2026 00:42:25 +0300 Subject: [PATCH 2/5] Tests for arena operations LLM generated, seems good though and covers a lot of cases --- src/compressor_tests.cpp | 237 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 236 insertions(+), 1 deletion(-) diff --git a/src/compressor_tests.cpp b/src/compressor_tests.cpp index 58d1aff..2daa57c 100644 --- a/src/compressor_tests.cpp +++ b/src/compressor_tests.cpp @@ -796,4 +796,239 @@ TEST_SUITE_END(); /// Arena /// ----------------------------------------------------------------------------- -// TODO: +TEST_SUITE_BEGIN("Arena"); + +struct ArenaFixture { + static constexpr u64 arenaSize = 1024; + u8 buff[arenaSize]; + Arena arena; + + ArenaFixture() { + ZeroMemory(buff, sizeof(buff)); + InitArena(&arena, buff, arenaSize); + } +}; + +struct TestStruct { + i32 a; + float b; + u64 c; +}; + +TEST_CASE_FIXTURE(ArenaFixture, "InitArena sets fields correctly") { + CHECK(arena.base == buff); + CHECK(arena.used == 0); + CHECK(arena.size == arenaSize); + CHECK(arena.totalUsed == 0); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush returns pointer to base on first alloc") { + u8* p = PushArray(&arena, u8, 8); + CHECK(p == buff); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush advances used by requested size") { + PushArray(&arena, u8, 16); + CHECK(arena.used == 16); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush returns non-overlapping pointers") { + u8* p1 = PushArray(&arena, u8, 32); + u8* p2 = PushArray(&arena, u8, 32); + CHECK(p2 == p1 + 32); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush accumulates totalUsed") { + PushArray(&arena, u8, 16); + PushArray(&arena, u8, 32); + CHECK(arena.totalUsed == 48); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush of size 1 works") { + u8* p = PushArray(&arena, u8, 1); + CHECK(p != nullptr); + CHECK(arena.used == 1); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush fills exactly to capacity") { + u8* p = PushArray(&arena, u8, arenaSize); + CHECK(p == buff); + CHECK(arena.used == arenaSize); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush multiple times up to capacity") { + u64 chunk = 64; + u64 count = arenaSize / chunk; + for (u64 i = 0; i < count; ++i) { + u8* p = PushArray(&arena, u8, chunk); + CHECK(p == buff + i * chunk); + } + + CHECK(arena.used == arenaSize); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPushZero returns zeroed memory") { + memset(buff, 0xCD, arenaSize); + for (i32 i = 0; i < arenaSize; ++i) { + REQUIRE(buff[i] == 0xCD); + } + + u8* p = PushArrayZero(&arena, u8, 32); + for (i32 i = 0; i < 32; ++i) { + CHECK(p[i] == 0); + } + + for (i32 i = 32; i < arenaSize; ++i) { + CHECK(p[i] == 0xCD); + } +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPushZero advances used correctly") { + PushArrayZero(&arena, u8, 64); + CHECK(arena.used == 64); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPop decreases used") { + PushArray(&arena, u8, 64); + ArenaPop(&arena, 32); + CHECK(arena.used == 32); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPop of full push returns to zero") { + PushArray(&arena, u8, 64); + ArenaPop(&arena, 64); + CHECK(arena.used == 0); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPop then push reuses memory") { + u8* p1 = PushArray(&arena, u8, 64); + ArenaPop(&arena, 64); + u8* p2 = PushArray(&arena, u8, 64); + CHECK(p1 == p2); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaPop does not affect totalUsed") { + PushArray(&arena, u8, 64); + u64 totalBefore = arena.totalUsed; + ArenaPop(&arena, 64); + CHECK(arena.totalUsed == totalBefore); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaClear resets used to zero") { + PushArray(&arena, u8, 128); + PushArray(&arena, u8, 64); + ArenaClear(&arena); + CHECK(arena.used == 0); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaClear does not reset totalUsed") { + PushArray(&arena, u8, 128); + u64 totalBefore = arena.totalUsed; + ArenaClear(&arena); + CHECK(arena.totalUsed == totalBefore); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaClear allows full reuse of bufffer") { + PushArray(&arena, u8, arenaSize); + ArenaClear(&arena); + u8* p = PushArray(&arena, u8, arenaSize); + CHECK(p == buff); + CHECK(arena.used == arenaSize); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaGetPos returns current used") { + PushArray(&arena, u8, 48); + CHECK(ArenaGetPos(&arena) == 48); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaSetPos restores position") { + PushArray(&arena, u8, 128); + u64 mark = ArenaGetPos(&arena); + PushArray(&arena, u8, 256); + ArenaSetPos(&arena, mark); + CHECK(arena.used == 128); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaSetPos then push reuses memory from that point") { + PushArray(&arena, u8, 128); + u64 mark = ArenaGetPos(&arena); + PushArray(&arena, u8, 64); + ArenaSetPos(&arena, mark); + u8* p = PushArray(&arena, u8, 64); + CHECK(p == buff + 128); +} + +TEST_CASE_FIXTURE(ArenaFixture, "ArenaSetPos to zero allows full reuse") { + PushArray(&arena, u8, arenaSize / 2); + ArenaSetPos(&arena, 0); + CHECK(arena.used == 0); + u8* p = PushArray(&arena, u8, arenaSize); + CHECK(p == buff); +} + +TEST_CASE_FIXTURE(ArenaFixture, "PushArray returns typed pointer and correct size") { + i32* arr = PushArray(&arena, i32, 4); + CHECK(arr != nullptr); + CHECK(arena.used == sizeof(i32) * 4); + arr[0] = 10; + arr[1] = 20; + arr[2] = 30; + arr[3] = 40; + CHECK(arr[0] == 10); + CHECK(arr[3] == 40); +} + +TEST_CASE_FIXTURE(ArenaFixture, "PushStruct returns pointer of correct size") { + TestStruct* s = PushStruct(&arena, TestStruct); + CHECK(s != nullptr); + CHECK(arena.used == sizeof(TestStruct)); + s->a = 1; + s->b = 2.0f; + s->c = 3; + CHECK(s->a == 1); + CHECK(s->b == doctest::Approx(2.0f)); + CHECK(s->c == 3); +} + +TEST_CASE_FIXTURE(ArenaFixture, "PushStructZero returns zeroed struct") { + memset(buff, 0xFF, arenaSize); + + TestStruct* s = PushStructZero(&arena, TestStruct); + CHECK(s->a == 0); + CHECK(s->b == doctest::Approx(0.0f)); + CHECK(s->c == 0); +} + +TEST_CASE_FIXTURE(ArenaFixture, "Scratch pattern: push pop leaves arena unchanged") { + PushArray(&arena, u8, 32); + u64 usedBefore = arena.used; + + char* tmp = PushArray(&arena, char, 64); + _snprintf_s(tmp, 64, _TRUNCATE, "temporary string"); + ArenaPop(&arena, 64); + + CHECK(arena.used == usedBefore); +} + +TEST_CASE_FIXTURE(ArenaFixture, "Scratch pattern: save restore via GetPos SetPos") { + PushArray(&arena, u8, 32); + u64 mark = ArenaGetPos(&arena); + + PushArray(&arena, char, 128); + PushArray(&arena, i32, 16); + + ArenaSetPos(&arena, mark); + CHECK(arena.used == 32); +} + +TEST_CASE_FIXTURE(ArenaFixture, "Scratch pattern: multiple scratch allocs then clear") { + for (i32 frame = 0; frame < 8; ++frame) { + PushArray(&arena, char, 64); + PushArray(&arena, TestStruct, 4); + PushArray(&arena, i32, 16); + ArenaClear(&arena); + CHECK(arena.used == 0); + } +} + +TEST_SUITE_END(); From f5d31ca859f27f4b9daf3222ffd983826cf64773 Mon Sep 17 00:00:00 2001 From: Juuso Piippo Date: Tue, 9 Jun 2026 03:15:13 +0300 Subject: [PATCH 3/5] Fix flaky test, add maxUsed to Arena Tests failed due to VirtualAlloc failing because it had a fixed address in dev builds --- src/arena.h | 5 +++++ src/compressor.h | 6 +++++- src/compressor_tests.cpp | 24 ++---------------------- src/win32_compressor.cpp | 17 +++++++++++------ 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/arena.h b/src/arena.h index fba5632..44b6554 100644 --- a/src/arena.h +++ b/src/arena.h @@ -7,6 +7,7 @@ struct Arena { // Debug u64 totalUsed; // Accumulated used + u64 maxUsed; // Updated at every push }; static void @@ -29,7 +30,11 @@ ArenaPush(Arena* arena, u64 size) { void* result = arena->base + arena->used; arena->used += size; + + // Debug arena->totalUsed += size; + arena->maxUsed = MAX(arena->used, arena->maxUsed); + return result; } diff --git a/src/compressor.h b/src/compressor.h index 9cbe3c7..0870a99 100644 --- a/src/compressor.h +++ b/src/compressor.h @@ -39,11 +39,15 @@ typedef wchar_t wchar; // clang-format on -#define KB(x) (x * 1024ULL) +#define KB(x) ((x) * 1024ULL) #define MB(x) (KB(x) * 1024ULL) #define GB(x) (MB(x) * 1024ULL) #define TB(x) (GB(x) * 1024ULL) +// The compiler warns about implicit conversion at /W4 at least +#define MIN(a, b) ((a) <= (b) ? (a) : (b)) +#define MAX(a, b) ((a) <= (b) ? (b) : (a)) + /** * Returns number of bytes, NOT the number of "real" characters */ diff --git a/src/compressor_tests.cpp b/src/compressor_tests.cpp index 2daa57c..5689178 100644 --- a/src/compressor_tests.cpp +++ b/src/compressor_tests.cpp @@ -45,7 +45,7 @@ struct AppStateFixture { static AppState staticAppState = {}; static bool32 initialized = false; if (!initialized) { - InitAppState(&staticAppState); + REQUIRE(InitAppState(&staticAppState)); initialized = true; } @@ -75,6 +75,7 @@ struct AppStateFixture { struct AddJobAppStateFixture : AppStateFixture { AddJobAppStateFixture() { // Other stuff? + REQUIRE(appState->outputFolder != nullptr); _snprintf_s(appState->outputFolder, MAX_PATH_COUNT, _TRUNCATE, "C:\\output"); } }; @@ -819,7 +820,6 @@ TEST_CASE_FIXTURE(ArenaFixture, "InitArena sets fields correctly") { CHECK(arena.base == buff); CHECK(arena.used == 0); CHECK(arena.size == arenaSize); - CHECK(arena.totalUsed == 0); } TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush returns pointer to base on first alloc") { @@ -838,12 +838,6 @@ TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush returns non-overlapping pointers") { CHECK(p2 == p1 + 32); } -TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush accumulates totalUsed") { - PushArray(&arena, u8, 16); - PushArray(&arena, u8, 32); - CHECK(arena.totalUsed == 48); -} - TEST_CASE_FIXTURE(ArenaFixture, "ArenaPush of size 1 works") { u8* p = PushArray(&arena, u8, 1); CHECK(p != nullptr); @@ -907,13 +901,6 @@ TEST_CASE_FIXTURE(ArenaFixture, "ArenaPop then push reuses memory") { CHECK(p1 == p2); } -TEST_CASE_FIXTURE(ArenaFixture, "ArenaPop does not affect totalUsed") { - PushArray(&arena, u8, 64); - u64 totalBefore = arena.totalUsed; - ArenaPop(&arena, 64); - CHECK(arena.totalUsed == totalBefore); -} - TEST_CASE_FIXTURE(ArenaFixture, "ArenaClear resets used to zero") { PushArray(&arena, u8, 128); PushArray(&arena, u8, 64); @@ -921,13 +908,6 @@ TEST_CASE_FIXTURE(ArenaFixture, "ArenaClear resets used to zero") { CHECK(arena.used == 0); } -TEST_CASE_FIXTURE(ArenaFixture, "ArenaClear does not reset totalUsed") { - PushArray(&arena, u8, 128); - u64 totalBefore = arena.totalUsed; - ArenaClear(&arena); - CHECK(arena.totalUsed == totalBefore); -} - TEST_CASE_FIXTURE(ArenaFixture, "ArenaClear allows full reuse of bufffer") { PushArray(&arena, u8, arenaSize); ArenaClear(&arena); diff --git a/src/win32_compressor.cpp b/src/win32_compressor.cpp index 3858e9e..a94dd12 100644 --- a/src/win32_compressor.cpp +++ b/src/win32_compressor.cpp @@ -1697,8 +1697,10 @@ OverwriteAndLoadNewConfig(HWND hWnd, AppState* appState) { static bool32 InitAppState(AppState* appState) { - // 64 bit -#if COMPRESSOR_DEV +#if COMPRESSOR_DEV && !COMPRESSOR_TESTS + // This caused some problems if tests were ran too quickly sequentially + // I think VirtualAlloc failed, Windows couldn't clean the address up quickly enough I guess + // 64 bit... LPVOID baseAddress = reinterpret_cast(TB(2)); #else LPVOID baseAddress = nullptr; @@ -1820,17 +1822,20 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) Arena* scratch = &appState->scratchArena; f32 toMB = 1 / (1024.0f * 1024.0f); - f32 scratchUsedMB = scratch->used * toMB; - f32 scratchTotalUsedMB = scratch->totalUsed * toMB; f32 permUsedMB = permanent->used * toMB; f32 permTotalUsedMB = permanent->totalUsed * toMB; + f32 scratchUsedMB = scratch->used * toMB; + f32 scratchTotalUsedMB = scratch->totalUsed * toMB; + f32 scratchMaxUsedMB = scratch->maxUsed * toMB; + ImGui::TextDisabled("Permanent size: %llu, used: %llu (%.2f MB), total used: %llu (%.2f MB)", permanent->size, permanent->used, permUsedMB, permanent->totalUsed, permTotalUsedMB); - ImGui::TextDisabled("Scratch size: %llu, used: %llu (%.2f MB), total used: %llu (%.2f MB)", + ImGui::TextDisabled("Scratch size: %llu, used: %llu (%.2f MB), total used: %llu (%.2f MB), max " + "used: %llu (%.2f MB)", scratch->size, scratch->used, scratchUsedMB, scratch->totalUsed, - scratchTotalUsedMB); + scratchTotalUsedMB, scratch->maxUsed, scratchMaxUsedMB); # endif const f32 sliderWidth = 190 * scale; From 1d194a8591f61a9282079bc77e4ff3e14516767e Mon Sep 17 00:00:00 2001 From: Juuso Piippo Date: Wed, 10 Jun 2026 19:58:02 +0300 Subject: [PATCH 4/5] Arena freed diagnostics and some renaming --- src/arena.h | 16 ++++++++++++---- src/compressor_tests.cpp | 16 ++++++++-------- src/win32_compressor.cpp | 27 +++++++++++++++++---------- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/arena.h b/src/arena.h index 44b6554..2fa117e 100644 --- a/src/arena.h +++ b/src/arena.h @@ -8,15 +8,16 @@ struct Arena { // Debug u64 totalUsed; // Accumulated used u64 maxUsed; // Updated at every push + u64 totalFreed; + u64 maxFreed; }; static void -InitArena(Arena* arena, void* base, u64 size) { +ArenaInit(Arena* arena, void* base, u64 size) { + *arena = {}; + arena->base = static_cast(base); - arena->used = 0; arena->size = size; - - arena->totalUsed = 0; } static void* @@ -48,11 +49,18 @@ ArenaPushZero(Arena* arena, u64 size) { static void ArenaPop(Arena* arena, u64 size) { ASSERT(arena->used >= size); + + arena->totalFreed += size; + arena->maxFreed = MAX(size, arena->maxUsed); + arena->used -= size; } static void ArenaClear(Arena* arena) { + arena->totalFreed += arena->used; + arena->maxFreed = MAX(arena->used, arena->maxUsed); + arena->used = 0; } diff --git a/src/compressor_tests.cpp b/src/compressor_tests.cpp index 5689178..09ec458 100644 --- a/src/compressor_tests.cpp +++ b/src/compressor_tests.cpp @@ -22,7 +22,7 @@ CANDIDATES TO TEST: // Now that we use a fixed permanent storage, we have to clear manually static void -ResetAppState(AppState* appState) { +AppStateReset(AppState* appState) { appState->jobCount = 0; for (i32 i = 0; i < ARR_COUNT(appState->jobs); ++i) { appState->jobs[i].input[0] = '\0'; @@ -52,7 +52,7 @@ struct AppStateFixture { appState = &staticAppState; } - ~AppStateFixture() { ResetAppState(appState); } + ~AppStateFixture() { AppStateReset(appState); } bool32 IsZeroed() { @@ -763,7 +763,7 @@ TEST_CASE_FIXTURE(LoadConfigFileFixture, "LoadConfigFile applies fallbacks corre //appState = {}; //ZeroMemory(appState, sizeof(appState)); // Manual reset as now we use arenas with fixed permanent storage - ResetAppState(appState); + AppStateReset(appState); this->WriteConfig("[Sizes]\n2.0\n5.0 !\n[Codecs]\nh265 !\n[OutputPath]\nC:\\EasyCompTemp"); REQUIRE(LoadConfigFile(nullptr, appState, path)); CHECK(appState->defaultTargetSize == doctest::Approx(5.0f)); @@ -771,19 +771,19 @@ TEST_CASE_FIXTURE(LoadConfigFileFixture, "LoadConfigFile applies fallbacks corre CHECK(StrEqual(appState->outputFolder, "C:\\EasyCompTemp")); this->CleanupCreatedOutputFolder(); - ResetAppState(appState); + AppStateReset(appState); this->WriteConfig("[Sizes]\n7.5\n 6.42\n"); REQUIRE(LoadConfigFile(nullptr, appState, path)); CHECK(appState->defaultTargetSize == doctest::Approx(7.5f)); this->CleanupCreatedOutputFolder(); - ResetAppState(appState); + AppStateReset(appState); this->WriteConfig("[Sizes]\n2.0\n5.0 !\n4.0\n"); REQUIRE(LoadConfigFile(nullptr, appState, path)); CHECK(appState->defaultTargetSize == doctest::Approx(5.0f)); this->CleanupCreatedOutputFolder(); - ResetAppState(appState); + AppStateReset(appState); this->WriteConfig("[Sizes]\n2.\n[Codecs]"); REQUIRE(LoadConfigFile(nullptr, appState, path)); CHECK(appState->defaultTargetSize == doctest::Approx(2.0f)); @@ -806,7 +806,7 @@ struct ArenaFixture { ArenaFixture() { ZeroMemory(buff, sizeof(buff)); - InitArena(&arena, buff, arenaSize); + ArenaInit(&arena, buff, arenaSize); } }; @@ -816,7 +816,7 @@ struct TestStruct { u64 c; }; -TEST_CASE_FIXTURE(ArenaFixture, "InitArena sets fields correctly") { +TEST_CASE_FIXTURE(ArenaFixture, "ArenaInit sets fields correctly") { CHECK(arena.base == buff); CHECK(arena.used == 0); CHECK(arena.size == arenaSize); diff --git a/src/win32_compressor.cpp b/src/win32_compressor.cpp index a94dd12..6099cf5 100644 --- a/src/win32_compressor.cpp +++ b/src/win32_compressor.cpp @@ -1726,7 +1726,7 @@ InitAppState(AppState* appState) { // Assign permanent storage locations { Arena* permanent = &appState->permanentArena; - InitArena(permanent, appState->permanentMemory, appState->permanentMemorySize); + ArenaInit(permanent, appState->permanentMemory, appState->permanentMemorySize); DEBUG_PRINTF("Permanent arena base: %llu, size: %llu\n", appState->permanentMemory, appState->permanentMemorySize); @@ -1749,7 +1749,7 @@ InitAppState(AppState* appState) { } Arena* scratch = &appState->scratchArena; - InitArena(scratch, appState->scratchMemory, appState->scratchMemorySize); + ArenaInit(scratch, appState->scratchMemory, appState->scratchMemorySize); DEBUG_PRINTF("Scratch arena base: %llu, size: %llu\n", appState->scratchMemory, appState->scratchMemorySize); @@ -1824,18 +1824,25 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) f32 toMB = 1 / (1024.0f * 1024.0f); f32 permUsedMB = permanent->used * toMB; f32 permTotalUsedMB = permanent->totalUsed * toMB; + f32 permSizeMB = permanent->size * toMB; + f32 scratchSizeMB = scratch->size * toMB; f32 scratchUsedMB = scratch->used * toMB; f32 scratchTotalUsedMB = scratch->totalUsed * toMB; f32 scratchMaxUsedMB = scratch->maxUsed * toMB; - - ImGui::TextDisabled("Permanent size: %llu, used: %llu (%.2f MB), total used: %llu (%.2f MB)", - permanent->size, permanent->used, permUsedMB, permanent->totalUsed, - permTotalUsedMB); - ImGui::TextDisabled("Scratch size: %llu, used: %llu (%.2f MB), total used: %llu (%.2f MB), max " - "used: %llu (%.2f MB)", - scratch->size, scratch->used, scratchUsedMB, scratch->totalUsed, - scratchTotalUsedMB, scratch->maxUsed, scratchMaxUsedMB); + f32 scratchTotalFreedMB = scratch->totalFreed * toMB; + f32 scratchMaxFreedMB = scratch->maxFreed * toMB; + + ImGui::TextDisabled( + "Permanent size: %llu (%.2f MB), used: %llu (%.2f MB), total used: %llu (%.2f MB)", + permanent->size, permSizeMB, permanent->used, permUsedMB, permanent->totalUsed, + permTotalUsedMB); + ImGui::TextDisabled( + "Scratch size: %llu (%.2f MB), used: %llu (%.2f MB), total used: %llu (%.2f MB), max " + "used: %llu (%.2f MB), total freed: %llu (%.2f MB), max freed: %llu (%.2f MB)", + scratch->size, scratchSizeMB, scratch->used, scratchUsedMB, scratch->totalUsed, + scratchTotalUsedMB, scratch->maxUsed, scratchMaxUsedMB, scratch->totalFreed, + scratchTotalFreedMB, scratch->maxFreed, scratchMaxFreedMB); # endif const f32 sliderWidth = 190 * scale; From d18abe266e32dbd269d5ee55e8cc509a9395bf30 Mon Sep 17 00:00:00 2001 From: Juuso Piippo Date: Wed, 10 Jun 2026 23:30:59 +0300 Subject: [PATCH 5/5] More arena diagnostics and seperate to own struct --- src/arena.h | 47 ++++++++++++++++++++++++++---------- src/win32_compressor.cpp | 51 +++++++++++++++++++++++++++++----------- 2 files changed, 72 insertions(+), 26 deletions(-) diff --git a/src/arena.h b/src/arena.h index 2fa117e..8695e44 100644 --- a/src/arena.h +++ b/src/arena.h @@ -1,15 +1,27 @@ #pragma once +// Diagnostics for debugging +struct ArenaDiagnostics { + u64 totalUsed; // Accumulated used + u64 maxUsed; // Updated at every push + u64 totalFreed; + u64 maxFreed; + + // Counts + u64 initCount; + u64 pushCount; // Includes pushZeroCount as well + u64 pushZeroCount; + u64 popCount; + u64 clearCount; + u64 setPosCount; +}; + struct Arena { u8* base; u64 used; u64 size; - // Debug - u64 totalUsed; // Accumulated used - u64 maxUsed; // Updated at every push - u64 totalFreed; - u64 maxFreed; + ArenaDiagnostics diagn; }; static void @@ -18,6 +30,8 @@ ArenaInit(Arena* arena, void* base, u64 size) { arena->base = static_cast(base); arena->size = size; + + ++arena->diagn.initCount; } static void* @@ -33,14 +47,18 @@ ArenaPush(Arena* arena, u64 size) { arena->used += size; // Debug - arena->totalUsed += size; - arena->maxUsed = MAX(arena->used, arena->maxUsed); + arena->diagn.totalUsed += size; + arena->diagn.maxUsed = MAX(arena->used, arena->diagn.maxUsed); + ++arena->diagn.pushCount; + ASSERT(arena->diagn.pushCount >= arena->diagn.pushZeroCount); return result; } static void* ArenaPushZero(Arena* arena, u64 size) { + ++arena->diagn.pushZeroCount; + void* result = ArenaPush(arena, size); ZeroMemory(result, size); return result; @@ -50,16 +68,18 @@ static void ArenaPop(Arena* arena, u64 size) { ASSERT(arena->used >= size); - arena->totalFreed += size; - arena->maxFreed = MAX(size, arena->maxUsed); + arena->diagn.totalFreed += size; + arena->diagn.maxFreed = MAX(size, arena->diagn.maxFreed); + ++arena->diagn.popCount; arena->used -= size; } static void ArenaClear(Arena* arena) { - arena->totalFreed += arena->used; - arena->maxFreed = MAX(arena->used, arena->maxUsed); + arena->diagn.totalFreed += arena->used; + arena->diagn.maxFreed = MAX(arena->used, arena->diagn.maxFreed); + ++arena->diagn.clearCount; arena->used = 0; } @@ -72,8 +92,11 @@ ArenaGetPos(Arena* arena) { static void ArenaSetPos(Arena* arena, u64 pos) { // It would be a programmer error to set equal to size - ASSERT(pos >= 0 && pos < arena->size); + ASSERT( //pos >= 0 && // unsigned... + pos < arena->size); arena->used = pos; + + ++arena->diagn.setPosCount; } #define PushArray(arena, type, count) (type*)ArenaPush((arena), sizeof(type) * (count)) diff --git a/src/win32_compressor.cpp b/src/win32_compressor.cpp index 6099cf5..01e1a2f 100644 --- a/src/win32_compressor.cpp +++ b/src/win32_compressor.cpp @@ -1818,31 +1818,54 @@ DrawUi(AppState* appState, HINSTANCE hInstance, HWND hWnd, f32 scale, f32 delta) # if COMPRESSOR_DEV ImGui::TextDisabled("DEV: compressing: %d, cancelled: %d", compressing, cancelled); - Arena* permanent = &appState->permanentArena; + + /// Arena diagnostics + + Arena* perm = &appState->permanentArena; Arena* scratch = &appState->scratchArena; + auto* permDiagn = &perm->diagn; + auto* scratchDiagn = &scratch->diagn; + f32 toMB = 1 / (1024.0f * 1024.0f); - f32 permUsedMB = permanent->used * toMB; - f32 permTotalUsedMB = permanent->totalUsed * toMB; - f32 permSizeMB = permanent->size * toMB; + f32 permSizeMB = perm->size * toMB; + f32 permUsedMB = perm->used * toMB; + f32 permTotalUsedMB = permDiagn->totalUsed * toMB; + f32 permMaxUsedMB = permDiagn->maxUsed * toMB; + f32 permTotalFreedMB = permDiagn->totalFreed * toMB; + f32 permMaxFreedMB = permDiagn->maxFreed * toMB; f32 scratchSizeMB = scratch->size * toMB; f32 scratchUsedMB = scratch->used * toMB; - f32 scratchTotalUsedMB = scratch->totalUsed * toMB; - f32 scratchMaxUsedMB = scratch->maxUsed * toMB; - f32 scratchTotalFreedMB = scratch->totalFreed * toMB; - f32 scratchMaxFreedMB = scratch->maxFreed * toMB; + f32 scratchTotalUsedMB = scratchDiagn->totalUsed * toMB; + f32 scratchMaxUsedMB = scratchDiagn->maxUsed * toMB; + f32 scratchTotalFreedMB = scratchDiagn->totalFreed * toMB; + f32 scratchMaxFreedMB = scratchDiagn->maxFreed * toMB; ImGui::TextDisabled( - "Permanent size: %llu (%.2f MB), used: %llu (%.2f MB), total used: %llu (%.2f MB)", - permanent->size, permSizeMB, permanent->used, permUsedMB, permanent->totalUsed, - permTotalUsedMB); + "Permanent size: %llu (%.2f MB), used: %llu (%.2f MB), total used: %llu (%.2f MB), max " + "used: %llu (%.2f MB), total freed: %llu (%.2f MB), max freed: %llu (%.2f MB)", + perm->size, permSizeMB, perm->used, permUsedMB, permDiagn->totalUsed, permTotalUsedMB, + permDiagn->maxUsed, permMaxUsedMB, permDiagn->totalFreed, permTotalFreedMB, + permDiagn->maxFreed, permMaxFreedMB); + ImGui::TextDisabled("Permanent counts: init: %llu, push: %llu, pushZero: %llu, pop: %llu, " + "clear: %llu, setPos: %llu", + permDiagn->initCount, permDiagn->pushCount, permDiagn->pushZeroCount, + permDiagn->popCount, permDiagn->clearCount, permDiagn->setPosCount); + + // Because the scratch arena is essentially a per-frame arena which is cleared at the start of + // every frame, the used, totalFreed, maxFreed and clearCount (more?) are not good metrics here ImGui::TextDisabled( "Scratch size: %llu (%.2f MB), used: %llu (%.2f MB), total used: %llu (%.2f MB), max " "used: %llu (%.2f MB), total freed: %llu (%.2f MB), max freed: %llu (%.2f MB)", - scratch->size, scratchSizeMB, scratch->used, scratchUsedMB, scratch->totalUsed, - scratchTotalUsedMB, scratch->maxUsed, scratchMaxUsedMB, scratch->totalFreed, - scratchTotalFreedMB, scratch->maxFreed, scratchMaxFreedMB); + scratch->size, scratchSizeMB, scratch->used, scratchUsedMB, scratchDiagn->totalUsed, + scratchTotalUsedMB, scratchDiagn->maxUsed, scratchMaxUsedMB, scratchDiagn->totalFreed, + scratchTotalFreedMB, scratchDiagn->maxFreed, scratchMaxFreedMB); + ImGui::TextDisabled("Scratch counts: init: %llu, push: %llu, pushZero: %llu, pop: %llu, clear: " + "%llu, setPos: %llu", + scratchDiagn->initCount, scratchDiagn->pushCount, + scratchDiagn->pushZeroCount, scratchDiagn->popCount, + scratchDiagn->clearCount, scratchDiagn->setPosCount); # endif const f32 sliderWidth = 190 * scale;