-
Notifications
You must be signed in to change notification settings - Fork 422
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
layers: Fix PIPELINE_STATE::GetStageStates bug #4760
layers: Fix PIPELINE_STATE::GetStageStates bug #4760
Conversation
CI Vulkan-ValidationLayers build queued with queue ID 28432. |
CI Vulkan-ValidationLayers build # 9324 running. |
CI Vulkan-ValidationLayers build # 9324 failed. |
CI Vulkan-ValidationLayers build queued with queue ID 28459. |
self.flag_values_definitions[flag] = Guarded(self.featureExtraProtect, decl) | ||
if flag in self.flagBitsAsArray: | ||
decl = 'const std::array<%s, %d> All%s = {%s};' % (flag, len(bits), flag, ','.join(bits)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: maybe use formatted string literals instead (not 100% sure about the syntax):
decl = 'const std::array<{flag}, {len(bits)}> All{flag} = {{{','.join(bits)}}}'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So yes, the python string format debate 😆
I have zero preference how we printf
in the scripts, but as long as it is consistent everywhere... which is far from currently. I just mimicked the styles in the function above and where the other std::array
string is
There is a long needed TODO to align all these to a single style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Only one nit
CI Vulkan-ValidationLayers build # 9326 running. |
CI Vulkan-ValidationLayers build # 9326 failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question, but otherwise LGTM!
CI Vulkan-ValidationLayers build queued with queue ID 28800. |
CI Vulkan-ValidationLayers build # 9344 running. |
CI Vulkan-ValidationLayers build # 9344 failed. |
CI Vulkan-ValidationLayers build queued with queue ID 28994. |
CI Vulkan-ValidationLayers build # 9347 running. |
CI Vulkan-ValidationLayers build # 9347 failed. |
CI Vulkan-ValidationLayers build queued with queue ID 29032. |
CI Vulkan-ValidationLayers build # 9349 running. |
CI Vulkan-ValidationLayers build # 9349 passed. |
closes #4689
Better version of #4690