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

GL_EXT_spirv_intrinsics port of GL_EXT_shader_realtime_clock #2749

Merged
merged 1 commit into from
Oct 18, 2021

Conversation

AaronHaganAMD
Copy link
Contributor

Add mechanism to use GL_EXT_spirv_intrinsics headers in glslang.
Ported GL_EXT_shader_realtime_clock as an example.

@greg-lunarg greg-lunarg added the kokoro:run Trigger Google bot runs label Sep 13, 2021
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Sep 13, 2021
@greg-lunarg
Copy link
Contributor

Re testing failures, it looks like you will need to inform the other builds of the new files.

Copy link
Contributor

@greg-lunarg greg-lunarg left a comment

Choose a reason for hiding this comment

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

Some general comments:

  • I am generally fine with moving ahead with this new framework for extension intrinsic functions.
  • I would like to see the extension-specific headers generated into the "build" directory rather than into the "source" tree.
  • If I am understanding this PR correctly, the header file glslang/ExtensionHeaders/GL_EXT_shader_realtime_clock.h is a generated file and should not be part of the source controlled repo, right?

@AaronHaganAMD
Copy link
Contributor Author

AaronHaganAMD commented Oct 5, 2021

I re-worked this to take the header files from glslang\ExtensionHeaders and put them in build\include\glslang\glsl_intrinsic_header.h

@greg-lunarg greg-lunarg added the kokoro:run Trigger Google bot runs label Oct 12, 2021
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Oct 12, 2021
@greg-lunarg
Copy link
Contributor

I think you are going to have to update the bazel and gn builds, which will likely mean changes to BUILD.bazel and BUILD.gn in the root directory, and possibly other supporting scripts or files.

@greg-lunarg
Copy link
Contributor

I am still seeing that you are attempting to add glslang/ExtensionHeaders/GL_EXT_shader_realtime_clock.h to the repo (see my comment above) which I believe is incorrect. This is the reason for the license failure above. I think if you fix this as I suggest above the license failure will go away.

Copy link
Contributor

@greg-lunarg greg-lunarg left a comment

Choose a reason for hiding this comment

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

Please fix the bazel and gn builds and fix GL_EXT_shader_realtime_clock.glsl as I have requested in the comments section.

@AaronHaganAMD
Copy link
Contributor Author

I am still seeing that you are attempting to add glslang/ExtensionHeaders/GL_EXT_shader_realtime_clock.h to the repo (see my comment above) which I believe is incorrect. This is the reason for the license failure above. I think if you fix this as I suggest above the license failure will go away.

glslang/ExtensionHeaders/GL_EXT_shader_realtime_clock.h should be removed from my latest commit (802731d)

@greg-lunarg greg-lunarg added the kokoro:run Trigger Google bot runs label Oct 14, 2021
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Oct 14, 2021
@greg-lunarg greg-lunarg added the kokoro:run Trigger Google bot runs label Oct 15, 2021
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Oct 15, 2021
Add mechanism to use GL_EXT_spirv_intrinsics headers in glslang.
Ported GL_EXT_shader_realtime_clock as an example.
@greg-lunarg greg-lunarg added the kokoro:run Trigger Google bot runs label Oct 15, 2021
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Oct 15, 2021
Copy link
Contributor

@greg-lunarg greg-lunarg left a comment

Choose a reason for hiding this comment

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

Latest code LGTM

@greg-lunarg greg-lunarg changed the title GL_EXT_spirv_intrinsics - Port extensions GL_EXT_spirv_intrinsics port of GL_EXT_shader_realtime_clock Oct 18, 2021
@greg-lunarg greg-lunarg merged commit 7f1d926 into KhronosGroup:master Oct 18, 2021
@greg-lunarg
Copy link
Contributor

Heads up: this PR was partially reverted by #2817, specifically the port of GL_EXT_shader_realtime_clock. This was due to regressions caused for users of the glslang library interface. We were building for the SDK and did not have time to fix this, hence the revert. Please address this problem and re-submit.

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.

3 participants