Vulkan#156
Draft
boludoz wants to merge 21 commits into
Draft
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an SDL3-based Vulkan platform backend, including shader assets and native texture/raster streaming, plus a few supporting fixes/improvements in GL3 and image code.
Changes:
- Introduces Vulkan platform integration (driver plugin registration, raster implementation, shader bytecode/assets).
- Hooks Vulkan into engine initialization, device selection, and texture native streaming.
- Updates build tooling (CMake + Premake) to support Vulkan/SDL3, and includes a small DXT/image fix + GL3 adjustments.
Reviewed changes
Copilot reviewed 26 out of 33 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vulkan/vulkanshaders.inc | Adds embedded SPIR-V shader bytecode blobs for Vulkan pipelines. |
| src/vulkan/vulkanraster.cpp | Implements Vulkan native raster lifecycle, locking, mip levels, and native texture IO. |
| src/vulkan/vulkan.cpp | Registers Vulkan platform driver hooks with the engine. |
| src/vulkan/rwvulkan.h | Declares Vulkan platform API, context, and raster structures. |
| src/vulkan/generated_shaders/im2d.vert | Adds generated Vulkan GLSL vertex shader for Im2D. |
| src/vulkan/generated_shaders/im2d.frag | Adds generated Vulkan GLSL fragment shader for Im2D with alpha test. |
| src/vulkan/generated_shaders/color3d.vert | Adds generated Vulkan GLSL vertex shader for 3D colored/textured pipeline. |
| src/vulkan/generated_shaders/color3d.frag | Adds generated Vulkan GLSL fragment shader for 3D colored/textured pipeline. |
| src/vulkan/generated_shaders/color3d_ray.frag | Adds generated Vulkan GLSL fragment shader with ray query occlusion logic. |
| src/texture.cpp | Enables reading/writing Vulkan native textures via stream API (guarded by RW_VULKAN). |
| src/skin.cpp | Calls Vulkan skin init when RW_VULKAN is enabled. |
| src/matfx.cpp | Calls Vulkan MatFX init when RW_VULKAN is enabled. |
| src/rwbase.h | Adds PLATFORM_VULKAN and Vulkan raster plugin ID; sets RWDEVICE for Vulkan builds. |
| src/image.cpp | Fixes unaligned read in DXT5 decompression and corrects DXT1 flip loop pointers. |
| src/gl/gl3raster.cpp | Changes GL3 raster unlock texture upload/mipmap behavior. |
| src/gl/gl3device.cpp | Adds GL info logging helper; adjusts GLES beginUpdate handling; improves display enumeration and mode selection. |
| src/engine.cpp | Includes Vulkan header, registers Vulkan plugins, and selects Vulkan device when RW_VULKAN. |
| src/charset.cpp | Includes Vulkan header (likely for platform types). |
| src/base.cpp | Selects Vulkan platform id in RW_VULKAN builds. |
| src/CMakeLists.txt | Adds Vulkan sources/headers and Vulkan platform link dependencies; installs Vulkan header. |
| CMakeLists.txt | Adds VULKAN as selectable platform; adds SDL3 helper; updates install platform suffix. |
| premake5.lua | Adds Vulkan SDK option and Vulkan platforms; links SDL3/Vulkan appropriately. |
| skeleton/sdl3.cpp | Allows SDL3 skeleton main for Vulkan builds; sets fullscreen param. |
| skeleton/CMakeLists.txt | Ensures SDL_MAIN_HANDLED for Vulkan platform skeleton builds. |
| rw.h | Exposes Vulkan header from umbrella include. |
| README.cmake | Documents Android NDK + SDL3 + Vulkan CMake build invocation. |
| .gitignore | Ignores Vulkan-related build directories. |
Comments suppressed due to low confidence (3)
src/vulkan/vulkanraster.cpp:1
rwNew(...)allocation failure isn’t handled. If it returnsnil, the code immediately dereferencesnatras->levels(lines 78–80), leading to a crash. Add a null check after allocation and return0(and keepnatras->levels == nil) on failure.
src/vulkan/vulkanraster.cpp:1- In the
elsebranch,rasterFromImageuploads immediately even though the modified pixels are still in the caller’s locked buffer (raster->pixels) and have not been copied back intonatras->levels->levels[level].data(that copy only happens inrasterUnlockwhenLOCKWRITEis set). This can upload stale/previous content. A safer approach is: whenunlock == false, only markgpuDirty/gpuReadyand defer uploading untilrasterUnlock, or explicitly copy the updated pixels into the backing store before callingensureTextureUploaded.
src/vulkan/vulkanraster.cpp:1 - If
natras->levels == nil, this writes the mip sizes but does not write any mip data, producing an invalid/corrupt native texture chunk (andgetSizeNativeTexture()still counts the bytes). Either make this path impossible (error out) or (preferably) mirror the GL3 path by locking each mip level for read and streaming out the pixels regardless of whether a CPU backing store is currently present.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include "d3d/rwd3d9.h" | ||
| #include "gl/rwgl3.h" | ||
| #include "gl/rwwdgl.h" | ||
| #include "vulkan/rwvulkan.h" |
Comment on lines
237
to
+240
| d3d9::registerPlatformPlugins(); | ||
| wdgl::registerPlatformPlugins(); | ||
| gl3::registerPlatformPlugins(); | ||
| vulkan::registerPlatformPlugins(); |
Comment on lines
+552
to
559
| } else if(level == 0) { | ||
| glPixelStorei(GL_UNPACK_ALIGNMENT, 1); | ||
| glTexImage2D(GL_TEXTURE_2D, level, natras->internalFormat, | ||
| raster->width, raster->height, | ||
| 0, natras->format, natras->type, raster->pixels); | ||
| } | ||
| if(level == 0 && natras->autogenMipmap) | ||
| // if(level == 0 && natras->autogenMipmap) | ||
| glGenerateMipmap(GL_TEXTURE_2D); |
Comment on lines
2346
to
2349
| case DEVICEGETVIDEOMODEINFO: | ||
| if (n <= 0) | ||
| return 0; | ||
| rwmode = (VideoMode*)arg; | ||
| rwmode->width = glGlobals.modes[n].mode.w; | ||
| rwmode->height = glGlobals.modes[n].mode.h; |
madebr
reviewed
Jun 10, 2026
Comment on lines
+59
to
+61
| if(WIN32 AND MSVC AND (LIBRW_PLATFORM_GL3 OR LIBRW_PLATFORM_VULKAN)) | ||
| target_link_options(${TARGET} PRIVATE /ENTRY:mainCRTStartup) | ||
| endif() |
Contributor
There was a problem hiding this comment.
Why is this done? The linker usually selects a sane default. This forces a terminal.
Use WIN32 and WIN32_EXECUTABLE instead to select a terminal or not.
Contributor
There was a problem hiding this comment.
The default is already to use C runtime initialization. Adding this option is not needed.
Contributor
There was a problem hiding this comment.
Because Vulkan requires this.
Respectfully, that's bs. On Windows Vulkan is a collection 3rd party dll librararies that have its own crt.
madebr
reviewed
Jun 10, 2026
Comment on lines
+65
to
+73
| set(LIBRW_SDL3_SOURCE_DIR "" CACHE PATH "SDL3 source directory to add with add_subdirectory before find_package(SDL3)") | ||
| function(librw_ensure_sdl3) | ||
| if(NOT TARGET SDL3::SDL3 AND LIBRW_SDL3_SOURCE_DIR) | ||
| add_subdirectory("${LIBRW_SDL3_SOURCE_DIR}" "${CMAKE_BINARY_DIR}/_deps/SDL3" EXCLUDE_FROM_ALL) | ||
| endif() | ||
| if(NOT TARGET SDL3::SDL3) | ||
| find_package(SDL3 CONFIG REQUIRED) | ||
| endif() | ||
| endfunction() |
Contributor
There was a problem hiding this comment.
This does not belong here but in your master cmake project imho.
Contributor
There was a problem hiding this comment.
Or at the least, only do:
if(NOT TARGET SDL3::SDL)
find_package(SDL3 CONFIG REQUIRED)
endif()
The optional vendored SDL3 is unnecessary complex.
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
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.
No description provided.