Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embed GLSL files during build #238

Merged
merged 7 commits into from
Nov 11, 2022
Merged

Embed GLSL files during build #238

merged 7 commits into from
Nov 11, 2022

Conversation

dethrace-labs
Copy link
Owner

  • Adds separate files for our GLSL shaders
  • Adds generation step for each one to convert it into a header file during the cmake pipeline
  • Begin to separate out a section at the top of the logs that we will ask users to copy when filing issues

foreach(elem resources/framebuffer_vert.glsl resources/framebuffer_frag.glsl resources/3d_vert.glsl resources/3d_frag.glsl)
add_custom_command(
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${elem}.h"
COMMAND ${CMAKE_COMMAND} -DSOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR} -DFILE="${elem}" -P ${CMAKE_SOURCE_DIR}/cmake/EmbedResource.cmake
Copy link
Collaborator

@madebr madebr Nov 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMAKE_SOURCE_DIR points to the root of a build tree, where we want the root of the the dethrace project.

Suggested change
COMMAND ${CMAKE_COMMAND} -DSOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR} -DFILE="${elem}" -P ${CMAKE_SOURCE_DIR}/cmake/EmbedResource.cmake
COMMAND "${CMAKE_COMMAND}" "-DSOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR}" "-DFILE=${elem}" -P "${dethrace_SOURCE_DIR}/cmake/EmbedResource.cmake"

Copy link
Owner Author

@dethrace-labs dethrace-labs Nov 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the ", but why dethrace_SOURCE_DIR? These are resources specific to harness, and should only be available to #include from harness code? Currently the files are generated in src/harness/resources/

Copy link
Collaborator

@madebr madebr Nov 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it, use of CMAKE_CURRENT_SOURCE_DIR is correct.
But CMAKE_SOURCE_DIR isn't: it should be either PROJECT_SOURCE_DIR or dethrace_SOURCE_DIR.
I updated my suggestion.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you suggesting to move cmake/EmbedResource.cmake to src/DETHRACE/cmake/EmbedResource.cmake?

Copy link
Collaborator

@madebr madebr Nov 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, if you happen to include dethrace as a subproject for your own project (using add_subdirectory), CMAKE_SOURCE_DIR points to the root source directory of the master project. Not to the root directory of dethrace.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, yep, I got it I think.

v_normal = a_normal;
v_tex_coord = a_uv;
gl_Position = u_projection * u_view * vec4(v_frag_pos, 1.0);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing final newline

@dethrace-labs dethrace-labs changed the title Embedded glsl files Embed GLSL files during build Nov 10, 2022
Copy link
Collaborator

@madebr madebr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works on my pc :)

@dethrace-labs dethrace-labs merged commit 4246f66 into main Nov 11, 2022
@dethrace-labs dethrace-labs deleted the embedded_glsl_files branch November 11, 2022 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants