From 64e96d1b6f09c0bd4cfb8b5819e018c28ebe090a Mon Sep 17 00:00:00 2001 From: Ghabry Date: Tue, 5 Dec 2023 18:18:36 +0100 Subject: [PATCH] Emscripten: Reimplement Start and Stop without emscripten_sleep - emscripten_sleep requires asynchify. - Final fixes for save and load scene. - New function for taking a screenshot and downloading it. - Fix unit tests --- CMakeLists.txt | 13 +++++---- resources/emscripten/emscripten-post.js | 7 ++--- resources/emscripten/emscripten-pre.js | 2 +- src/bitmap.cpp | 2 +- src/bitmap.h | 2 +- src/image_png.cpp | 3 +- src/image_png.h | 2 +- src/output.cpp | 8 +++++- src/output.h | 2 +- src/platform/emscripten/interface.cpp | 24 ++++++++++++---- src/platform/emscripten/interface.h | 1 + src/platform/emscripten/main.cpp | 38 ++++++++++++++++++------- src/player.cpp | 7 ++--- src/scene_file.cpp | 2 +- src/window_savefile.cpp | 6 +++- src/window_savefile.h | 7 ++++- 16 files changed, 85 insertions(+), 41 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 347735a1cc..89c2404311 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1124,7 +1124,11 @@ if(${PLAYER_BUILD_EXECUTABLE} AND ${PLAYER_TARGET_PLATFORM} MATCHES "^SDL(1|2)$" set(PLAYER_JS_POSTJS "${CMAKE_CURRENT_SOURCE_DIR}/resources/emscripten/emscripten-post.js") set(PLAYER_JS_SHELL "${CMAKE_CURRENT_SOURCE_DIR}/resources/emscripten/emscripten-shell.html") - set_property(TARGET ${EXE_NAME} PROPERTY LINK_FLAGS "-s ALLOW_MEMORY_GROWTH -s MINIFY_HTML=0 -s MODULARIZE -s EXIT_RUNTIME -s EXPORT_NAME='createEasyRpgPlayer' --bind --pre-js ${PLAYER_JS_PREJS} --post-js ${PLAYER_JS_POSTJS} -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=['$autoResumeAudioContext','$dynCall']") + set_property(TARGET ${EXE_NAME} PROPERTY LINK_FLAGS + "-sALLOW_MEMORY_GROWTH -sMINIFY_HTML=0 -sMODULARIZE -sEXPORT_NAME=createEasyRpgPlayer \ + -sEXIT_RUNTIME --bind --pre-js ${PLAYER_JS_PREJS} --post-js ${PLAYER_JS_POSTJS} \ + -sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=['$autoResumeAudioContext','$dynCall'] \ + -sEXPORTED_FUNCTIONS=_main,_malloc,_free") set_source_files_properties("src/platform/sdl/main.cpp" PROPERTIES OBJECT_DEPENDS "${PLAYER_JS_PREJS};${PLAYER_JS_POSTJS};${PLAYER_JS_SHELL}") @@ -1420,7 +1424,10 @@ if(PLAYER_ENABLE_TESTS) if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten") target_compile_definitions(test_runner_player PUBLIC EP_NATIVE_TEST_PATH=\"${CMAKE_CURRENT_SOURCE_DIR}/tests/assets\") target_compile_definitions(test_runner_player PUBLIC EP_TEST_PATH=\"/assets\") + target_compile_definitions(test_runner_player PUBLIC DOCTEST_CONFIG_NO_EXCEPTIONS_BUT_WITH_ALL_ASSERTS=1) + target_link_libraries(test_runner_player "nodefs.js") + set_property(TARGET test_runner_player PROPERTY LINK_FLAGS "-sALLOW_MEMORY_GROWTH --bind") else() target_compile_definitions(test_runner_player PUBLIC EP_TEST_PATH=\"${CMAKE_CURRENT_SOURCE_DIR}/tests/assets\") endif() @@ -1434,10 +1441,6 @@ if(PLAYER_ENABLE_TESTS) endif() add_dependencies(check_player test_runner_player) add_dependencies(check check_player) - - if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten") - set_property(TARGET test_runner_player APPEND_STRING PROPERTY LINK_FLAGS "-s ALLOW_MEMORY_GROWTH") - endif() endif() # Instrumentation framework diff --git a/resources/emscripten/emscripten-post.js b/resources/emscripten/emscripten-post.js index 53a90053d7..7277533b83 100644 --- a/resources/emscripten/emscripten-post.js +++ b/resources/emscripten/emscripten-post.js @@ -14,14 +14,11 @@ if (Module.saveFs === undefined) { } Module.initApi = function() { - Module.api_private.downloadSavegame_js = function(buffer, size, index) { + Module.api_private.download_js = function(buffer, size, filename) { const blob = new Blob([Module.HEAPU8.slice(buffer, buffer + size)]); const link = document.createElement('a'); link.href = window.URL.createObjectURL(blob); - if (index < 10) { - index = "0" + index; - } - link.download = "Save" + index + ".lsd"; + link.download = UTF8ToString(filename); link.click(); link.remove(); } diff --git a/resources/emscripten/emscripten-pre.js b/resources/emscripten/emscripten-pre.js index f27a0e4854..6467c5a550 100644 --- a/resources/emscripten/emscripten-pre.js +++ b/resources/emscripten/emscripten-pre.js @@ -42,7 +42,7 @@ Module = { ...Module, Module.totalDependencies = Math.max(Module.totalDependencies, left); Module.setStatus(left ? `Preparing... (${Module.totalDependencies - left}/${Module.totalDependencies})` : 'Downloading game data...'); } -}); +}; /** * Parses the current location query to setup a specific game diff --git a/src/bitmap.cpp b/src/bitmap.cpp index 9cc54864e0..25992d6366 100644 --- a/src/bitmap.cpp +++ b/src/bitmap.cpp @@ -173,7 +173,7 @@ Bitmap::Bitmap(Bitmap const& source, Rect const& src_rect, bool transparent) { Blit(0, 0, source, src_rect, Opacity::Opaque()); } -bool Bitmap::WritePNG(Filesystem_Stream::OutputStream& os) const { +bool Bitmap::WritePNG(std::ostream& os) const { size_t const width = GetWidth(), height = GetHeight(); size_t const stride = width * 4; diff --git a/src/bitmap.h b/src/bitmap.h index ee7a77fddc..3fbfc699d4 100644 --- a/src/bitmap.h +++ b/src/bitmap.h @@ -198,7 +198,7 @@ class Bitmap { * @param os output stream that PNG will be output. * @return true if success, otherwise false. */ - bool WritePNG(Filesystem_Stream::OutputStream&) const; + bool WritePNG(std::ostream& os) const; /** * Gets the background color diff --git a/src/image_png.cpp b/src/image_png.cpp index d301ec9a80..1862416616 100644 --- a/src/image_png.cpp +++ b/src/image_png.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include "output.h" #include "image_png.h" @@ -250,7 +251,7 @@ static void flush_stream(png_structp out_ptr) { reinterpret_cast(png_get_io_ptr(out_ptr))->flush(); } -bool ImagePNG::WritePNG(Filesystem_Stream::OutputStream& os, uint32_t width, uint32_t height, uint32_t* data) { +bool ImagePNG::WritePNG(std::ostream& os, uint32_t width, uint32_t height, uint32_t* data) { png_structp write = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL); if (!write) { Output::Warning("Bitmap::WritePNG: error in png_create_write"); diff --git a/src/image_png.h b/src/image_png.h index 18352e3a1f..f81dd9496c 100644 --- a/src/image_png.h +++ b/src/image_png.h @@ -24,7 +24,7 @@ namespace ImagePNG { bool ReadPNG(const void* buffer, bool transparent, int& width, int& height, void*& pixels); bool ReadPNG(Filesystem_Stream::InputStream& is, bool transparent, int& width, int& height, void*& pixels); - bool WritePNG(Filesystem_Stream::OutputStream& os, uint32_t width, uint32_t height, uint32_t* data); + bool WritePNG(std::ostream& os, uint32_t width, uint32_t height, uint32_t* data); } #endif diff --git a/src/output.cpp b/src/output.cpp index 7b7f692ddd..62606ede50 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -28,6 +28,7 @@ # include #elif defined(EMSCRIPTEN) # include +# include "platform/emscripten/interface.h" #elif defined(__vita__) # include #endif @@ -275,12 +276,17 @@ void Output::Quit() { } bool Output::TakeScreenshot() { +#ifdef EMSCRIPTEN + Emscripten_Interface::TakeScreenshot(); + return true; +#else int index = 0; std::string p; do { p = "screenshot_" + std::to_string(index++) + ".png"; } while(FileFinder::Save().Exists(p)); return TakeScreenshot(p); +#endif } bool Output::TakeScreenshot(StringView file) { @@ -293,7 +299,7 @@ bool Output::TakeScreenshot(StringView file) { return false; } -bool Output::TakeScreenshot(Filesystem_Stream::OutputStream& os) { +bool Output::TakeScreenshot(std::ostream& os) { return DisplayUi->GetDisplaySurface()->WritePNG(os); } diff --git a/src/output.h b/src/output.h index 78ff3c0cad..fe69144346 100644 --- a/src/output.h +++ b/src/output.h @@ -78,7 +78,7 @@ namespace Output { * @param os output stream that PNG will be stored. * @return true if success, otherwise false. */ - bool TakeScreenshot(Filesystem_Stream::OutputStream& os); + bool TakeScreenshot(std::ostream& os); /** * Shows/Hides the output log overlay. diff --git a/src/platform/emscripten/interface.cpp b/src/platform/emscripten/interface.cpp index ec9a154efe..10524d6d5f 100644 --- a/src/platform/emscripten/interface.cpp +++ b/src/platform/emscripten/interface.cpp @@ -20,7 +20,9 @@ #include #include #include +#include +#include "async_handler.h" #include "filefinder.h" #include "player.h" #include "scene_save.h" @@ -38,9 +40,10 @@ bool Emscripten_Interface::DownloadSavegame(int slot) { return false; } auto save_buffer = Utils::ReadStream(is); + std::string filename = std::get<1>(FileFinder::GetPathAndFilename(name)); EM_ASM_ARGS({ - Module.api_private.downloadSavegame_js($0, $1, $2); - }, save_buffer.data(), save_buffer.size(), slot); + Module.api_private.download_js($0, $1, $2); + }, save_buffer.data(), save_buffer.size(), filename.c_str()); return true; } @@ -54,6 +57,17 @@ void Emscripten_Interface::RefreshScene() { Scene::instance->Refresh(); } +void Emscripten_Interface::TakeScreenshot() { + static int index = 0; + std::ostringstream os; + Output::TakeScreenshot(os); + std::string screenshot = os.str(); + std::string filename = "screenshot_" + std::to_string(index++) + ".png"; + EM_ASM_ARGS({ + Module.api_private.download_js($0, $1, $2); + }, screenshot.data(), screenshot.size(), filename.c_str()); +} + bool Emscripten_Interface_Private::UploadSavegameStep2(int slot, int buffer_addr, int size) { auto fs = FileFinder::Save(); std::string name = Scene_Save::GetSaveFilename(fs, slot); @@ -72,10 +86,7 @@ bool Emscripten_Interface_Private::UploadSavegameStep2(int slot, int buffer_addr os.write(reinterpret_cast(buffer_addr), size); } - // Save changed file system - EM_ASM({ - FS.syncfs(function(err) {}); - }); + AsyncHandler::SaveFilesystem(); return true; } @@ -87,6 +98,7 @@ EMSCRIPTEN_BINDINGS(player_interface) { .class_function("downloadSavegame", &Emscripten_Interface::DownloadSavegame) .class_function("uploadSavegame", &Emscripten_Interface::UploadSavegame) .class_function("refreshScene", &Emscripten_Interface::RefreshScene) + .class_function("takeScreenshot", &Emscripten_Interface::TakeScreenshot) ; emscripten::class_("api_private") diff --git a/src/platform/emscripten/interface.h b/src/platform/emscripten/interface.h index 11140a7bc7..b84c9b14f9 100644 --- a/src/platform/emscripten/interface.h +++ b/src/platform/emscripten/interface.h @@ -23,6 +23,7 @@ class Emscripten_Interface { static bool DownloadSavegame(int slot); static void UploadSavegame(int slot); static void RefreshScene(); + static void TakeScreenshot(); static void Reset(); }; diff --git a/src/platform/emscripten/main.cpp b/src/platform/emscripten/main.cpp index 9d4571ef00..2792061078 100644 --- a/src/platform/emscripten/main.cpp +++ b/src/platform/emscripten/main.cpp @@ -19,28 +19,46 @@ #include #include #include +#include "baseui.h" #include "output.h" #include "player.h" +namespace { + std::vector args; + int counter = 0; +} + +void main_loop() { + if (counter < 5) { + ++counter; + } + + if (counter == 5) { + // Yield on start to ensure async operations (e.g. "mounting" of filesystems) can finish + Player::Init(std::move(args)); + Player::Run(); + ++counter; + } else if (counter == 6) { + Player::MainLoop(); + if (!DisplayUi.get()) { + // Yield on shutdown to ensure async operations (e.g. IDBFS saving) can finish + counter = -5; + } + } else if (counter == -1) { + emscripten_cancel_main_loop(); + } +} + /** * If the main function ever needs to change, be sure to update the `main()` * functions of the other platforms as well. */ extern "C" int main(int argc, char* argv[]) { - // Yield on start to ensure async operations (e.g. "mounting" of filesystems) can finish - // 10ms appears to work already but to be on the safe side better use 100ms - emscripten_sleep(100); - - std::vector args; args.assign(argv, argv + argc); Output::IgnorePause(true); - Player::Init(std::move(args)); - Player::Run(); - - // Yield on shutdown to ensure async operations (e.g. IDBFS saving) can finish - emscripten_sleep(100); + emscripten_set_main_loop(main_loop, 0, 0); return EXIT_SUCCESS; } diff --git a/src/player.cpp b/src/player.cpp index 0a9d5786b0..2279b02d49 100644 --- a/src/player.cpp +++ b/src/player.cpp @@ -214,9 +214,8 @@ void Player::Run() { Game_Clock::ResetFrame(Game_Clock::now()); // Main loop -#ifdef EMSCRIPTEN - emscripten_set_main_loop(Player::MainLoop, 0, 0); -#elif defined(USE_LIBRETRO) +#if defined(USE_LIBRETRO) || defined(EMSCRIPTEN) + // emscripten implemented in main.cpp // libretro invokes the MainLoop through a retro_run-callback #else while (Transition::instance().IsActive() || (Scene::instance && Scene::instance->type != Scene::Null)) { @@ -394,8 +393,6 @@ void Player::Exit() { Graphics::UpdateSceneCallback(); #ifdef EMSCRIPTEN - emscripten_cancel_main_loop(); - BitmapRef surface = DisplayUi->GetDisplaySurface(); std::string message = "It's now safe to turn off\n your browser."; DisplayUi->CleanDisplay(); diff --git a/src/scene_file.cpp b/src/scene_file.cpp index da916f6e16..4e23f64c9c 100644 --- a/src/scene_file.cpp +++ b/src/scene_file.cpp @@ -204,7 +204,7 @@ void Scene_File::vUpdate() { #ifdef EMSCRIPTEN extra_commands_window->SetX(SCREEN_TARGET_WIDTH - extra_commands_window->GetWidth() - 8); extra_commands_window->SetY(file_windows[index]->GetY() + 8); - extra_commands_window->SetItemEnabled(0, file_windows[index]->IsValid()); + extra_commands_window->SetItemEnabled(0, file_windows[index]->IsValid() && file_windows[index]->HasParty()); extra_commands_window->SetVisible(true); return; #endif diff --git a/src/window_savefile.cpp b/src/window_savefile.cpp index f531120ba3..bf858ab1e9 100644 --- a/src/window_savefile.cpp +++ b/src/window_savefile.cpp @@ -82,10 +82,14 @@ void Window_SaveFile::SetCorrupted(bool corrupted) { this->corrupted = corrupted; } -bool Window_SaveFile::IsValid() { +bool Window_SaveFile::IsValid() const { return has_save && !corrupted; } +bool Window_SaveFile::HasParty() const { + return has_party; +} + void Window_SaveFile::SetHasSave(bool valid) { this->has_save = valid; } diff --git a/src/window_savefile.h b/src/window_savefile.h index 17c60672cd..e48093405d 100644 --- a/src/window_savefile.h +++ b/src/window_savefile.h @@ -67,7 +67,12 @@ class Window_SaveFile : public Window_Base { * * @return Whether save is valid */ - bool IsValid(); + bool IsValid() const; + + /** + * @return Whether the save slot contains party information from a save file. + */ + bool HasParty() const; /** * Sets if there is a savegame in the slot.