Decode RLE textures, light dungeons, and add a screenshot path - #20
Closed
psi29a wants to merge 36 commits into
Closed
Decode RLE textures, light dungeons, and add a screenshot path#20psi29a wants to merge 36 commits into
psi29a wants to merge 36 commits into
Conversation
OSG sizes a texture's mipmap chain from the power-of-two-rounded dimensions but allocates immutable storage at the true size, so every NPOT texture asked glTexStorage3D for one level more than it can hold: glTexStorage3D(too many levels for max texture dimension) glTexSubImage3D(invalid texture level 0) glGenerateMipmap(zero size base image) The failed allocation took the image upload down with it, leaving those textures empty -- they rendered solid black. Daggerfall's art is full of NPOT sizes (13x13, 40x52, ...), so this hit a good number of objects; power-of-two textures were unaffected, which is why it looked arbitrary. Opt out of immutable storage. OSG then uses glTexImage3D plus glGenerateMipmap, which keeps both the exact dimensions and the mipmaps, so nothing is rescaled and the pixely look is preserved. Verified by capturing the same frame with and without: 55 GL errors to 0, and the previously black objects render textured while the rest of the scene is unchanged.
The same data path is routinely listed by more than one config file -- the launcher writes it to the user's settings.cfg and the build tree ships its own -- and the loader merges them all. Every duplicate was appended to gRootPaths and then searched again on each file lookup, and showed up twice in the startup log.
Print the version of each subsystem we bring up, the way thirdeye does. Knowing which SDL, OSG, MyGUI and GL driver a run actually used makes bug reports far easier to act on -- the last two bugs here (a MyGUI ABI mismatch and an OSG texture-storage quirk) both turned on exactly that. The SDL and GL versions are queried at runtime rather than taken from the headers, since those are what the process really linked against. The GL strings need a current context, which the draw thread owns after realize(), so the query is handed to that thread as a graphics operation instead of taking the context away from it. Also collapse the duplicated data-path listing into a single loop so the config-file and -data paths are reported once each.
INNOEXTRACT_TAG names a branch rather than a tag, so let FetchContent re-check it on every configure - a shallow clone of a moving branch otherwise stays pinned to whatever it first fetched. Also record why this points at a fork: no released innoextract can read the installer DaggerfallSetup ships. The only release (1.9, 2020) stops at Inno Setup 6.0.5, upstream master reaches 6.4.0, and DaggerfallSetup 3.2.0 is built with Inno Setup 6.6.1.
The README's CI badge pointed at travis-ci.org/kcat/opendf -- a service that no longer exists, for the old upstream repo. It rendered "build unknown". Drop it and .travis.yml along with it; that config was pinned to Ubuntu trusty, g++-5 and OSG 3.4 and had long stopped describing how this builds. The dependency list was also incomplete: it named OSG, MyGUI and SDL2 but not OpenGL, nor Qt6 for the launcher -- which is ON by default, so a fresh clone failed to configure without it. There were no build or run instructions at all. Add per-platform dependency and build instructions for Linux, macOS and Windows, using Ninja throughout, in the style of thirdeye's README. All Debian package names are checked against the archive, and the whole sequence was run from a clean clone: it configures and builds opendf, opendf-launcher and bsatool. Also document how to actually start the game -- the launcher first, since it writes the config for you, then the manual data-root route and the engine's command line options. Add docs/ROADMAP.md. The first milestone is clearing the TODO/FIXMEs already in the tree, each described with where it is and why it matters rather than as a bare list, and it points at Daggerfall Unity as prior art for everything after that.
OSG is effectively unmaintained -- two of the bugs worked around in this tree are its own -- so the question of moving to VulkanSceneGraph keeps coming up. Write down the answer rather than re-deriving it each time. Supporting both is ruled out: OSG is OpenGL and VSG is Vulkan, so it would mean two renderers, two shader sets and two GUI backends for a project that is not yet a game. Migrating is more tractable than it used to be, and the section says why with figures taken from the tree. MyGUI 3.5 ships a Vulkan platform that initialises from raw VkInstance/VkDevice/VkQueue handles, and VSG exposes exactly those, so our ~990-line mygui_osg backend would mostly be deleted rather than ported. SDL creates Vulkan surfaces already -- in SDL2 as well as SDL3, so that is not a reason to move to SDL3 either. What is left is the real cost: the hand-built deferred G-buffer in pipeline.cpp, and rewriting 14 GLSL 130 shaders that use OSG's built-in uniforms into Vulkan dialect. Not scheduled, and the section says so, along with the two things worth doing anyway that would make it cheaper whenever it happens.
Don't make Qt6 a hard dependency of the engine. BUILD_LAUNCHER becomes a tri-state: AUTO (the new default) builds the launcher when Qt6 6.5 is present and quietly skips it otherwise, ON demands it, OFF never builds it. Configuring the engine on a machine without qt6-base-dev no longer fails. Raise the CMake minimum to 3.16. Qt6's config files require it, and the optional bundled innoextract uses FetchContent_MakeAvailable, which needs 3.14; 3.10 could stop configuration outright. Make MYGUI_DONT_USE_OBSOLETE an option rather than hard-coding it. It still defaults ON because that matches the Debian/Ubuntu MyGUI packages, but MyGUI's own default is FALSE and it installs nothing that records the choice, so a distro following upstream needs to turn this off rather than silently mismatching the widget layout. Stop excluding user input from the three processEvents calls that keep the progress dialog alive. ExcludeUserInputEvents swallowed the click on Cancel, so wasCanceled() never became true and neither the install nor the Wine fallback could be interrupted. (QEventLoop::AllEvents is 0x00, so OR-ing it in added nothing.) Escape apostrophes in the PowerShell Expand-Archive fallback. A path such as C:/Users/O'Brien/... terminated the single-quoted literal and broke the only unzip fallback available on Windows. Set *err when a copied directory is missing required game files, instead of returning an empty string that produced an error dialog with no text.
MyGUI 3.5 generates a MyGUI_Config.h recording the options it was built with and includes it from MyGUI_Prerequest.h, so on 3.5+ the headers already agree with the library. Defining MYGUI_DONT_USE_OBSOLETE ourselves there is actively harmful: #cmakedefine writes "/* #undef MYGUI_DONT_USE_OBSOLETE */", a comment rather than an #undef, so a -D on the command line silently wins over the library's own truthful config and reintroduces exactly the EventPair layout mismatch the flag is meant to avoid. So make it tri-state. AUTO looks for MyGUI_Config.h next to the headers: when it's there we define nothing and let MyGUI speak for itself, and when it isn't (3.4 and earlier, which record nothing) we fall back to defining it, matching the Debian/Ubuntu packages. Forcing ON against a MyGUI that ships the config header now warns.
Modelled on thirdeye's workflow: a three-platform matrix with ccache on all of them, vcpkg cached wholesale on Windows, and the built binaries uploaded per platform so a PR can be smoke-tested without building locally. fail-fast is off so one platform breaking still reports the other two. Dependencies differ per platform in ways worth recording: Linux takes everything from apt, launcher included. SDL2, OSG and MyGUI are all in noble, so ubuntu-latest is enough -- no need for a newer image. macOS needs more work than the README implies. Homebrew has no mygui formula at all, so MyGUI is built from source (engine only, dummy render system, demos and tools off, which keeps it short); and there is no sdl2 formula either, so it uses sdl2-compat, the SDL2 shim over SDL3 that installs the sdl2.pc and headers find_package(SDL2) looks for. Windows uses vcpkg for osg, mygui and sdl2, but takes Qt6 from install-qt-action rather than vcpkg's qtbase, which is a 45+ minute from-source build. No test step: opendf has no test suite yet, so the build is the check.
Homebrew has no mygui formula, and sdl2 was replaced by sdl2-compat, so the brew line as written could not work. Document building MyGUI from source with MYGUI_HOME, and switch sdl2 to sdl2-compat. Also update the CMake minimum to the 3.16 the build now requires.
Set OSG_GL_TEXTURE_STORAGE portably and unconditionally. setenv() does not exist on MSVC, and this file already has _WIN32 branches, so the call would have broken a native Windows build; use _putenv_s there. The overwrite argument was also 0, meaning an inherited OSG_GL_TEXTURE_STORAGE=ON would silently win and bring the black NPOT textures back. It is a correctness workaround rather than a preference, so it now overwrites. Pin innoextract to a commit SHA instead of a branch name. With BUILD_INNOEXTRACT=ON this is source fetched and compiled at configure time, so a moving ref means the code built here can change with nothing in this repo changing. GIT_SHALLOW has to go with it -- it fetches only a ref tip and cannot satisfy a full SHA -- and UPDATE_DISCONNECTED is now safe since the ref is immutable. Stop promising three binaries in the README: BUILD_LAUNCHER=AUTO drops the launcher when Qt6 is absent, leaving two.
innoextract was already behind BUILD_LAUNCHER -- it lives in src/launcher/CMakeLists.txt, which is only added when the launcher is built -- so the separate BUILD_INNOEXTRACT flag only ever chose between a working "install from archive" and a broken one. There is no usable system innoextract to fall back on: every release is 1.9 (2020), which stops at Inno Setup 6.0.5, while DaggerfallSetup is built with 6.6.x, so a packaged innoextract just refuses the file. That makes it a hard dependency of the launcher rather than an option, so build it always and drop the flag. Revisit once 6.4-6.6 support is released upstream and distros carry it. With a capable innoextract guaranteed, the Wine fallback has nothing left to do, so remove it -- along with the throwaway WINEPREFIX handling, the prefix-to-ARENA2 copy and the now-unused QProcessEnvironment include -- and rewrite the failure guidance that pointed at it. Lower the Qt requirement from 6.5 to 6.2. 6.5 was an arbitrary floor: the launcher uses nothing newer than Qt 6.0, and ubuntu-latest ships 6.4.2, so CI could not configure the launcher at all. Fix two libc++ build errors in archive.cpp. Mixing pos_type (fpos<mbstate_t>) with streamsize is ambiguous under libc++ though libstdc++ accepts it; convert through off_type explicitly. Define GL_SILENCE_DEPRECATION on macOS. OpenGL has been deprecated there since 10.14 and OSG's headers pull it into every translation unit that touches osg/GL, burying real warnings under hundreds of notices. Install the Boost and liblzma innoextract needs on all three CI platforms, and bump the vcpkg cache key to match.
ccache already covered the innoextract the launcher builds -- FetchContent puts it under build/_deps in the same tree, so CMAKE_CXX_COMPILER_LAUNCHER applies to its 50 objects too -- but it only hit when the build path was identical, because CMake bakes absolute paths into some generated sources. Set CCACHE_BASEDIR and CCACHE_NOHASHDIR so the hash uses relative paths. Measured over two builds in different directories, that takes the second build from 40/91 hits to 91/91. Raise max-size to 1G: the engine, the launcher and innoextract now share one cache, and 500M was sized for the engine alone. Cache the macOS MyGUI install tree as well. It was rebuilt from source on every run (~48s); the key carries the tag, so bumping MyGUI3.4.4 misses and rebuilds on its own.
MyGUI 3.4.3 changed DataManager::getDataPath from returning const std::string& to returning std::string by value. We only matched the newer signature, so building against 3.4.2 -- what ubuntu-latest ships -- failed with a conflicting return type. Declare whichever the headers we compile against use, keyed off MYGUI_VERSION. The pre-3.4.3 signature hands back a reference, and the implementation builds its result locally, so that branch keeps the string in a member rather than returning a dangling reference to a temporary. The earlier libc++ fix in archive.cpp was incomplete: it wrapped the result of `newPos - mStart` but left the subtraction itself mixing fpos<mbstate_t> with streamsize, which is the ambiguity clang reports. Keep newPos as off_type for the whole function instead, so no fpos arithmetic happens at all, and fix two more instances of the same pattern: mEnd - tellg() in underflow(), and the fpos-to-size_t conversion in the MyGUI data stream's size(). Verified both getDataPath branches compile by building against the real 3.4.3 headers and against a 3.4.2 tree, and confirmed each selects the intended branch.
Mesa's glext.h declares every GL enum regardless of which version is actually available, so these names resolve on Linux. macOS's OpenGL/gl.h stops at 2.1 and ships no equivalent, so GL_RED_INTEGER was simply undeclared there and the build failed on it. Fall back to osg/Texture's EXT spelling of GL_RED_INTEGER where it exists, and to the standard value otherwise. Also cover pipeline.cpp, which the macOS build had not reached yet: it uses GL_RGBA16F, GL_DEPTH_STENCIL, GL_DEPTH32F_STENCIL8 and GL_FLOAT_32_UNSIGNED_INT_24_8_REV, none of which OSG declares, so all four would have failed the same way on the next run. Values checked against glcorearb.h. Every definition is behind #ifndef, so platforms whose headers already declare these keep using them.
FindMyGUI locates MyGUI.h and reports the directory holding the headers themselves, .../include/MYGUI. Our sources include them prefixed, as <MYGUI/MyGUI_RenderManager.h>, which needs the parent .../include on the search path as well. On Linux that parent is /usr/include, which every compiler searches anyway, so the omission never showed. macOS builds MyGUI from source into a private prefix, where nothing is implicit, and every prefixed include failed to resolve. Verified by installing MyGUI to a non-system prefix and building against it, which reproduces the macOS failure and now succeeds.
The Windows job was spending hours in `vcpkg install` before reaching configure. The cause is osg's default features: plugins, openexr, nvtt and fontconfig exist for GIS and film formats, and pull in gdal, hdf5, netcdf-c, proj, openssl, libpq, geos, libspatialite and the rest. The transitive closure is 119 ports with them and 68 without -- all compiled from source whenever the cache misses. opendf needs none of it. The only images OSG loads for us are the three PNGs in the MyGUI skin; Daggerfall's own art comes out of the BSAs through our own loader. So ask for osg[core] plus freetype and libpng explicitly. Also stop throwing the work away. actions/cache only saves at the end of a successful job, so a run that timed out mid-install cached nothing and the next one started from zero. Split it into restore + an always() save, and cap the job at 90 minutes so a stuck run fails fast instead of holding a runner for the default six hours.
Building the Windows dependencies with vcpkg meant roughly 119 ports compiled from source -- osg's default features alone pull in gdal, hdf5, netcdf-c, proj, openssl and friends. It ran for hours without reaching the configure step, and because actions/cache only saves after a successful job, each timed-out run cached nothing and the next started over. OpenMW builds on OSG and MyGUI just as we do and hit the same wall. They solved it by publishing a prebuilt x64-windows vcpkg tree, so use that instead of compiling our own. The archive carries everything we need: openmw-osg (their OSG fork, which we want), MyGUI 3.4.3, SDL2, and the four Boost components plus lzma that innoextract needs. 89 MB, seconds to fetch. Pinned by release tag and verified against the SHA512 they publish, so the build is reproducible and a tampered or truncated download fails the job rather than silently building against whatever arrived. Verified by downloading the archive and checking its contents: the checksum matches, scripts/buildsystems/vcpkg.cmake sits where the toolchain option points, all four OSG components opendf links are present, and MyGUI ships without MyGUI_Config.h at 3.4.3 -- so our AUTO detection correctly defines MYGUI_DONT_USE_OBSOLETE, matching how vcpkg's port builds it.
The prebuilt tree got Windows as far as compiling, where innoextract stopped on a missing boost/ptr_container/ptr_map.hpp. OpenMW has no use for that library, so their tree does not carry it. Checked every Boost header innoextract includes against the archive rather than fixing one error per run: ptr_container and format are the only two absent. Both are header-only, so drop the headers in beside the rest instead of pulling vcpkg back in to build them. Taken at the boost-1.90.0 tag to match the Boost already in the tree; vcpkg's current ports are 1.92, and mixing two Boost versions in one include tree invites subtle breakage. Verified by extracting both tarballs, confirming the copy puts boost/ptr_container/ptr_map.hpp exactly where the compiler looked, and compiling a translation unit that instantiates boost::ptr_map and boost::format against the result.
The previous commit added the headers but they never reached the compiler. The step was gated on a dependency-cache miss, and the cache had been written by an earlier run -- before the step existed. So the restore hit, the step was skipped, and the build failed on the very headers it adds. Two fixes, either of which is sufficient, because this is easy to trip over again: Version the cache key. It previously tracked only OpenMW's release tag, but what we cache is their archive plus our own overlay, so the key has to change when the overlay does. Make the step idempotent and drop the cache-miss condition. It now runs every time and skips per-library only when the headers are already there, so a tree cached without them repairs itself instead of failing forever.
MSVC's build died in a cascade of errors inside <xutility>, all traced back to the console's tab-completion calling std::mismatch on MyGUI::UString::iterator. Those iterators have no usable std::iterator_traits: _base_iterator declares no iterator_category, value_type, pointer or reference, and its difference_type is protected. C++17 requires all of them for an iterator passed to a standard algorithm. libstdc++ never instantiates the traits here so it compiled by luck; MSVC's does, and cannot. Compute the longest common prefix with an indexed loop instead, which needs nothing of the iterators beyond operator[] and erase. Checked against the original over 81 combinations of prefixes, divergent strings and empty inputs: identical results in every case. This is the only place we hand UString iterators to the standard library.
MSVC stopped on std::to_string in sparsearray.hpp, which includes <stdexcept> but not <string>. libstdc++ drags <string> in through other headers, so it built everywhere else; MSVC's standard library does not, and the failed to_string call took the std::out_of_range construction around it down too. Rather than fix the one file and wait for the next run to find the next one, audit the tree for symbols used without their defining header: <string> 6 files using std::to_string <stdexcept> 9 files throwing out_of_range / runtime_error <algorithm> 4 files calling std::min / std::max <memory> 6 files using std::unique_ptr All are real uses, not comments. Both affected public headers now compile standalone.
Caught in review, and a real ABI bug rather than a style point. MyGUI 3.4.4 defaults MYGUI_DONT_USE_OBSOLETE to FALSE and installs no MyGUI_Config.h, so opendf's AUTO detection correctly reads it as a pre-3.5 install and defines the macro -- while the library itself was built without it. That is the exact widget-layout disagreement the AUTO logic exists to prevent, and it corrupts memory rather than failing to link. Pass the flag when building MyGUI, in CI and in the README recipe, and version the MyGUI cache key so a tree built with the old options isn't restored over the fix. Also from review: pin gha-setup-ninja to v6 by SHA rather than tracking the mutable @master, and set persist-credentials: false on checkout since no step pushes back to the repository.
Modernize CMake and improve const-correctness
…aints via legacy GL 2.1 + `#version 120` shaders + MyGUI FetchContent + BGR/BGRA texture fallback.
MacOS Rendering
Both terrain shaders declared `flat varying uint TexIndex`, which Apple's GLSL 1.20 compiler rejects with a syntax error even though it advertises GL_EXT_gpu_shader4 and accepts usampler2D, texelFetch2D and gl_InstanceIDARB from that same extension. Probing each construct separately showed the uint varying is the only one it refuses. TexIndex is a float now -- still flat, so it is not interpolated, and a float carries tile indices exactly. Terrain had never rendered on macOS: neither shader compiled, so the ground outside a dungeon was black.
kcat
reviewed
Sep 1, 2026
| { | ||
| // The table is binary-searched, so a mis-ordered entry would silently stop | ||
| // matching rather than fail loudly. | ||
| assert(std::is_sorted(std::begin(sEmissiveFlats), std::end(sEmissiveFlats))); |
Owner
There was a problem hiding this comment.
With C++20, this can be checked at compile time. Making sEmissiveFlats constexpr, and std::is_sorted is constexpr in C++20, so you could do
static_assert(std::ranges::is_sorted(sEmissiveFlats));
It's probably worth enabling C++20, if not 23.
Alternatively, this could be done once at load time instead of when querying a flat being emissive, by putting something like this in the anonymous namespace:
auto const dummy = std::invoke([] { assert(...); return 0; });
Contributor
Author
There was a problem hiding this comment.
I wanted C++17, do you want me to update the project to C++20?
kcat
reviewed
Sep 1, 2026
| @@ -1,2 +1,3 @@ | |||
| *.kdev4 | |||
| build | |||
| CLAUDE.md | |||
Owner
There was a problem hiding this comment.
I should probably have something about this somewhere, but has this been written with AI? I don't think I want to support or accept AI generated code even if it's been looked over or touched up by hand. Aside from any moral issues, this can create a serious copyright risk without knowing all training code is from clean sources that can be used in a GPL3 project, and also not scraped from some illegitimate sources (e.g. leaks from proprietary code bases), that can poison the project.
Contributor
Author
There was a problem hiding this comment.
It's your repo and project, while I don't hold the same moral problems with AI as likely you do, I'll respect what you wish here.
Anyway, if you'd like to revert or git history re-write to purge my PRs, no worries.
Contributor
Author
There was a problem hiding this comment.
@kcat close or merge? :)
I don't agree with Codeberg's stance at all.
I think Debian made a reasonable decision:
https://www.phoronix.com/news/Debian-Votes-Responsible-AI-Use
Same with Linus as well:
https://www.reddit.com/r/sysadmin/comments/1uy08tz/linus_torvalds_puts_the_foot_down_against_antiai/
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We now have light flickering. :)
New Features
-set.relightcommand.Bug Fixes
Compatibility