From 321bb7e057f11c0d88d25b5dbb89910fbc1ca778 Mon Sep 17 00:00:00 2001 From: Bradley Arsenault Date: Wed, 9 Sep 2026 17:20:08 -0400 Subject: [PATCH 1/2] Retain selected artwork across menus and matches --- libgag/include/SDLGraphicContext.h | 10 +- libgag/src/SConscript | 3 + libgag/src/Sprite.cpp | 172 +++++++++++++++------- libgag/src/Toolkit.cpp | 2 + src/GlobalContainer.cpp | 1 + src/SettingsScreenGeneral.cpp | 2 +- src/gui/GameGUI.cpp | 2 - src/map/edit/MapEditCtor.cpp | 3 - test/ArtworkPackLifecycleHarness.cpp | 155 +++++++++++++++++++ test/HighResolutionIntegrationHarness.cpp | 145 +++++++++++++++++- test/README.md | 16 ++ 11 files changed, 452 insertions(+), 59 deletions(-) create mode 100644 test/ArtworkPackLifecycleHarness.cpp diff --git a/libgag/include/SDLGraphicContext.h b/libgag/include/SDLGraphicContext.h index 92b256423..29ef38e80 100644 --- a/libgag/include/SDLGraphicContext.h +++ b/libgag/include/SDLGraphicContext.h @@ -504,6 +504,8 @@ namespace GAGCore }; friend class GraphicContext; + friend class Toolkit; + static void resetHighResolutionState(); std::string fileName; //#define DEBUG_SPRITE_NOT_DRAWN @@ -544,9 +546,15 @@ namespace GAGCore public: //! Opt into batching variable-size frames; callers must finishDrawingSprite. bool createTextureAtlas(bool allowVariableSizes = false); - struct HighResolutionStats {size_t cpuBytes=0, coloredFrames=0;}; + struct HighResolutionStats + { + size_t cpuBytes = 0, coloredFrames = 0; + // Cumulative load work for this Toolkit session, including failed pack reads. + size_t manifestParses = 0, imageLoads = 0, packReloads = 0; + }; static HighResolutionStats highResolutionStats(); static void setHighResolution(bool enabled); + static void reloadHighResolutionPack(); static void flushBatches(GraphicContext *gc); //! Constructor Sprite() : fileName("not loaded yet") { } diff --git a/libgag/src/SConscript b/libgag/src/SConscript index 67cfb1344..8c7aa8d10 100644 --- a/libgag/src/SConscript +++ b/libgag/src/SConscript @@ -33,6 +33,9 @@ if not env['server']: aspect_test = aspect_env.Program('FullscreenAspectHarness', [aspect_env.Object('FullscreenAspectHarness.o', '#test/FullscreenAspectHarness.cpp'), l1]) env.Alias('aspect-test', aspect_test) + artwork_test = aspect_env.Program('ArtworkPackLifecycleHarness', + [aspect_env.Object('ArtworkPackLifecycleHarness.o', '#test/ArtworkPackLifecycleHarness.cpp'), l1]) + env.Alias('artwork-pack-test', artwork_test) resize_test = aspect_env.Program('WindowResizeHarness', [aspect_env.Object('WindowResizeHarness.o', '#test/WindowResizeHarness.cpp'), l1]) env.Alias('resize-test', resize_test) diff --git a/libgag/src/Sprite.cpp b/libgag/src/Sprite.cpp index 8713ae563..d57af116d 100644 --- a/libgag/src/Sprite.cpp +++ b/libgag/src/Sprite.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #if __cplusplus >= 201402L #include @@ -50,24 +51,70 @@ using boost::make_unique; namespace GAGCore { static std::set loadedSprites; - static bool highResolutionEnabled = false; - static std::string packDirectory, packText; - static bool packRead=false; - static bool readPack(const std::string &directory) - { - if(packRead && packDirectory==directory)return !packText.empty(); - packRead=true;packDirectory=directory;packText.clear(); - auto input=Toolkit::getFileManager()->open((directory+"/frames.txt").c_str(),"rb"); - if(!input){std::cerr<<"High-resolution pack unavailable: "<1024*1024){SDL_RWclose(input);std::cerr<<"Invalid high-resolution manifest size"<(size),'\0'); - const auto count=SDL_RWread(input,text.data(),1,text.size());SDL_RWclose(input); - if(count!=text.size())return false; - std::istringstream header(text);std::string magic;int version=0;header>>magic>>version; - if(magic!="GLOB2_HIGHRES"||version!=1){std::cerr<<"Unsupported high-resolution pack"< packFrames; + Sprite::HighResolutionStats loadStats; + + ArtworkSelection selectedArtwork(bool requested, GraphicContext* gc) + { + const char* overrideDir = std::getenv("GLOB2_EXPERIMENT_TEXTURE_DIR"); + const bool gpu = gc && (gc->getOptionFlags() & GraphicContext::USEGPU); + return {(requested || overrideDir) && gpu, gpu, overrideDir ? overrideDir : "data/highres/v1"}; + } + + void readPack() + { + if (packRead) return; + packRead = true; + ++loadStats.manifestParses; + auto input = Toolkit::getFileManager()->open((artwork.directory + "/frames.txt").c_str(), "rb"); + if (!input) + { + std::cerr << "High-resolution pack unavailable: " << artwork.directory << std::endl; + return; + } + const auto size = SDL_RWsize(input); + if (size <= 0 || size > 1024 * 1024) + { + SDL_RWclose(input); + std::cerr << "Invalid high-resolution manifest size" << std::endl; + return; + } + std::string text(static_cast(size), '\0'); + const auto count = SDL_RWread(input, text.data(), 1, text.size()); + SDL_RWclose(input); + if (count != text.size()) return; + std::istringstream stream(text); + std::string magic, id; + int version = 0; + stream >> magic >> version; + if (magic != "GLOB2_HIGHRES" || version != 1) + { + std::cerr << "Unsupported high-resolution pack" << std::endl; + return; + } + PackFrame frame; + while (stream >> id >> frame.width >> frame.height >> frame.scale >> frame.base >> frame.team) + packFrames.emplace(id, frame); + } + } Sprite::RotatedImage::~RotatedImage() { @@ -84,6 +131,7 @@ namespace GAGCore SDL_RWops *rotatedStream; unsigned i = 0; + if (!artworkSelected) setHighResolution(highResolutionRequested); this->fileName = filename; loadedSprites.insert(this); @@ -265,7 +313,7 @@ namespace GAGCore Sprite::HighResolutionStats Sprite::highResolutionStats() { - HighResolutionStats stats; + HighResolutionStats stats = loadStats; for(auto sprite:loadedSprites) { for(auto s:sprite->experimentImages)if(s)stats.cpuBytes+=s->getW()*s->getH()*4; @@ -283,11 +331,31 @@ namespace GAGCore void Sprite::setHighResolution(bool enabled) { - highResolutionEnabled = enabled; - packRead=false;packText.clear(); + highResolutionRequested = enabled; + const auto selection = selectedArtwork(enabled, Toolkit::gc); + if (artworkSelected && artwork == selection) return; + reloadHighResolutionPack(); + } + + void Sprite::reloadHighResolutionPack() + { + if (Toolkit::gc) flushBatches(Toolkit::gc); + artwork = selectedArtwork(highResolutionRequested, Toolkit::gc); + artworkSelected = true; + packRead = false; + packFrames.clear(); + ++loadStats.packReloads; for (auto sprite : loadedSprites) sprite->reloadHighResolution(); } + void Sprite::resetHighResolutionState() + { + artwork = {}; + artworkSelected = highResolutionRequested = packRead = false; + packFrames.clear(); + loadStats = {}; + } + void Sprite::reloadHighResolution() { highResolutionAtlas.reset(); @@ -324,13 +392,13 @@ namespace GAGCore for(int i=0;imaxSize || atlasH>maxSize){reject();return;} - const char *overrideDir=std::getenv("GLOB2_EXPERIMENT_TEXTURE_DIR"); - std::string directory=overrideDir?overrideDir:"data/highres/v1"; + const auto& directory = artwork.directory; std::vector> levels; for(int mip=0;mip<4;++mip) { auto rw=Toolkit::getFileManager()->open((directory+"/"+prefix+"-atlas-mip"+std::to_string(mip)+".png").c_str(),"rb"); if(!rw){reject();return;} + ++loadStats.imageLoads; auto s=IMG_Load_RW(rw,1);if(!s){reject();return;} if(s->w!=(atlasW>>mip)||s->h!=(atlasH>>mip)){SDL_FreeSurface(s);reject();return;} levels.emplace_back(new DrawableSurface(s));SDL_FreeSurface(s); @@ -376,36 +444,38 @@ namespace GAGCore { const size_t index=experimentImages.size(); experimentImages.push_back(nullptr); experimentRotated.push_back(nullptr); - const char *overrideDir=std::getenv("GLOB2_EXPERIMENT_TEXTURE_DIR"); - if ((!highResolutionEnabled && !overrideDir) || !Toolkit::gc || !(Toolkit::gc->getOptionFlags() & GraphicContext::USEGPU)) return; - std::string directory=overrideDir ? overrideDir : "data/highres/v1"; - if(!readPack(directory))return; - std::istringstream stream(packText);std::string magic,id,base,team;int version,w,h,scale; - stream>>magic>>version; - std::string wanted=frameName.substr(frameName.find_last_of('/')+1);wanted.resize(wanted.size()-4); - while(stream>>id>>w>>h>>scale>>base>>team) + if (!artwork.enabled) return; + readPack(); + std::string wanted = frameName.substr(frameName.find_last_of('/') + 1); + wanted.resize(wanted.size() - 4); + const auto found = packFrames.find(wanted); + if (found == packFrames.end()) return; + const auto& [w, h, scale, base, team] = found->second; + const auto& id = found->first; + const auto& directory = artwork.directory; + if (w != getW(index) || h != getH(index) || scale != 4) { - if(id!=wanted)continue; - if(w!=getW(index)||h!=getH(index)||scale!=4){std::cerr<<"High-resolution dimensions rejected: "<DrawableSurface* - { - if(name=="-")return nullptr; - if(name.find_first_of("/\\:")!=std::string::npos || name.find("..")!=std::string::npos)return nullptr; - SDL_RWops *rw=Toolkit::getFileManager()->open((directory+"/"+name).c_str(),"rb"); - if(!rw)return nullptr; - SDL_Surface *surface=IMG_Load_RW(rw,1);if(!surface)return nullptr; - int lw=original?original->getW():w,lh=original?original->getH():h; - if(surface->w!=lw*scale||surface->h!=lh*scale){SDL_FreeSurface(surface);return nullptr;} - auto result=new DrawableSurface(surface);result->highResolutionSampling=true;SDL_FreeSurface(surface);return result; - }; - auto normal=load(base,images[index]); - auto colored=load(team,rotated[index]?rotated[index]->orig:nullptr); - if((base!="-"&&!normal)||(team!="-"&&!colored)||(images[index]&&base=="-")||(rotated[index]&&team=="-")) - {delete normal;delete colored;std::cerr<<"High-resolution frame rejected: "<DrawableSurface* + { + if(name=="-")return nullptr; + if(name.find_first_of("/\\:")!=std::string::npos || name.find("..")!=std::string::npos)return nullptr; + SDL_RWops *rw=Toolkit::getFileManager()->open((directory+"/"+name).c_str(),"rb"); + if(!rw)return nullptr; + ++loadStats.imageLoads; + SDL_Surface *surface=IMG_Load_RW(rw,1);if(!surface)return nullptr; + int lw=original?original->getW():w,lh=original?original->getH():h; + if(surface->w!=lw*scale||surface->h!=lh*scale){SDL_FreeSurface(surface);return nullptr;} + auto result=new DrawableSurface(surface);result->highResolutionSampling=true;SDL_FreeSurface(surface);return result; + }; + auto normal=load(base,images[index]); + auto colored=load(team,rotated[index]?rotated[index]->orig:nullptr); + if((base!="-"&&!normal)||(team!="-"&&!colored)||(images[index]&&base=="-")||(rotated[index]&&team=="-")) + {delete normal;delete colored;std::cerr<<"High-resolution frame rejected: "<setMinRes(640, 480); + Sprite::setHighResolution(settings.highResolutionArtwork); // load data required for drawing progress screen title = std::make_unique("data/gfx/title.png"); diff --git a/src/SettingsScreenGeneral.cpp b/src/SettingsScreenGeneral.cpp index 7dcc58a24..4a3bddb3b 100644 --- a/src/SettingsScreenGeneral.cpp +++ b/src/SettingsScreenGeneral.cpp @@ -51,7 +51,7 @@ void SettingsScreen::buildGeneral() auto& flags=globalContainer->settings.optionFlags; if(v)flags|=GlobalContainer::OPTION_LOW_SPEED_GFX;else flags&=~GlobalContainer::OPTION_LOW_SPEED_GFX;commit(); }); - toggle("graphics.artwork","High-resolution artwork","Apply artwork on the next game or editor load (OpenGL).",s.highResolutionArtwork,[this](int v){globalContainer->settings.highResolutionArtwork=v;commit();}); + toggle("graphics.artwork","High-resolution artwork","Applies immediately (OpenGL).",s.highResolutionArtwork,[this](int v){globalContainer->settings.highResolutionArtwork=v;Sprite::setHighResolution(v);commit();}); toggle("graphics.torus","Automatic torus view","Automatically show the torus overview while moving around the map (OpenGL).",s.automaticTorus,[this](int v){globalContainer->settings.automaticTorus=v;commit();}); choice("graphics.renderer","Renderer","Changing the renderer requires a restart.",bool(s.screenFlags & GraphicContext::USEGPU), {tr("Software"),"OpenGL"},[this](int v){changeDisplay([v](Settings& s){if(v)s.screenFlags|=GraphicContext::USEGPU;else s.screenFlags&=~GraphicContext::USEGPU;});}); diff --git a/src/gui/GameGUI.cpp b/src/gui/GameGUI.cpp index 437e678c2..3ce7fa61a 100644 --- a/src/gui/GameGUI.cpp +++ b/src/gui/GameGUI.cpp @@ -63,7 +63,6 @@ GameGUI::GameGUI() GameGUI::~GameGUI() { - if (!globalContainer->runNoX) Sprite::setHighResolution(false); for (ParticleSet::iterator it = particles.begin(); it != particles.end(); ++it) delete *it; if (globalContainer->settings.rememberUnit) @@ -84,7 +83,6 @@ void GameGUI::init() torusView.reset(); torusPointerDown = false; camera=MapCamera();zoomControlPushed=false; - if (!globalContainer->runNoX) Sprite::setHighResolution(globalContainer->settings.highResolutionArtwork); notmenu = false; isRunning=true; gamePaused=false; diff --git a/src/map/edit/MapEditCtor.cpp b/src/map/edit/MapEditCtor.cpp index 69cf0dba1..99780dcfd 100644 --- a/src/map/edit/MapEditCtor.cpp +++ b/src/map/edit/MapEditCtor.cpp @@ -22,7 +22,6 @@ MapEdit::MapEdit() 128, // height Minimap::HideFOW) { - Sprite::setHighResolution(globalContainer->settings.highResolutionArtwork); doQuit=false; doFullQuit=false; doQuitAfterLoadSave=false; @@ -307,8 +306,6 @@ MapEdit::MapEdit() MapEdit::~MapEdit() { - Sprite::setHighResolution(false); - Toolkit::releaseSprite("data/gui/editor"); for(std::vector::iterator i=mew.begin(); i!=mew.end(); ++i) { delete *i; diff --git a/test/ArtworkPackLifecycleHarness.cpp b/test/ArtworkPackLifecycleHarness.cpp new file mode 100644 index 000000000..35644fef4 --- /dev/null +++ b/test/ArtworkPackLifecycleHarness.cpp @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace GAGCore; +namespace fs = std::filesystem; + +class InspectSprite : public Sprite +{ +public: + bool hasHD() const { return !experimentImages.empty() && experimentImages[0]; } + DrawableSurface* hd() const { return hasHD() ? experimentImages[0] : nullptr; } +}; + +static void noLoads(const Sprite::HighResolutionStats& before) +{ + const auto after = Sprite::highResolutionStats(); + assert(after.imageLoads == before.imageLoads); + assert(after.manifestParses == before.manifestParses); + assert(after.packReloads == before.packReloads); +} + +static void png(const fs::path& path, int side) +{ + auto surface = SDL_CreateRGBSurfaceWithFormat(0, side, side, 32, SDL_PIXELFORMAT_RGBA32); + assert(surface); + SDL_FillRect(surface, nullptr, SDL_MapRGBA(surface->format, 20, 180, 80, 255)); + assert(IMG_SavePNG(surface, path.string().c_str()) == 0); + SDL_FreeSurface(surface); +} + +int main(int argc, char** argv) +{ + const bool software = argc > 1 && std::string(argv[1]) == "software"; + const fs::path root = fs::absolute(".cache/artwork-pack-lifecycle"); + fs::create_directories(root / "a"); + fs::create_directories(root / "b"); + fs::create_directories(root / "missing"); + const auto manifest = root / "a/frames.txt"; + const auto missing = root / "missing/frames.txt"; + fs::remove(missing); + auto write = [&](const std::string& body) { std::ofstream(manifest) << body; }; + const std::string valid = "GLOB2_HIGHRES 1\nprobe0 2 2 4 hd.png -\n"; + const auto original = (root / "probe").string(); + for (int session = 0; session < 2; ++session) + { + unsetenv("GLOB2_EXPERIMENT_TEXTURE_DIR"); + Toolkit::init("glob2-artwork-pack-test"); + auto gfx = Toolkit::initGraphic(640, 480, software ? 0 : GraphicContext::USEGPU, "Artwork lifecycle test"); + Toolkit::getFileManager()->addDir(root.string()); + png(root / "probe0.png", 2); + png(root / "hd.png", 8); + png(root / "a/hd.png", 8); + png(root / "b/hd.png", 8); + assert(Sprite::highResolutionStats().imageLoads == 0); + if (session == 1) + { + InspectSprite sprite; + assert(sprite.load(original)); + assert(!sprite.hasHD()); + } + else + { + // Select before loading any sprites, then inherit that selection. + write(valid + "probe0 99 99 4 absent.png -\n"); + std::ofstream(root / "b/frames.txt") << valid; + setenv("GLOB2_EXPERIMENT_TEXTURE_DIR", (root / "a").string().c_str(), 1); + Sprite::setHighResolution(true); + InspectSprite sprite; + assert(sprite.load(original)); + assert(sprite.hasHD() == !software); + auto before = Sprite::highResolutionStats(); + assert(before.manifestParses == (software ? 0u : 1u)); + auto hd = sprite.hd(); + gfx->drawSprite(0, 0, &sprite, 0); + Sprite::setHighResolution(true); + noLoads(before); + assert(sprite.hd() == hd); + { + InspectSprite late; + assert(late.load(original)); + assert(late.hasHD() == !software); + assert(Sprite::highResolutionStats().manifestParses == before.manifestParses); + } + before = Sprite::highResolutionStats(); + Sprite::reloadHighResolutionPack(); + assert(Sprite::highResolutionStats().packReloads == before.packReloads + 1); + assert(sprite.hasHD() == !software); + setenv("GLOB2_EXPERIMENT_TEXTURE_DIR", (root / "b").string().c_str(), 1); + before = Sprite::highResolutionStats(); + Sprite::setHighResolution(true); + assert(Sprite::highResolutionStats().packReloads == before.packReloads + 1); + assert(sprite.hasHD() == !software); + // The experimental override intentionally enables HD even when the preference is off. + before = Sprite::highResolutionStats(); + Sprite::setHighResolution(false); + noLoads(before); + unsetenv("GLOB2_EXPERIMENT_TEXTURE_DIR"); + Sprite::setHighResolution(false); + assert(!sprite.hasHD()); + assert(Sprite::highResolutionStats().cpuBytes == 0); + before = Sprite::highResolutionStats(); + Sprite::setHighResolution(false); + noLoads(before); + + setenv("GLOB2_EXPERIMENT_TEXTURE_DIR", (root / "missing").string().c_str(), 1); + Sprite::setHighResolution(true); + assert(!sprite.hasHD()); + before = Sprite::highResolutionStats(); + std::ofstream(missing) << "GLOB2_HIGHRES 1\nprobe0 2 2 4 ../hd.png -\n"; + Sprite::setHighResolution(true); + noLoads(before); // Missing packs remain cached until deliberate invalidation. + Sprite::reloadHighResolutionPack(); + assert(!sprite.hasHD()); // Unsafe filenames remain rejected. + + setenv("GLOB2_EXPERIMENT_TEXTURE_DIR", (root / "a").string().c_str(), 1); + for (const auto& invalid : {"bad header", "GLOB2_HIGHRES 9\n", "GLOB2_HIGHRES 1\nprobe0 3 2 4 hd.png -\n", "GLOB2_HIGHRES 1\nprobe0 2 2 4 absent.png -\n"}) + { + write(invalid); + Sprite::reloadHighResolutionPack(); + assert(!sprite.hasHD()); + before = Sprite::highResolutionStats(); + Sprite::setHighResolution(true); + noLoads(before); + } + write("GLOB2_HIGHRES 1\nprobe0 3 2 4 hd.png -\nprobe0 2 2 4 hd.png -\n"); + Sprite::reloadHighResolutionPack(); + assert(!sprite.hasHD()); // An invalid first entry is not replaced by a duplicate. + write(valid); + png(root / "a/hd.png", 4); + Sprite::reloadHighResolutionPack(); + assert(!sprite.hasHD()); + png(root / "a/hd.png", 8); + write(std::string(1024 * 1024 + 1, 'x')); + Sprite::reloadHighResolutionPack(); + assert(!sprite.hasHD()); + write(valid); + Sprite::reloadHighResolutionPack(); + assert(sprite.hasHD() == !software); + assert(Sprite::highResolutionStats().cpuBytes == (software ? 0u : 8u * 8u * 4u)); + Sprite::flushBatches(gfx); + } + Toolkit::close(); + assert(Sprite::highResolutionStats().imageLoads == 0); + } + unsetenv("GLOB2_EXPERIMENT_TEXTURE_DIR"); + std::cout << "PASS artwork selection, inheritance, no-op application, explicit reload, pack validation, fallback and toolkit reinitialization\n"; +} diff --git a/test/HighResolutionIntegrationHarness.cpp b/test/HighResolutionIntegrationHarness.cpp index 0e7e5606d..2e7928548 100644 --- a/test/HighResolutionIntegrationHarness.cpp +++ b/test/HighResolutionIntegrationHarness.cpp @@ -6,6 +6,9 @@ #include "SettingsScreen.h" #include "Order.h" #include "Unit.h" +#include "Player.h" +#include "FrontendTheme.h" +#include #include #include #ifdef __APPLE__ @@ -23,6 +26,19 @@ GlobalContainer* globalContainer=nullptr; class SettingsPaintHarness:public SettingsScreen { public: + void chooseArtwork(bool enabled) + { + for (auto widget : widgets) + if (auto button = dynamic_cast(widget); button && button->returnCode == HIGHRES) + { + button->setState(enabled); + onAction(button, BUTTON_STATE_CHANGED, HIGHRES, 0); + return; + } + assert(false); + } + void confirm() { onAction(nullptr, BUTTON_RELEASED, OK, 0); } + void cancel() { onAction(nullptr, BUTTON_RELEASED, CANCEL, 0); } void draw(GraphicContext *surface){gfx=surface;dispatchInit();paint();for(auto widget:widgets)if(widget->visible)widget->paint();} }; class HighResolutionIntegrationHarness @@ -67,6 +83,7 @@ class HighResolutionIntegrationHarness { auto gfx=globalContainer->gfx; globalContainer->settings.highResolutionArtwork=true; + Sprite::setHighResolution(globalContainer->settings.highResolutionArtwork); MapEdit editor;editor.game.map.setSize(4,4,GRASS);editor.game.map.setGame(&editor.game);editor.game.addTeam(0); auto building=editor.game.addBuilding(15,15,globalContainer->buildingsTypes.getFinishedTypeNum("swarm"),0);assert(building); editor.regenerateGameHeader();editor.minimap.setGame(editor.game);editor.updateCamera(); @@ -136,9 +153,122 @@ class HighResolutionIntegrationHarness std::cout<<"PASS full-period seam sprite coverage, single building identity, minimap outline and native cursor scale\n"; } public: + static void unchangedArtwork(const Sprite::HighResolutionStats& before) + { + const auto after = Sprite::highResolutionStats(); + assert(before.imageLoads == after.imageLoads); + assert(before.manifestParses == after.manifestParses); + assert(before.packReloads == after.packReloads); + } + static void lifecycle() + { + FrontendTheme frontend; + FrontendScope menus; + auto drawMenu = [&]() { + frontend.onFrame(); + frontend.background(globalContainer->gfx, false); + globalContainer->gfx->nextFrame(); + }; + drawMenu(); + const bool originalSetting = globalContainer->settings.highResolutionArtwork; + const bool gpu = globalContainer->gfx->getOptionFlags() & GraphicContext::USEGPU; + const auto startup = Sprite::highResolutionStats(); + assert(startup.manifestParses == (originalSetting && gpu ? 1u : 0u)); + assert((startup.cpuBytes > 0) == (originalSetting && gpu)); + { + SettingsPaintHarness screen; + auto before = Sprite::highResolutionStats(); + screen.chooseArtwork(!originalSetting); + unchangedArtwork(before); + screen.cancel(); + unchangedArtwork(before); + assert(globalContainer->settings.highResolutionArtwork == originalSetting); + } + { + SettingsPaintHarness screen; + auto before = Sprite::highResolutionStats(); + screen.chooseArtwork(!originalSetting); + screen.chooseArtwork(originalSetting); + screen.confirm(); + unchangedArtwork(before); + } + for (const bool hd : { !originalSetting, originalSetting }) + { + SettingsPaintHarness screen; + const auto before = Sprite::highResolutionStats(); + screen.chooseArtwork(hd); + unchangedArtwork(before); + const auto start = std::chrono::steady_clock::now(); + screen.confirm(); + const double ms = std::chrono::duration(std::chrono::steady_clock::now() - start).count(); + const auto after = Sprite::highResolutionStats(); + assert(after.packReloads == before.packReloads + (gpu ? 1 : 0)); + assert((after.cpuBytes > 0) == (hd && gpu)); + std::cout << "ARTWORK_SETTINGS hd=" << hd << " ms=" << ms << " cpu_bytes=" << after.cpuBytes << '\n'; + } + drawMenu(); + // Editor-only UI sprites can be loaded lazily, but subsequent visits reuse them. + { MapEdit editor; assert(editor.load("maps/SmallForTwo.map")); } + auto before = Sprite::highResolutionStats(); + for (int repeat = 0; repeat < 3; ++repeat) + { + { MapEdit editor; assert(editor.load("maps/SmallForTwo.map")); } + unchangedArtwork(before); + } + globalContainer->automaticEndingGame = true; + globalContainer->automaticEndingSteps = 1; + for (const std::string mapName : {"SmallForTwo", "Oazis"}) + { + size_t warmBytes = 0; + for (int repeat = 0; repeat < 3; ++repeat) + { + { + Engine engine; + auto map = Engine::loadMapHeader("maps/" + mapName + ".map"); + GameHeader header; + std::vector aiTeams; + const auto ai = mapName == "SmallForTwo" ? AI::NUMBI : AI::CORTEX; + for (int team = 0; team < map.getNumberOfTeams(); ++team) + { + header.getBasePlayer(team) = BasePlayer(team, team == 0 ? "Human" : "AI", team, + team == 0 ? BasePlayer::P_LOCAL : Player::playerTypeFromImplementationID(ai)); + if (team) aiTeams.push_back(team); + } + header.setNumberOfPlayers(map.getNumberOfTeams()); + header.setDefaultAlliances(0, aiTeams); + header.setRandomSeed(42); + engine.gui.localPlayer = engine.gui.localTeamNo = 0; + const auto loads = Sprite::highResolutionStats(); + const auto start = std::chrono::steady_clock::now(); + assert(engine.initGame(map, header) == Engine::EE_NO_ERROR); + engine.run(); // One tick, including the first presented frame and session teardown. + const double ms = std::chrono::duration(std::chrono::steady_clock::now() - start).count(); + unchangedArtwork(loads); + assert(engine.gui.game.stepCounter == 1); + const auto resident = Sprite::highResolutionStats(); + if (repeat) assert(resident.cpuBytes == warmBytes); + warmBytes = resident.cpuBytes; + std::cout << "ARTWORK_MATCH map=" << mapName << " repeat=" << repeat << " ms=" << ms + << " cpu_bytes=" << resident.cpuBytes << " gpu_bytes=" << DrawableSurface::allocatedTextureBytes() + << " image_loads=0 manifest_parses=0 pack_reloads=0\n"; + } + drawMenu(); + unchangedArtwork(before); + } + } + if (gpu) + { + frontend.background(globalContainer->gfx, false); + capture("lifecycle-menu"); + unchangedArtwork(before); + } + globalContainer->automaticEndingGame = false; + std::cout << "PASS startup, settings confirmation/cancel, repeated matches/editor visits, and stable retained artwork\n"; + } static void runSoftware() { globalContainer->settings.highResolutionArtwork=true; + Sprite::setHighResolution(globalContainer->settings.highResolutionArtwork); { Engine engine;assert(engine.initCustom("games/gd-small-2ai.game")==Engine::EE_NO_ERROR); auto &gui=engine.gui;gui.updateCamera();gui.zoomMap(10,300,300);gui.drawAll(0); @@ -162,6 +292,7 @@ class HighResolutionIntegrationHarness for(bool hd:{false,true}) { globalContainer->settings.highResolutionArtwork=hd; + Sprite::setHighResolution(globalContainer->settings.highResolutionArtwork); Engine engine;assert(engine.initCustom("games/gd-small-2ai.game")==Engine::EE_NO_ERROR); auto &gui=engine.gui;gui.updateCamera(); const auto checksum=gui.game.checkSum(nullptr,nullptr,nullptr,true); @@ -208,6 +339,7 @@ class HighResolutionIntegrationHarness } } globalContainer->settings.highResolutionArtwork=true; + Sprite::setHighResolution(globalContainer->settings.highResolutionArtwork); { // Record with this build's version: master intentionally rejects // historical replays after simulation/pathfinding changes. @@ -262,6 +394,7 @@ class HighResolutionIntegrationHarness for(bool hd:{false,true}) { globalContainer->settings.highResolutionArtwork=hd; + Sprite::setHighResolution(globalContainer->settings.highResolutionArtwork); MapEdit dense;dense.game.map.setSize(6,6,GRASS);dense.game.map.setGame(&dense.game); for(int team=0;team<4;++team)dense.game.addTeam(team); const char *types[]={"swarm","inn","hospital","school","swimmingpool","barracks"}; @@ -284,6 +417,7 @@ class HighResolutionIntegrationHarness dense.drawMenu();dense.drawMiniMap();dense.drawWidgets();capture(std::string(hd?"dense-hd-":"dense-original-")+std::to_string(int(zoom*100))); } } + Sprite::setHighResolution(false); assert(Sprite::highResolutionStats().cpuBytes==0); std::cout<<"PASS gameplay/editor conversions, Alt-wheel isolation, zoom controls, replay drawing, stable simulation checksums and resource release\n"; } @@ -296,8 +430,15 @@ int main(int argc,char **argv) globals.settings.screenWidth=1024;globals.settings.screenHeight=768;globals.settings.screenFlags=GraphicContext::USEGPU|GraphicContext::CUSTOMCURSOR; globals.settings.rememberUnit=false;globals.settings.mute=1; globals.fileManager->addDir(".cache/highres-replay-fixture"); - const bool software=argc>1&&std::string(argv[1])=="software"; + const std::string mode = argc > 1 ? argv[1] : ""; + const bool lifecycle = mode.starts_with("lifecycle"); + const bool software = mode == "software" || mode == "lifecycle-software"; + globals.settings.highResolutionArtwork = mode != "lifecycle-original"; if(software)globals.settings.screenFlags=0; + const auto start = std::chrono::steady_clock::now(); globals.load(); - if(software)HighResolutionIntegrationHarness::runSoftware();else HighResolutionIntegrationHarness::run(); + std::cout << "ARTWORK_STARTUP ms=" << std::chrono::duration(std::chrono::steady_clock::now() - start).count() + << " cpu_bytes=" << Sprite::highResolutionStats().cpuBytes << " gpu_bytes=" << DrawableSurface::allocatedTextureBytes() << '\n'; + if (lifecycle) HighResolutionIntegrationHarness::lifecycle(); + else if(software)HighResolutionIntegrationHarness::runSoftware();else HighResolutionIntegrationHarness::run(); } diff --git a/test/README.md b/test/README.md index 88a44aeaa..2ec233508 100644 --- a/test/README.md +++ b/test/README.md @@ -345,3 +345,19 @@ keyboard file formats remain unchanged. settings, language, persistence, keyboard, multiplayer eligibility and camera cadence regressions without starting the unrelated engine/replay scenarios. The full invocation remains available and reports buffered diagnostics on timeout. + +## Artwork lifecycle + +Build with `scons release=1 server=0 -j8 artwork-pack-test highres-integration-test` +from the repository root. Run `build/libgag/src/ArtworkPackLifecycleHarness` and +repeat with `software` to check pack caching, validation, explicit reload, late +sprites, and toolkit reinitialization. + +Run `build/src/HighResolutionIntegrationHarness lifecycle`, `lifecycle-original`, +and `lifecycle-software` to check startup selection, settings confirmation/cancel, +and repeated game/editor transitions. The harness reports startup, settings and +one-tick game timings, retained artwork bytes, and asserts zero sprite image +loads, manifest parses or pack reloads during matches. Its normal invocation and +`software` mode additionally check rendering, replay loading and simulation +checksums. Test settings and replays use a separate profile. On macOS, prefix +these commands with `DYLD_LIBRARY_PATH=/opt/homebrew/lib` if SDL requires it. From 9783fd40d40b39259e78dfd81f0b79ba5b5ef22d Mon Sep 17 00:00:00 2001 From: Bradley Arsenault Date: Thu, 10 Sep 2026 17:18:02 -0400 Subject: [PATCH 2/2] Address review feedback on the artwork-lifecycle rebase - Sprite::readPack(): report a truncated manifest read to std::cerr like every other failure path in the function, instead of failing silently (a truncated manifest previously looked identical to a manifest that simply lacks the frame). - HighResolutionIntegrationHarness: print the real measured image_loads/manifest_parses/pack_reloads deltas in the ARTWORK_MATCH line instead of hardcoded zeros, so the log can't be misread as telemetry when it was actually a literal. - Adapt SettingsPaintHarness and the settings lifecycle sub-tests to the redesigned SettingsScreen (#236), which landed on master after this branch and removed the old OK/Cancel button-driven flow in favor of a semantic changeSetting()/rows() interface with immediate per-toggle apply. The high-resolution-artwork toggle now calls Sprite::setHighResolution() directly from its change callback; the tests and test/README.md are updated to reflect that a no-op re-choice reloads nothing, while a real change reloads on every apply (there is no batched confirm step left to coalesce repeated toggles). Verified: scons -j8 release=1 server=0 artwork-pack-test highres-integration-test builds clean; ArtworkPackLifecycleHarness (default and software) and HighResolutionIntegrationHarness lifecycle/lifecycle-original/lifecycle-software all pass. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KFxsZmLM4qsovemHqDrHGP --- libgag/src/Sprite.cpp | 6 +++- test/HighResolutionIntegrationHarness.cpp | 40 +++++++++-------------- test/README.md | 6 ++-- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/libgag/src/Sprite.cpp b/libgag/src/Sprite.cpp index d57af116d..ebf06266f 100644 --- a/libgag/src/Sprite.cpp +++ b/libgag/src/Sprite.cpp @@ -100,7 +100,11 @@ namespace GAGCore std::string text(static_cast(size), '\0'); const auto count = SDL_RWread(input, text.data(), 1, text.size()); SDL_RWclose(input); - if (count != text.size()) return; + if (count != text.size()) + { + std::cerr << "Truncated high-resolution manifest: " << artwork.directory << std::endl; + return; + } std::istringstream stream(text); std::string magic, id; int version = 0; diff --git a/test/HighResolutionIntegrationHarness.cpp b/test/HighResolutionIntegrationHarness.cpp index 2e7928548..665b47207 100644 --- a/test/HighResolutionIntegrationHarness.cpp +++ b/test/HighResolutionIntegrationHarness.cpp @@ -8,7 +8,6 @@ #include "Unit.h" #include "Player.h" #include "FrontendTheme.h" -#include #include #include #ifdef __APPLE__ @@ -26,19 +25,10 @@ GlobalContainer* globalContainer=nullptr; class SettingsPaintHarness:public SettingsScreen { public: - void chooseArtwork(bool enabled) - { - for (auto widget : widgets) - if (auto button = dynamic_cast(widget); button && button->returnCode == HIGHRES) - { - button->setState(enabled); - onAction(button, BUTTON_STATE_CHANGED, HIGHRES, 0); - return; - } - assert(false); - } - void confirm() { onAction(nullptr, BUTTON_RELEASED, OK, 0); } - void cancel() { onAction(nullptr, BUTTON_RELEASED, CANCEL, 0); } + // The redesigned screen has no separate confirm/cancel step for discrete + // toggles (see the "Stable semantic interface" in SettingsScreen.h): + // changeSetting applies and persists immediately, same as a real click. + void chooseArtwork(bool enabled) { assert(changeSetting("graphics.artwork", enabled)); } void draw(GraphicContext *surface){gfx=surface;dispatchInit();paint();for(auto widget:widgets)if(widget->visible)widget->paint();} }; class HighResolutionIntegrationHarness @@ -176,30 +166,30 @@ class HighResolutionIntegrationHarness assert(startup.manifestParses == (originalSetting && gpu ? 1u : 0u)); assert((startup.cpuBytes > 0) == (originalSetting && gpu)); { + // Choosing the value already in effect is a no-op: no reload. SettingsPaintHarness screen; auto before = Sprite::highResolutionStats(); - screen.chooseArtwork(!originalSetting); - unchangedArtwork(before); - screen.cancel(); + screen.chooseArtwork(originalSetting); unchangedArtwork(before); assert(globalContainer->settings.highResolutionArtwork == originalSetting); } { + // Toggling away and back applies live each time and reloads on + // both edges (there is no batched confirm left to coalesce them). SettingsPaintHarness screen; auto before = Sprite::highResolutionStats(); screen.chooseArtwork(!originalSetting); screen.chooseArtwork(originalSetting); - screen.confirm(); - unchangedArtwork(before); + const auto after = Sprite::highResolutionStats(); + assert(after.packReloads == before.packReloads + (gpu ? 2u : 0u)); + assert(globalContainer->settings.highResolutionArtwork == originalSetting); } for (const bool hd : { !originalSetting, originalSetting }) { SettingsPaintHarness screen; const auto before = Sprite::highResolutionStats(); - screen.chooseArtwork(hd); - unchangedArtwork(before); const auto start = std::chrono::steady_clock::now(); - screen.confirm(); + screen.chooseArtwork(hd); const double ms = std::chrono::duration(std::chrono::steady_clock::now() - start).count(); const auto after = Sprite::highResolutionStats(); assert(after.packReloads == before.packReloads + (gpu ? 1 : 0)); @@ -250,7 +240,9 @@ class HighResolutionIntegrationHarness warmBytes = resident.cpuBytes; std::cout << "ARTWORK_MATCH map=" << mapName << " repeat=" << repeat << " ms=" << ms << " cpu_bytes=" << resident.cpuBytes << " gpu_bytes=" << DrawableSurface::allocatedTextureBytes() - << " image_loads=0 manifest_parses=0 pack_reloads=0\n"; + << " image_loads=" << (resident.imageLoads - loads.imageLoads) + << " manifest_parses=" << (resident.manifestParses - loads.manifestParses) + << " pack_reloads=" << (resident.packReloads - loads.packReloads) << "\n"; } drawMenu(); unchangedArtwork(before); @@ -263,7 +255,7 @@ class HighResolutionIntegrationHarness unchangedArtwork(before); } globalContainer->automaticEndingGame = false; - std::cout << "PASS startup, settings confirmation/cancel, repeated matches/editor visits, and stable retained artwork\n"; + std::cout << "PASS startup, live settings application, repeated matches/editor visits, and stable retained artwork\n"; } static void runSoftware() { diff --git a/test/README.md b/test/README.md index 2ec233508..b5873fb56 100644 --- a/test/README.md +++ b/test/README.md @@ -354,8 +354,10 @@ repeat with `software` to check pack caching, validation, explicit reload, late sprites, and toolkit reinitialization. Run `build/src/HighResolutionIntegrationHarness lifecycle`, `lifecycle-original`, -and `lifecycle-software` to check startup selection, settings confirmation/cancel, -and repeated game/editor transitions. The harness reports startup, settings and +and `lifecycle-software` to check startup selection, live settings application +(a no-op re-choice reloads nothing, a real change reloads on every apply since +the redesigned settings screen has no batched confirm step), and repeated +game/editor transitions. The harness reports startup, settings and one-tick game timings, retained artwork bytes, and asserts zero sprite image loads, manifest parses or pack reloads during matches. Its normal invocation and `software` mode additionally check rendering, replay loading and simulation