pthom / hello_imgui

Hello, Dear ImGui: unleash your creativity in app development and prototyping
https://pthom.github.io/hello_imgui
MIT License
604 stars 91 forks source link

adding generated files to assets directory? #86

Closed wkjarosz closed 5 months ago

wkjarosz commented 5 months ago

What is the correct way to add generated files to the assets directory, and have HIM pick up these changes and copy them over into the appropriate build-time assets location (e.g. inside the Resources folder of an app bundle, or the disk image for emscripten)?

There are several use-cases, but the one I'm struggling with now is precompiling metal shaders into metallib files, which I'd like to store in the bundle's assets.

I can precompile them using a custom command like:

add_custom_command(
      OUTPUT ${shader_lib}
      DEPENDS ${shader_source}
      COMMAND xcrun -sdk macosx metal -std=osx-metal2.0 -O3 "${shader_source}" -o "${shader_lib}"
      WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
      COMMENT "Compiling Metal shader \"${shader}\" to MLTB library \"${shader}lib\"."
      VERBATIM
    )

but even if the OUTPUT ${shader_lib} is placed into the assets directory that HIM is informed about, HIM doesn't know that new files have been placed there upon building.

Thanks

pthom commented 5 months ago

Hello,

Actually, there are 3 distinct moments when assets can be deployed:

Let's see how it is done in HelloImGui:

Main assets logic

The main part of the assets logic is inside hello_imgui_cmake/assets/:

image

The main file, hello_imgui_assets.cmake will include platform dependent variations for hello_imgui_bundle_assets_from_folder():

# Platform dependent definition of hello_imgui_bundle_assets_from_folder
if (EMSCRIPTEN)
    include(${CMAKE_CURRENT_LIST_DIR}/him_assets_emscripten.cmake)
elseif(IOS OR (MACOSX AND (NOT HELLOIMGUI_MACOS_NO_BUNDLE)))
    include(${CMAKE_CURRENT_LIST_DIR}/him_assets_apple_bundle.cmake)
elseif(ANDROID)
    include(${CMAKE_CURRENT_LIST_DIR}/him_assets_android.cmake)
else()
    include(${CMAKE_CURRENT_LIST_DIR}/him_assets_desktop.cmake)
endif()

The emscripten variation will do this:

# Bundle assets / emscripten version
function(hello_imgui_bundle_assets_from_folder app_name assets_folder)
    if (IS_DIRECTORY ${assets_folder})                 
        target_link_options(
            ${app_name} 
            PRIVATE
            "SHELL:--preload-file ${assets_folder}@/"
        )
    else()
        message(WARNING "hello_imgui_bundle_assets_from_folder: ignoring missing folder ${assets_folder}")
    endif()

    if (HELLOIMGUI_ADD_APP_WITH_INSTALL)
        hello_imgui_get_real_output_directory(${app_name} real_output_directory)
        install(FILES
            ${real_output_directory}/${app_name}.html
            ${real_output_directory}/${app_name}.data
            ${real_output_directory}/${app_name}.js
            ${real_output_directory}/${app_name}.wasm
            DESTINATION ${CMAKE_INSTALL_PREFIX}
        )
    endif()
endfunction()

target_link_options populates the disk image for emscripten, but since this is provided by emscripten, there is no way to change when it is done. For emscripten, this is done during CMake configure. The install part will only happen during the install step. I guess it is not important here.

Copying custom resources

hello_imgui_cmake/emscripten/hello_imgui_emscripten.cmake will copy js and css resources from assets/app_settings/emscripten, but those are outside of the disk image.


As a conclusion, you should

pthom commented 5 months ago

PS (unrelated, and absolutely not urgent): I saw that once used a PImpl. If you have time someday, and if you have time I would appreciate your inputs if you could take some time to look at a PImpl generator I wrote (I had this idea when developping the automatic python bindings generator for ImGui Bundle).

It is available here: https://pthom.github.io/litgen/litgen_book/20_10_00_pimpl_my_class.html

And you can run it online here: https://mybinder.org/v2/gh/pthom/litgen/main?urlpath=lab/tree/litgen-book/20_15_00_pimpl_online.ipynb

wkjarosz commented 5 months ago

I've read the documentation of your pimpl generator, though haven't run it. Here are some thoughts:

Minor tidbit: I typically don't pollute my global namespace with the MyClassPImpl class declaration. This declaration can be put into the private block within MyClass.

I wouldn't want to introduce an extra stage in my development process to make this work, but I assume your python script can be packaged up in some cmake commands to re-generate these things on rebuild if the MyClassPImpl is modified.

I think even with those wrappers, I'm not sure I would have a need to use this since my use of PImpl has typically required relatedly little boilerplate compared to how you have structured things. I treat the MyClassPImpl as pure data, with all members public (I actually use a struct) and with no methods (beyond perhaps a constructor and destructor). That means any public methods of MyClass just need to use m_pimpl->member_variable in place of a typical m_member_variable. Any required private functions can still be left out of the MyClass header, and can be implemented as free (static) functions in myclass.cpp which take a pointer to the MyClassPImpl as the first parameter. If you write MyClass from the beginning knowing it will use PImpl, then the only boilerplate is forwarding function calls, but I find this to be a relatively minor inconvenience to write once.

Now, if you don't write MyClass with PImpl in mind from the beginning, then it can be quite some work to refactor it once you decide you want pimpl . A one-time script that would take a non-pimpled header and implementation, and convert it to pimpl (with all private members being wrapped in a pimpl struct, and private methods moved to static free functions) would be something I may use. But that is probably harder to get right.

FYI: I first learned about PImpl when working on OpenEXR 20 years ago, and this is how that codebase did things. You can take a look at its codebase if you are interested.

wkjarosz commented 5 months ago

Thanks for the detailed reponse.

As a conclusion, you should

  • replace add_custom_command by something that is done at configure time, and before the call to hello_imgui_add_app so that assets can be populated correctly. See below, for possible solutions
  • run cmake whenever you want to get newly built shaders

Two solutions are possible, I guess:

  • run a script that precompiles the shaders and places them in the assets directory before running CMake
  • call execute_process instead of add_custom_command, because this one is called during configure

I would prefer to have everything happen via cmake so I don't require running another script. However, I'd like to be able to edit the metal shader source files, and just perform a standard rebuild, without remembering to manually clean and reconfigure. Do you know if there is a way to accomplish this (if shader sources are modified (or generated metallib files change), to inform cmake to rerun the add_custom_command or execute_process before HIM grabs things from the assets directory?).

I had a somewhat simpler use-case before (you can check SamplinSafari) that I managed to get to work: there was one dependency which I grab via cpm.cmake which contained a data file that I wanted to use as an asset. Since the data is from the dependency, I didn't want to copy it into my source asset directory, so I instead copied all asset files into an assets directory within the build tree and then pass ASSETS_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/assets to hello_imgui_add_app.

The problem was that copying using the file command only happened once, and if I changed other things in the source asset directory (like a shader) it was not re-copied to inform HIM before it does its asset processing.

I was able to resolve this by using configure_file with COPYONLY instead of the file command:

# copy asset directory to the build tree file
file(
  GLOB_RECURSE MY_RESOURCE_FILES
  RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
  # CONFIGURE_DEPENDS
  ${CMAKE_CURRENT_SOURCE_DIR}/assets/**.*
)

foreach(MY_RESOURCE_FILE ${MY_RESOURCE_FILES})
  configure_file(
    ${CMAKE_CURRENT_SOURCE_DIR}/${MY_RESOURCE_FILE} ${CMAKE_CURRENT_BINARY_DIR}/${MY_RESOURCE_FILE} COPYONLY
  )
endforeach()

since this introduces a dependency so that cmake knew to re-copy my assets directory before running HIM's asset processing.

The situation I have now is only slightly different: some of the files are generated by compiling metal shaders, but I'd still like cmake to know to automatically rerun that part of configure (before HIM processes assets) if anything changes.

Suggestions?

pthom commented 5 months ago

Thanks for your inputs on PImpl, I will take them into account, and actually I do prefer your way of using a data only impl.

pthom commented 5 months ago

I just pushed a commit that updates assets at build time for Linux and Windows.

On macOS and emscripten, IMHO everything was working find and still works fine: you just need to add, a line similar to this, so that your app detect the newly compiled asset file.

set_property(SOURCE your_app_main_file.cpp APPEND PROPERTY OBJECT_DEPENDS your_compiled_shader_inside_assets)

Demo (50" video)

pthom commented 5 months ago

Unrelated: I just added dedicated pages for the Hello ImGui documentation, which was a bit raw up until now.

wkjarosz commented 5 months ago

The set_property command does indeed work for me. Thanks. As far as I'm concerned this issue is resolved so feel free to close.