bkaradzic / bgfx.cmake

CMake build scripts for bgfx. Released under public domain.
Creative Commons Zero v1.0 Universal
210 stars 111 forks source link

Improve shader profile extension to be more compatible with bgfx #247

Closed Andrewfeng123 closed 1 week ago

Andrewfeng123 commented 1 month ago

Currently, the profiles spirv and metal are abbreviated to spv and mtl, which means the compiled shaders are in the directories spv and mtl. This is different from the convention used in bgfx (see bgfx/examples/runtime), which leaves them as spirv and metal. Conforming to the bgfx has an added benefit: it is now possible to directly use the loadShader and loadProgram functions defined in bgfx_utils (in bgfx/examples/common/) just like the example programs. This was not possible before since the extensions spirv and metal were hard-coded into bgfx_utils.

Example:

In the CMakeLists.txt at the root of the project:

bgfx_compile_shaders(
    TYPE VERTEX
    SHADERS ${CMAKE_SOURCE_DIR}/shaders/vs.sc
    VARYING_DEF ${CMAKE_SOURCE_DIR}/shaders/varying.def.sc
    INCLUDE_DIRS "${CMAKE_SOURCE_DIR}/bgfx.cmake/bgfx/examples/" "${CMAKE_SOURCE_DIR}/bgfx.cmake/bgfx/src/"
    OUTPUT_DIR ${CMAKE_BINARY_DIR}/shaders
)

bgfx_compile_shaders(
    TYPE FRAGMENT
    SHADERS ${CMAKE_SOURCE_DIR}/shaders/fs.sc
    VARYING_DEF ${CMAKE_SOURCE_DIR}/shaders/varying.def.sc
    INCLUDE_DIRS "${CMAKE_SOURCE_DIR}/bgfx.cmake/bgfx/examples/" "${CMAKE_SOURCE_DIR}/bgfx.cmake/bgfx/src/"
    OUTPUT_DIR ${CMAKE_BINARY_DIR}/shaders
)

add_executable(main src/main.cpp 
    shaders/vs.sc 
    shaders/fs.sc
)

In main.cpp:

// ...
#include "bgfx_utils.h"
// ...
bgfx::ProgramHandle m_program = loadProgram("vs.sc", "fs.sc");
bwrsandman commented 1 month ago

Dx9 has been dropped. Can you take this opportunity to remove it?

bwrsandman commented 1 month ago

Unfortunately the changes break embedded shaders with BGFX_EMBEDDED_SHADER when using bgfx_compile_shaders with AS_HEADERS:

error: ‘vs_line_spv’ was not declared in this scope; did you mean ‘vs_line_spirv’?
[build]    97 |     BGFX_EMBEDDED_SHADER(vs_line), BGFX_EMBEDDED_SHADER(vs_line_instanced),        

image

The headers generate a variable with this naming:

static const uint8_t vs_line_spirv[924] =

but the BGFX_EMBEDDED_SHADER macro expects vs_line_spv

Andrewfeng123 commented 1 month ago

@bwrsandman Okay I pushed a new commit, please let me know if this works. This is a bit of a hack, but I don't see another way of doing so without changing bgfx.