batterycenter / embed

A CMake/C++20 library to embed resource files at compile time
Apache License 2.0
116 stars 4 forks source link

Add ability to embed generated files #10

Closed abuneri closed 1 month ago

abuneri commented 1 month ago

This is a very cool/handy project!

One workflow that would great to have support for is GLSL -> SPIR-V -> embedded SPIR-V. For example, I have this function:

function(compile_shader target_name shader_file_path)
    set(spv_file_path "${CMAKE_CURRENT_SOURCE_DIR}/${shader_file_path}.spv")
    add_custom_command(
        OUTPUT ${spv_file_path}
        COMMAND ${Vulkan_GLSLC_EXECUTABLE} -o ${spv_file_path} ${shader_file_path}
        DEPENDS ${shader_file_path}
        WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
    )

    set(spv_target "spv_${file_name}")
    add_custom_target(${spv_target}
        DEPENDS
            ${spv_file_path}
    )
    add_dependencies(${target_name} ${spv_target})
endfunction(compile_shader)

add_executable(app main.cpp)
compile_shader(app shaders/simple.vert)
compile_shader(app shaders/frag.vert)

Which will automatically re-compile any specified shaders if they have been changed when (re)building the app target.

What I wanted to do was then automatically embed the compiled shaders for direct use in the app target such that whenever I modify any of my shaders and rebuild the target, they automatically get re-compiled and then re-embedded with the updated compiler output:

auto vert_shader = b::embed<"shaders/simple.vert.spv">();
auto frag_shader = b::embed<"shaders/simple.frag.spv">();

I found the simplest way to do this was to just add the ability to optionally specify custom dependencies to ensure we don't attempt to embed until those dependencies are ready. So now the compile_shader function adds the following at the end:

file(RELATIVE_PATH spv_file_path_relative ${CMAKE_CURRENT_SOURCE_DIR} ${spv_file_path})
b_embed(${target_name} ${spv_file_path_relative} DEPENDENCIES ${spv_target})

To emulate the behaviour I wanted, the generatedfile example I added has an .zip archive with a single text file helloworld.txt in it that contains the text Hello, World!. I use the new DEPENDENCIES argument to embed the text file that CMake extracts via its cross platform tar sub-command.

HerrNamenlos123 commented 1 month ago

Hi @abuneri , Thank you for your interest in the project, as well as your pull request, it's always awesome if someone already tried to find a solution to a problem.

Please allow me some time until i can properly think through this, I have a lot on my mind currently

HerrNamenlos123 commented 1 month ago

Alright, so am I correct that your proposed change is to add a feature to add custom dependencies to the target, because that allows you to embed files that do not exist at configure time? And in order to prove that everything works, you also created an examples that demonstrates the feature?

HerrNamenlos123 commented 1 month ago

I do not see any issue with this change.

I will merge it and simplify/extend the example. It can be greatly simplified because you can directly pass the generated filename as a dependency instead of a TARGET (and CMake sees that the file it depends on is the OUTPUT of a command, so it calls the command), thus you don't need an extra target extractedfile where you attach the file to.

However, now that I think about it, it would actually be great to support both, so I will add both cases in the example.

HerrNamenlos123 commented 1 month ago

@abuneri It turns out maybe the change wasn't even necessary. The absolute minimal example for embedding a generated file looks like:

set(RESOURCES_DIR "${CMAKE_CURRENT_SOURCE_DIR}/resources")
set(ARCHIVE_FILE_PATH "${RESOURCES_DIR}/helloworld.zip")

add_example(generatedfile-direct)

add_custom_command(
    COMMAND ${CMAKE_COMMAND} -E tar xzf ${ARCHIVE_FILE_PATH} "helloworld.txt"
    DEPENDS ${ARCHIVE_FILE_PATH}
    OUTPUT "${RESOURCES_DIR}/helloworld.txt"
    WORKING_DIRECTORY ${RESOURCES_DIR}
)
b_embed(generatedfile-direct "resources/helloworld.txt")

DEPENDENCIES is not actually necessary, because the "resources/helloworld.txt" in b_embed() is already added to DEPENDS, this means if the file exists, it is loaded, and if the file does not exist, but is in the OUTPUT of a command, then that command gets executed beforehands. Only when the file does not exist, AND there is no command to create it, does it fail.

It turns out that the custom target had absolutely no impact, and it was only working because b_embed() directly saw the OUTPUT of the custom command. It did not actually care about the target. And there is no way it could care, because a target cannot have an output and only depending on it only means it is built before it, but has no impact on generated files.

Nevertheless, I will still keep the feature to additionally depend on a target. although I am not sure it even makes sense. And, I am renaming DEPENDENCIES to DEPENDS, to be consistent with the add_custom_command() function.

Please try if it works for you, if you simply try to embed the OUTPUT of the custom command, without any extra targets...

HerrNamenlos123 commented 1 month ago

@abuneri I think I am done with this now, I removed the DEPENDS feature again, as I do not think it is even useful, because it does not help for generating a file, and if you want the target to depend on another target, then it's not the job of b_embed() to achieve that.

The example from my comment above should work fine. I kept an example to prove it in the future. Maybe you did not know that you cannot only depend on targets, but you can also depend on files, which causes them to be generated on-demand.

Please try to use it this way for your project. If it does not work for another reason, feel free to re-open the issue or create a new one.

EDIT: And even if it wasn't beneficial in the end, thank you for writing such a good Pull Request especially with example. It is always great if someone goes the extra mile, I really apprechiate your efforts!

abuneri commented 1 month ago

Ah yeah, that's working for my scenario. Thanks for the help/simplification!