Polytonic / Glitter

Dead Simple OpenGL
http://polytonic.github.io/Glitter/
2.48k stars 418 forks source link

Shader Directory in glitter #20

Closed debangshuroy closed 8 years ago

debangshuroy commented 8 years ago

I cloned it and imported in Qt and it is working perfectly. Thank you for this great boilerplate. Can you add one more folder like shaders and change in CMakeLists.txt so that vertex shaders and fragment shaders are copied into build folder during building.

I found this in CMakeList.txt

file(GLOB PROJECT_SHADERS Glitter/Shaders/*.comp
                          Glitter/Shaders/*.frag
                          Glitter/Shaders/*.geom
                          Glitter/Shaders/*.vert)

A little more documentation on this will be helpful.

Polytonic commented 8 years ago
file(GLOB PROJECT_SHADERS Glitter/Shaders/*.comp
                          Glitter/Shaders/*.frag
                          Glitter/Shaders/*.geom
                          Glitter/Shaders/*.vert)

This is so shaders display in IDE sidebars.

I cloned it and imported in Qt and it is working perfectly. Thank you for this great boilerplate. Can you add one more folder like shaders and change in CMakeLists.txt so that vertex shaders and fragment shaders are copied into build folder during building.

I'm hesitant to add shaders to Glitter. This is a starting point for most people, and writing "your own shader class and loader" is one of those "OpenGL rites of passage." Everyone does it slightly differently. The point is to just get you started. If you can provide a convincing argument otherwise, I'm listening. :smile:

debangshuroy commented 8 years ago

My argument is as you included a Shaders folder in the source it would be nice if this folder is copied into the build directory during every build so that when I compile shaders in the runtime I will read from build directory not from source directory.

Currently I added a file copy in CMakeList like this

file(COPY ${PROJECT_SHADERS} DESTINATION ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/Shaders)

But I need to run CMake to actually copy the file. :smile_cat:

simphax commented 8 years ago

Use this to have it copy the shaders on each build! :smile:

add_custom_command(TARGET ${PROJECT_NAME} PRE_BUILD COMMAND ${CMAKE_COMMAND} -E copy_directory
${PROJECT_SOURCE_DIR}/Glitter/Shaders ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/Shaders)
Polytonic commented 8 years ago

My apologies -- I haven't had a chance to look at this until today.

@simphax that solution copies the shader files when cmake is run, not on every build. I use the same strategy in another project. To date, I have not been able to get PRE_BUILD to work as expected.

@debangshuroy if that behavior is okay with you, then you can use something like:

add_custom_command(TARGET Glitter PRE_BUILD
                   COMMAND ${CMAKE_COMMAND} -E copy_directory
                   ${PROJECT_SOURCE_DIR}/Glitter/Shaders/
                   $<TARGET_FILE_DIR:Glitter>/Shaders/)

Modify the paths as needed.

My argument is as you included a Shaders folder in the source it would be nice if this folder is copied into the build directory during every build so that when I compile shaders in the runtime I will read from build directory not from source directory.

As you can see, I have been struggling with this problem for some time now.

simphax commented 8 years ago

Ah!

My above solution works on Xcode but not Visual Studio, unless you "rebuild" or you mark the shader files as "Content" files. The below code works out of the box on both! :)

file(GLOB PROJECT_SHADERS_REL RELATIVE ${PROJECT_SOURCE_DIR} Glitter/Shaders/*.comp
                          Glitter/Shaders/*.frag
                          Glitter/Shaders/*.geom
                          Glitter/Shaders/*.vert)

foreach(shaderFile ${PROJECT_SHADERS_REL})
    configure_file(${PROJECT_SOURCE_DIR}/${shaderFile} ${CMAKE_BINARY_DIR}/${shaderFile} COPYONLY)
endforeach()
Polytonic commented 8 years ago

Does this work when using makefiles?

simphax commented 8 years ago

Yep :grinning: At least on Mac

simphax commented 8 years ago

For make we might want to do this too, so that the relative paths are always the same.

set_target_properties(${PROJECT_NAME} PROPERTIES
    RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/Bin)
set_target_properties(${PROJECT_NAME} PROPERTIES
    RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/Bin)
set_target_properties(${PROJECT_NAME} PROPERTIES
    RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/Bin)

and we can actually shorten the last part of my previous formula:

foreach(shaderFile ${PROJECT_SHADERS_REL})
    configure_file(${shaderFile} ${shaderFile} COPYONLY)
endforeach()
Polytonic commented 8 years ago

I'm reading through the CMake docs and it seems like it will rerun cmake if the COPYONLY flag is used. On large projects, having to regenerate the project file on each shader modification will likely not scale well.

If the input file is modified the build system will re-run CMake to re-configure the file and generate the build system again.

Is there an alternative solution? I must confess I am not particularly skilled with CMake.

simphax commented 8 years ago

True! This should work :smile:

add_custom_target(shaders_copy COMMAND ${CMAKE_COMMAND} -E copy_directory
                   ${PROJECT_SOURCE_DIR}/Glitter/Shaders/
                   $<TARGET_FILE_DIR:Glitter>/Shaders/)

add_dependencies(${PROJECT_NAME} shaders_copy)

If we would like to copy only changed files we might be able to do something like this:

add_custom_target(shaders_copy)

file(GLOB PROJECT_SHADERS_REL RELATIVE ${PROJECT_SOURCE_DIR} Glitter/Shaders/*.comp
                          Glitter/Shaders/*.frag
                          Glitter/Shaders/*.geom
                          Glitter/Shaders/*.vert)

foreach(shaderFile ${PROJECT_SHADERS_REL})
    add_custom_command(TARGET shaders_copy PRE_BUILD COMMAND ${CMAKE_COMMAND} -E copy_if_different
                   ${PROJECT_SOURCE_DIR}/${shaderFile}
                   ${CMAKE_BINARY_DIR}/${shaderFile})
endforeach()

add_dependencies(${PROJECT_NAME} shaders_copy)

I haven't tested the last one but the first snippet should work for both Xcode, VS and Makefile.

simphax commented 8 years ago

This could also be an alternative

file(GLOB PROJECT_SHADERS_REL RELATIVE ${PROJECT_SOURCE_DIR} Glitter/Shaders/*.comp
                          Glitter/Shaders/*.frag
                          Glitter/Shaders/*.geom
                          Glitter/Shaders/*.vert)

foreach(shaderFile ${PROJECT_SHADERS_REL})
    add_custom_command(OUTPUT ${shaderFile}
                       COMMAND ${CMAKE_COMMAND} -E copy ${PROJECT_SOURCE_DIR}/${shaderFile} ${CMAKE_BINARY_DIR}/${shaderFile}
                       MAIN_DEPENDENCY ${shaderFile})
endforeach()
Polytonic commented 8 years ago

Of the three, I like the first the most, mainly for its simplicity. However, with large numbers of shader files, copying the entire shaders directory may prove slow as well. The second and third approaches seem more complex, and I don't know if I want to rely on relative directory behavior. I've seen people do ... questionable file placement in the past.

I'll give the copy_directory approach a try over the weekend. If for whatever reason, I don't get to this by Sunday morning, feel free to bump this to remind me. :smile:

Polytonic commented 8 years ago

This snippet ...

add_custom_target(shaders_copy COMMAND ${CMAKE_COMMAND} -E copy_directory
                   ${PROJECT_SOURCE_DIR}/Glitter/Shaders/
                   $<TARGET_FILE_DIR:Glitter>/Shaders/)

add_dependencies(${PROJECT_NAME} shaders_copy)

Produces the following for me:

CMake Error: The inter-target dependency graph contains the following strongly connected component (cycle):
  "shaders_copy" of type UTILITY
    depends on "Glitter" (strong)
  "Glitter" of type EXECUTABLE
    depends on "shaders_copy" (strong)
At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies are allowed only among static libraries.

What am I missing?

simphax commented 8 years ago

Ah I'm sorry. It's probably because I threw the TARGET_FILE_DIR in there. You'd probably need to use the CMAKE_BINARY_DIR instead. Like so:

add_custom_target(shaders_copy COMMAND ${CMAKE_COMMAND} -E copy_directory
                   ${PROJECT_SOURCE_DIR}/Glitter/Shaders/
                   ${CMAKE_BINARY_DIR}/Glitter/Shaders/)

add_dependencies(${PROJECT_NAME} shaders_copy)
Polytonic commented 8 years ago

Sorry for the delay (American Thanksgiving)!

${CMAKE_BINARY_DIR}/Glitter/Shaders/

I suspect this will break in IDEs. Or at least, it breaks for me in Xcode since the executable is placed in $CMAKE_BINARY_DIR}/Debug/ so presumably file handling routines expect to open files in Debug/Shaders/. Correct me if I'm wrong?

simphax commented 8 years ago

No CMAKE_BINARY_DIR is normally the cmake "source dir" (the place from where you run cmake) . My above code will put the shaders in Build/Glitter/Shaders and the binary in Build/Glitter/Debug if built via Xcode and you ran cmake in the Build folder. The shader can be referenced as "../Shaders/shader.frag" for example. Compiling from a Makefile will put the binary in Build/Glitter though. To have all binaries end up in the same folder we could add this code, as I showed before.

set_target_properties(${PROJECT_NAME} PROPERTIES
    RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/Bin
    RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/Bin
    RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/Bin)
Polytonic commented 8 years ago

Hey, sorry for the delay. Just wanted to let you guys know I haven't forgotten about this and I'm going to look at this again tomorrow morning.

Polytonic commented 8 years ago

No CMAKE_BINARY_DIR is normally the cmake "source dir" (the place from where you run cmake) . My above code will put the shaders in Build/Glitter/Shaders and the binary in Build/Glitter/Debug if built via Xcode and you ran cmake in the Build folder. The shader can be referenced as "../Shaders/shader.frag" for example. Compiling from a Makefile will put the binary in Build/Glitter though. To have all binaries end up in the same folder we could add this code, as I showed before.

@simphax sorry again for the delays (holidays :disappointed: )

I've managed to get this working with makefiles, but it's still not happy under Xcode or Visual Studio. The two IDEs seem to be using a different working directory than ${CMAKE_BINARY_DIR} or wherever RUNTIME_OUTPUT_DIRECTORY is specified. I've been having this problem consistently for months now, and as far as I know, there's no good way to set the working directory via CMake.

Feel free to submit a pull request if you can get it working under makefiles, Xcode, and Visual Studio? I'm a little stumped at this point, maybe I'm doing something wrong.

simphax commented 8 years ago

No problem, it's already working for me :) I'm pretty sure my project over here works fine for both VS, Xcode and Make https://github.com/simphax/GlitterTest4

asma-AH commented 8 years ago

hi , thank you for your help , i 'm beginner in open gl and i'm using visual studio 2013 with windows10 there is an error in build projects:


CMake Error at CMakeLists.txt:7 (add_subdirectory): The source directory

C:/Users/asma_/Glitter-master/Glitter/Vendor/glfw

does not contain a CMakeLists.txt file.

CMake Error at CMakeLists.txt:12 (add_subdirectory): The source directory

C:/Users/asma_/Glitter-master/Glitter/Vendor/assimp

does not contain a CMakeLists.txt file.

CMake Error at CMakeLists.txt:19 (add_subdirectory): The source directory

C:/Users/asma_/Glitter-master/Glitter/Vendor/bullet

does not contain a CMakeLists.txt file.


please help me i have a project for my college ... Thank you ...

simphax commented 8 years ago

@asma-AH you need to run git submodule update --init

Polytonic commented 8 years ago

@simphax it's not working on my test machines here, but I'm open to the possibility that there's some quirky custom config that's being picked up. I want to resolve this the right way, but I just don't have the downtime available to wipe and reimage to confirm at this point. I'm going to tentatively close this issue for now. If more people start asking for this functionality, I'll carve out some time in a few months to give this my full, undivided attention.

@asma-AH if you continue having trouble, please feel free to open a new issue. :smile:

rafaelbeckel commented 7 years ago

How about this approach? http://hamelot.io/visualization/opengl-glsl-shader-as-a-string/