From 6801bdf714c134fcca84cca326b050293787eb1f Mon Sep 17 00:00:00 2001 From: UnknownException <25252012+UnknownException@users.noreply.github.com> Date: Sun, 7 Jun 2026 10:13:18 +0200 Subject: [PATCH] Improve OpenGL texture handling and fix some regressions Fix not using the index parameter in Palette::GetEntries Fix boundary checking in Surface::Blt Improve stack traversing Reuse OpenGL textures where possible to avoid re-allocating textures every frame --- README.md | 4 +-- subtitans/iddraw4.h | 2 +- subtitans/openglrenderer.cpp | 59 +++++++++++++++++++++------------- subtitans/palette.cpp | 14 +++++++- subtitans/pescalpel.cpp | 4 +-- subtitans/softwarerenderer.cpp | 2 +- subtitans/subtitans.cpp | 23 +++++++++---- subtitans/surface.cpp | 51 ++++++++++++++++++----------- subtitans/surface.h | 4 +-- 9 files changed, 107 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index fbabfd0..4732198 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ ## Question and Answers ### How to update Submarine Titans to version 1.1? -> Window: https://steamcommunity.com/sharedfiles/filedetails/?id=2129291420 \ +> Windows: https://steamcommunity.com/sharedfiles/filedetails/?id=2129291420 \ > Linux: See [PROTON-v1_0-v_1_1.md](PROTON-v1_0-v1_1.md) ### Why am I getting MSVCP140.dll errors? @@ -59,7 +59,7 @@ ### How do I use the mission skip cheat? > Load the mission you want to skip and write 'orbiton' without quotes in the chatbox. \ -> Exit to the menu and select a random campaign to start the next mission. \ +> Exit to the menu and select a random campaign to start the next mission. ### How do I uninstall the patch? > Deleting SubTitans.dll will disable the in-game patches. diff --git a/subtitans/iddraw4.h b/subtitans/iddraw4.h index 84b954e..3aa4713 100644 --- a/subtitans/iddraw4.h +++ b/subtitans/iddraw4.h @@ -30,7 +30,7 @@ namespace DDraw { uint32_t colorKeyCaps; uint32_t effectCaps; uint32_t effectAlphaCaps; - uint32_t palleteCaps; + uint32_t paletteCaps; uint32_t stereoVisionCaps; uint32_t alphaBltConstantBitDepth; uint32_t alphaBltPixelBitDepth; diff --git a/subtitans/openglrenderer.cpp b/subtitans/openglrenderer.cpp index 793d2e8..21244b1 100644 --- a/subtitans/openglrenderer.cpp +++ b/subtitans/openglrenderer.cpp @@ -322,7 +322,7 @@ uint32_t CreateShader(const char* fragmentShader) char errorBuffer[1024 - 16]; glGetShaderInfoLog(vertexShaderId, sizeof(errorBuffer), NULL, errorBuffer); - GetLogger()->Error(errorBuffer); + GetLogger()->Error("%s", errorBuffer); return 0; } @@ -337,7 +337,7 @@ uint32_t CreateShader(const char* fragmentShader) char errorBuffer[1024 - 16]; glGetShaderInfoLog(fragmentShaderId, sizeof(errorBuffer), NULL, errorBuffer); - GetLogger()->Error(errorBuffer); + GetLogger()->Error("%s", errorBuffer); return 0; } @@ -353,7 +353,7 @@ uint32_t CreateShader(const char* fragmentShader) char errorBuffer[1024 - 16]; glGetProgramInfoLog(shaderProgramId, sizeof(errorBuffer), NULL, errorBuffer); - GetLogger()->Error(errorBuffer); + GetLogger()->Error("%s", errorBuffer); return 0; } @@ -445,6 +445,17 @@ void OpenGLRenderer::Run() glGenTextures(1, &surfaceTextureId); glGenTextures(1, &paletteTextureId); + glActiveTexture(GL_TEXTURE0); + glBindTexture(GL_TEXTURE_2D, surfaceTextureId); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + + glActiveTexture(GL_TEXTURE1); + glBindTexture(GL_TEXTURE_2D, paletteTextureId); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 256, 1, 0, GL_BGRA, GL_UNSIGNED_BYTE, nullptr); + glViewport(0, 0, Global::MonitorWidth, Global::MonitorHeight); uint32_t vboId = 0; @@ -459,6 +470,9 @@ void OpenGLRenderer::Run() glEnableVertexAttribArray(0); glEnableVertexAttribArray(1); + glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, 0); + glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 0, (void*)(12 * sizeof(float))); + GetLogger()->Informational("%s %s\n", __FUNCTION__, "is running"); InitImGui(); @@ -474,6 +488,7 @@ void OpenGLRenderer::Run() int32_t currentWidth = 0; int32_t currentHeight = 0; int32_t currentBitsPerPixel = 0; + bool refreshGLTexture = false; Mutex.lock(); while (IsThreadRunning) @@ -483,42 +498,39 @@ void OpenGLRenderer::Run() uint32_t drawTime = timeGetTime(); // Clear - glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT); + glClear(GL_COLOR_BUFFER_BIT); if (surfaceBuffer) { - // Set images + // Upload the surface. Re-allocate only when the resolution or bit depth changed if (currentBitsPerPixel == 8 && paletteBuffer) { glActiveTexture(GL_TEXTURE0); glBindTexture(GL_TEXTURE_2D, surfaceTextureId); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); - glTexImage2D(GL_TEXTURE_2D, 0, GL_R8UI, currentWidth, currentHeight, 0, GL_RED_INTEGER, GL_UNSIGNED_BYTE, surfaceBuffer); + if (refreshGLTexture) + glTexImage2D(GL_TEXTURE_2D, 0, GL_R8UI, currentWidth, currentHeight, 0, GL_RED_INTEGER, GL_UNSIGNED_BYTE, surfaceBuffer); + else + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, currentWidth, currentHeight, GL_RED_INTEGER, GL_UNSIGNED_BYTE, surfaceBuffer); glActiveTexture(GL_TEXTURE1); glBindTexture(GL_TEXTURE_2D, paletteTextureId); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); - glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 256, 1, 0, GL_BGRA, GL_UNSIGNED_BYTE, paletteBuffer); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 256, 1, GL_BGRA, GL_UNSIGNED_BYTE, paletteBuffer); } else if (currentBitsPerPixel == 32) { glActiveTexture(GL_TEXTURE0); glBindTexture(GL_TEXTURE_2D, surfaceTextureId); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); - glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, currentWidth, currentHeight, 0, GL_BGRA, GL_UNSIGNED_BYTE, surfaceBuffer); + if (refreshGLTexture) + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, currentWidth, currentHeight, 0, GL_BGRA, GL_UNSIGNED_BYTE, surfaceBuffer); + else + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, currentWidth, currentHeight, GL_BGRA, GL_UNSIGNED_BYTE, surfaceBuffer); glActiveTexture(GL_TEXTURE1); glBindTexture(GL_TEXTURE_2D, 0); } - glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, 0); - glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 0, (void*)(12 * sizeof(float))); - - glBindBuffer(GL_ARRAY_BUFFER, vertexBufferId); glDrawArrays(GL_TRIANGLE_FAN, 0, 4); + refreshGLTexture = false; } #ifdef _IMGUI_ENABLED @@ -552,7 +564,7 @@ void OpenGLRenderer::Run() continue; // Prevent flickering caused by palette changes - if (PrimarySurface->PrimaryInvalid) + if (!PrimarySurface->IsPrimaryValid) continue; // Can't render without palette @@ -567,6 +579,7 @@ void OpenGLRenderer::Run() RecalculateGLSurface(); RecalculateSurface = false; + refreshGLTexture = true; } else if (RecalculateSurface) { @@ -580,6 +593,7 @@ void OpenGLRenderer::Run() RecalculateGLSurface(); RecalculateSurface = false; + refreshGLTexture = true; } PrimarySurface->PrimaryDrawMutex.lock(); @@ -597,9 +611,6 @@ void OpenGLRenderer::Run() PrimarySurface->PrimaryDrawMutex.unlock(); - currentWidth = PrimarySurface->Width; - currentHeight = PrimarySurface->Height; - if (currentBitsPerPixel != PrimarySurface->BitsPerPixel) { // BBP switched @@ -616,8 +627,12 @@ void OpenGLRenderer::Run() glUniform1i(glGetUniformLocation(shaderProgram32BitId, "surface"), 0); } + + refreshGLTexture = true; } + currentWidth = PrimarySurface->Width; + currentHeight = PrimarySurface->Height; currentBitsPerPixel = PrimarySurface->BitsPerPixel; } Mutex.unlock(); diff --git a/subtitans/palette.cpp b/subtitans/palette.cpp index 81fb9eb..89ff7a8 100644 --- a/subtitans/palette.cpp +++ b/subtitans/palette.cpp @@ -55,12 +55,18 @@ uint32_t __stdcall Palette::GetEntries(uint32_t shouldBeZero, uint32_t index, ui return ResultCode::InvalidArgument; } + if (index > 255 || count > 256 - index) + { + GetLogger()->Error("%s %s index:%i count:%i\n", __FUNCTION__, "requested palette range is out of bounds", index, count); + return ResultCode::InvalidArgument; + } + Mutex.lock(); // BGR -> RGB for (uint32_t i = 0; i < count; ++i) { - const uint32_t pixel = RawPalette[i]; + const uint32_t pixel = RawPalette[index + i]; result[i] = (pixel & 0xFF00FF00) | ((pixel << 16) & 0x00FF0000) | ((pixel >> 16) & 0x000000FF); @@ -83,6 +89,12 @@ uint32_t __stdcall Palette::SetEntries(uint32_t shouldBeZero, uint32_t index, ui return ResultCode::InvalidArgument; } + if (index > 255 || count > 256 - index) + { + GetLogger()->Error("%s %s index:%i count:%i\n", __FUNCTION__, "requested palette range is out of bounds", index, count); + return ResultCode::InvalidArgument; + } + Mutex.lock(); // RGB -> BGR diff --git a/subtitans/pescalpel.cpp b/subtitans/pescalpel.cpp index e2c1b3a..231944a 100644 --- a/subtitans/pescalpel.cpp +++ b/subtitans/pescalpel.cpp @@ -65,7 +65,7 @@ static bool UpdateChecksum(const wchar_t* applicationPath, BYTE* base) return false; } - GetLogger()->Debug("Header sum: %i, Check sum: %i\n", &headerSum, &checkSum); + GetLogger()->Debug("Header sum: 0x%08X, Check sum: 0x%08X\n", headerSum, checkSum); IMAGE_DOS_HEADER* dos = (IMAGE_DOS_HEADER*)base; if (dos->e_magic != IMAGE_DOS_SIGNATURE) @@ -81,7 +81,7 @@ static bool UpdateChecksum(const wchar_t* applicationPath, BYTE* base) return false; } - GetLogger()->Debug("PE header check sum: %i\n", nt->OptionalHeader.CheckSum); + GetLogger()->Debug("PE header check sum: 0x%08X\n", nt->OptionalHeader.CheckSum); // Original PE contains a value of 0, GOG seems to contain an incorrect value? // For now we'll just skip updating the check sum... diff --git a/subtitans/softwarerenderer.cpp b/subtitans/softwarerenderer.cpp index fbd59cb..2b86d25 100644 --- a/subtitans/softwarerenderer.cpp +++ b/subtitans/softwarerenderer.cpp @@ -107,7 +107,7 @@ void SoftwareRenderer::Run() continue; // Prevent flickering caused by palette changes - if (PrimarySurface->PrimaryInvalid) + if (!PrimarySurface->IsPrimaryValid) continue; if (PrimarySurface->BitsPerPixel == 8) diff --git a/subtitans/subtitans.cpp b/subtitans/subtitans.cpp index fbadd54..2a622d8 100644 --- a/subtitans/subtitans.cpp +++ b/subtitans/subtitans.cpp @@ -49,7 +49,7 @@ static void StackTrace() while (ebpRegister != nullptr) { - if ((uint32_t)ebpRegister >= (uint32_t)stackHighLimit || (uint32_t)ebpRegister < (uint32_t)stackLowLimit) + if ((uint32_t)ebpRegister + 8 >= (uint32_t)stackHighLimit || (uint32_t)ebpRegister < (uint32_t)stackLowLimit) { GetLogger()->Informational("Address out of range!\n"); break; @@ -76,29 +76,38 @@ static void StackTrace() && _splitpath_s(modulePath, nullptr, 0, nullptr, 0, name, _MAX_FNAME, ext, _MAX_EXT) == 0) { modulePaths.push_back(std::make_pair(std::string(name) + ext, modulePath)); - GetLogger()->Informational("[%i] Absolute 0x%04X Relative 0x%04X (%s%s)\n", depth++, returnAddress, returnAddress - (uint32_t)hMod, name, ext); + GetLogger()->Informational("[%i] Absolute 0x%08X Relative 0x%08X (%s%s)\n", depth++, returnAddress, returnAddress - (uint32_t)hMod, name, ext); } else { - GetLogger()->Informational("[%i] Absolute 0x%04X Relative 0x%04X\n", depth++, returnAddress, returnAddress - (uint32_t)hMod); + GetLogger()->Informational("[%i] Absolute 0x%08X Relative 0x%08X\n", depth++, returnAddress, returnAddress - (uint32_t)hMod); } } else { - GetLogger()->Informational("[%i] Address 0x%04X\n", depth++, returnAddress); + GetLogger()->Informational("[%i] Address 0x%08X\n", depth++, returnAddress); } - ebpRegister = (uint32_t*)(*ebpRegister); - if (ebpRegister == nullptr) + uint32_t* nextEbp = (uint32_t*)(*ebpRegister); + if ((uint32_t)nextEbp <= (uint32_t)ebpRegister) + { + if (nextEbp == nullptr) + GetLogger()->Informational("Reached base of stack\n"); + else + GetLogger()->Informational("Possible stack corruption\n"); + break; } + ebpRegister = nextEbp; + } + GetLogger()->Informational("End of stack trace\n"); for (const auto& modulePath : modulePaths) { uint32_t checksum = File::CalculateChecksumA(modulePath.second.c_str()); - GetLogger()->Informational("%s CRC32 %04X\n", modulePath.first.c_str(), checksum); + GetLogger()->Informational("%s CRC32 %08X\n", modulePath.first.c_str(), checksum); } } diff --git a/subtitans/surface.cpp b/subtitans/surface.cpp index 80deb39..b430985 100644 --- a/subtitans/surface.cpp +++ b/subtitans/surface.cpp @@ -61,7 +61,7 @@ Surface::Surface(SurfaceDescription* description) { TRACELOG("%s (%i)\n", __FUNCTION__, Identifier); - PrimaryInvalid = false; + IsPrimaryValid = false; MemoryDeviceContext = std::make_pair(nullptr, nullptr); if (IsPrimary()) @@ -160,13 +160,19 @@ uint32_t FillColorBlt(Surface* surface, const RECT* destinationRect, const uint3 const int32_t destinationWidth = destinationRect->right - destinationRect->left; + if (surface->Width < destinationRect->right || surface->Height < destinationRect->bottom) + { + GetLogger()->Error("%s %s\n", __FUNCTION__, "attempted to fill outside of surface boundaries"); + return ResultCode::Ok; + } + if (surface->IsPrimary()) surface->PrimaryDrawMutex.lock(); if (surface->BitsPerPixel == 8 || fillColor == 0) { // Quick copy (Only if full width and starts at the upper left corner) - if (destinationRect->top == 0 && destinationWidth == surface->Width) + if (destinationRect->top == 0 && destinationRect->left == 0 && destinationWidth == surface->Width) { uint32_t size = destinationRect->bottom * surface->Stride; memset(surface->SurfaceBuffer, (uint8_t)fillColor, size); @@ -195,9 +201,7 @@ uint32_t FillColorBlt(Surface* surface, const RECT* destinationRect, const uint3 if (surface->IsPrimary()) { surface->PrimaryDrawMutex.unlock(); - - if (surface->PrimaryInvalid) - surface->PrimaryInvalid = false; + surface->IsPrimaryValid = true; } return ResultCode::Ok; @@ -250,15 +254,15 @@ uint32_t __stdcall Surface::Blt(RECT* destinationRect, IDDrawSurface4* sourceDDS } if (Width < destinationRect->right || Height < destinationRect->bottom - || (!colorFill && (sourceSurface->Width < sourceRect->right || Height < sourceRect->bottom))) + || sourceSurface->Width < sourceRect->right || sourceSurface->Height < sourceRect->bottom) { GetLogger()->Error("%s %s (%i)\n", __FUNCTION__, "attempted to write outside of surface boundaries", Identifier); #if _DEBUG GetLogger()->Informational("Src rect: l:%i t:%i r:%i b:%i (%ix%i@%ibpp)\nDst rect: l:%i t:%i r:%i b:%i (%ix%i@%ibpp)\n" , sourceRect->left, sourceRect->top, sourceRect->right, sourceRect->bottom - , colorFill ? -1 : sourceSurface->Width - , colorFill ? -1 : sourceSurface->Height - , colorFill ? BitsPerPixel : sourceSurface->BitsPerPixel + , sourceSurface->Width + , sourceSurface->Height + , sourceSurface->BitsPerPixel , destinationRect->left, destinationRect->top, destinationRect->right, destinationRect->bottom, Width, Height, BitsPerPixel ); #endif @@ -309,9 +313,7 @@ uint32_t __stdcall Surface::Blt(RECT* destinationRect, IDDrawSurface4* sourceDDS if (IsPrimary()) { PrimaryDrawMutex.unlock(); - - if (PrimaryInvalid) - PrimaryInvalid = false; + IsPrimaryValid = true; } return ResultCode::Ok; @@ -453,6 +455,8 @@ uint32_t __stdcall Surface::Lock(RECT* rect, DDraw::SurfaceDescription* desc, ui TRACELOG("%s (%i)\n", __FUNCTION__, Identifier); uint32_t result = GetSurfaceDesc(desc); + if (result != ResultCode::Ok) + return result; if (IsPrimary()) PrimaryDrawMutex.lock(); @@ -491,8 +495,13 @@ uint32_t __stdcall Surface::SetClipper(IDDrawClipper* clipper) if (AttachedClipper) AttachedClipper->Release(); - AttachedClipper = (Clipper*)clipper; - AttachedClipper->AddRef(); + AttachedClipper = nullptr; + + if (clipper != nullptr) + { + AttachedClipper = (Clipper*)clipper; + AttachedClipper->AddRef(); + } return ResultCode::Ok; } @@ -510,13 +519,19 @@ uint32_t __stdcall Surface::SetPalette(IDDrawPalette* palette) if (AttachedPalette) AttachedPalette->Release(); - AttachedPalette = (Palette*)palette; - AttachedPalette->AddRef(); + AttachedPalette = nullptr; + + if (palette != nullptr) + { + AttachedPalette = (Palette*)palette; + AttachedPalette->AddRef(); + } if (IsPrimary()) + { PrimaryDrawMutex.unlock(); - - PrimaryInvalid = true; + IsPrimaryValid = false; + } return ResultCode::Ok; } diff --git a/subtitans/surface.h b/subtitans/surface.h index d68385e..267109b 100644 --- a/subtitans/surface.h +++ b/subtitans/surface.h @@ -79,9 +79,9 @@ class Surface : public DDraw::IDDrawSurface4 { uint8_t* SurfaceBuffer; std::mutex PrimaryDrawMutex; - std::atomic_bool PrimaryInvalid; + std::atomic_bool IsPrimaryValid; Palette* AttachedPalette; - bool IsPrimary() const { return (Description.flags & DDraw::SurfaceDescriptionFlag::Caps) && (Description.caps.caps[0] & DDraw::SurfaceCapsFlag::Primary); } + const bool IsPrimary() const { return (Description.flags & DDraw::SurfaceDescriptionFlag::Caps) && (Description.caps.caps[0] & DDraw::SurfaceCapsFlag::Primary); } }; \ No newline at end of file