-
Notifications
You must be signed in to change notification settings - Fork 470
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
Express "ALL_*" pipeline stages as including _valid_ bits #2219
base: main
Are you sure you want to change the base?
Conversation
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.
Following shorthand Bits should potentially receive analogous treatment:
VK_PIPELINE_STAGE_2_PRE_RASTERIZATION_SHADERS_BIT
VK_PIPELINE_STAGE_2_ALL_TRANSFER_BIT
VK_PIPELINE_STAGE_2_VERTEX_INPUT_BIT
VK_ACCESS_MEMORY_*_BIT
VK_ACCESS_2_SHADER_READ_BIT
VK_ACCESS_2_SHADER_WRITE_BIT
@@ -440,10 +440,11 @@ ifdef::VK_KHR_ray_tracing_maintenance1[] | |||
endif::VK_KHR_ray_tracing_maintenance1[] | |||
endif::VK_KHR_acceleration_structure,VK_NV_ray_tracing[] | |||
* ename:VK_PIPELINE_STAGE_2_ALL_GRAPHICS_BIT specifies the execution of | |||
all graphics pipeline stages, and is equivalent to the logical OR of: | |||
all graphics pipeline stages, that would be valid in the context that | |||
this bitflag is provided, from the following list of stages: |
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.
-
"specifies the execution" is weirdly too specific.
ALL_COMMANDS
says just "specifies all pipeline stages that". Should probably at least be consistent. So perhaps "specifies all applicable graphics pipeline stages, that is valid stages from this list: -
I think styleguide prescribes bitmask over "bitflag".
-
I think a good rule of thumb is that "which" is with comma, while "that" is wihout. "that" part usually contains information that should not be separated from rest of the sentence.
-
👍removal of incorrect "logical OR"
-
There's probably more elegant way to word it. Pehaps:
VK_PIPELINE_STAGE_2_ALL_GRAPHICS_BIT
specifies all applicable graphics pipeline stages, i.e. functionally it is a substitute for specifying those stages from the following list that would be valid to use:
* ename:VK_PIPELINE_STAGE_ALL_COMMANDS_BIT specifies all operations | ||
performed by all commands supported on the queue it is used with. | ||
* ename:VK_PIPELINE_STAGE_ALL_COMMANDS_BIT specifies all pipeline stages that | ||
would be valid in the context that this bitflag is provided. |
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.
Slightly ambiguous whether it includes STAGE_2
stages, or only those defined in this bitmask.
PS: Exported to #2235.
@@ -440,10 +440,11 @@ ifdef::VK_KHR_ray_tracing_maintenance1[] | |||
endif::VK_KHR_ray_tracing_maintenance1[] | |||
endif::VK_KHR_acceleration_structure,VK_NV_ray_tracing[] | |||
* ename:VK_PIPELINE_STAGE_2_ALL_GRAPHICS_BIT specifies the execution of | |||
all graphics pipeline stages, and is equivalent to the logical OR of: | |||
all graphics pipeline stages, that would be valid in the context that |
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.
"graphics pipeline stages" is somewhat misleading, since it seems to include Bits from e.g. subpass shading pipeline.
Fix #2187