Skip to content

Commit

Permalink
layers: Fix issues uncovered by SC downstream
Browse files Browse the repository at this point in the history 
- Use combined headers for chassis.cpp
- Re-add virtual qualifier lost during renaming
- Propagate code duplication of module query to all call sites
  • Loading branch information
@MathiasMagnus @spencer-lunarg
MathiasMagnus authored and spencer-lunarg committed Oct 2, 2024
1 parent a8fedaf commit 65f986c
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 16 deletions.
2 changes: 1 addition & 1 deletion layers/core_checks/core_validation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ class CoreChecks : public ValidationStateTracker {
bool PreCallValidateCreateShaderModule(VkDevice device, const VkShaderModuleCreateInfo* pCreateInfo,
const VkAllocationCallbacks* pAllocator, VkShaderModule* pShaderModule,
const ErrorObject& error_obj) const override;
bool ValidateShaderStage(const ShaderStageState& stage_state, const vvl::Pipeline* pipeline, const Location& loc) const;
virtual bool ValidateShaderStage(const ShaderStageState& stage_state, const vvl::Pipeline* pipeline, const Location& loc) const;
bool ValidatePointSizeShaderState(const spirv::Module& module_state, const spirv::EntryPoint& entrypoint,
const vvl::Pipeline& pipeline, VkShaderStageFlagBits stage, const Location& loc) const;
bool ValidatePrimitiveRateShaderState(const spirv::Module& module_state, const spirv::EntryPoint& entrypoint,
Expand Down
22 changes: 16 additions & 6 deletions layers/state_tracker/pipeline_sub_state.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ PreRasterState::PreRasterState(const vvl::Pipeline &p, const ValidationStateTrac
all_stages |= stage;

auto module_state = state_data.Get<vvl::ShaderModule>(stage_ci.module);
if (!module_state && p.pipeline_cache) {
// Attempt to look up the pipeline cache for shader module data
module_state = p.pipeline_cache->GetStageModule(p, i);
}
if (!module_state) {
// If module is null and there is a VkShaderModuleCreateInfo in the pNext chain of the stage info, then this
// module is part of a library and the state must be created.
Expand Down Expand Up @@ -193,12 +197,16 @@ std::unique_ptr<const vku::safe_VkPipelineShaderStageCreateInfo> ToShaderStageCI
}

template <typename CreateInfo>
void SetFragmentShaderInfoPrivate(FragmentShaderState &fs_state, const ValidationStateTracker &state_data,
const CreateInfo &create_info,
void SetFragmentShaderInfoPrivate(const vvl::Pipeline &pipeline_state, FragmentShaderState &fs_state,
const ValidationStateTracker &state_data, const CreateInfo &create_info,
spirv::StatelessData stateless_data[kCommonMaxGraphicsShaderStages]) {
for (uint32_t i = 0; i < create_info.stageCount; ++i) {
if (create_info.pStages[i].stage == VK_SHADER_STAGE_FRAGMENT_BIT) {
auto module_state = state_data.Get<vvl::ShaderModule>(create_info.pStages[i].module);
if (!module_state && pipeline_state.pipeline_cache) {
// Attempt to look up the pipeline cache for shader module data
module_state = pipeline_state.pipeline_cache->GetStageModule(pipeline_state, i);
}
if (!module_state) {
// If module is null and there is a VkShaderModuleCreateInfo in the pNext chain of the stage info, then this
// module is part of a library and the state must be created
Expand Down Expand Up @@ -239,17 +247,19 @@ void SetFragmentShaderInfoPrivate(FragmentShaderState &fs_state, const Validatio
}

// static
void FragmentShaderState::SetFragmentShaderInfo(FragmentShaderState &fs_state, const ValidationStateTracker &state_data,
void FragmentShaderState::SetFragmentShaderInfo(const vvl::Pipeline &pipeline_state, FragmentShaderState &fs_state,
const ValidationStateTracker &state_data,
const VkGraphicsPipelineCreateInfo &create_info,
spirv::StatelessData stateless_data[kCommonMaxGraphicsShaderStages]) {
SetFragmentShaderInfoPrivate(fs_state, state_data, create_info, stateless_data);
SetFragmentShaderInfoPrivate(pipeline_state, fs_state, state_data, create_info, stateless_data);
}

// static
void FragmentShaderState::SetFragmentShaderInfo(FragmentShaderState &fs_state, const ValidationStateTracker &state_data,
void FragmentShaderState::SetFragmentShaderInfo(const vvl::Pipeline &pipeline_state, FragmentShaderState &fs_state,
const ValidationStateTracker &state_data,
const vku::safe_VkGraphicsPipelineCreateInfo &create_info,
spirv::StatelessData stateless_data[kCommonMaxGraphicsShaderStages]) {
SetFragmentShaderInfoPrivate(fs_state, state_data, create_info, stateless_data);
SetFragmentShaderInfoPrivate(pipeline_state, fs_state, state_data, create_info, stateless_data);
}

FragmentShaderState::FragmentShaderState(const vvl::Pipeline &p, const ValidationStateTracker &dev_data,
Expand Down
17 changes: 9 additions & 8 deletions layers/state_tracker/pipeline_sub_state.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,20 +141,20 @@ std::unique_ptr<const vku::safe_VkPipelineShaderStageCreateInfo> ToShaderStageCI
std::unique_ptr<const vku::safe_VkPipelineShaderStageCreateInfo> ToShaderStageCI(const VkPipelineShaderStageCreateInfo &cbs);

struct FragmentShaderState : public PipelineSubState {
FragmentShaderState(const vvl::Pipeline &p, const ValidationStateTracker &dev_data, std::shared_ptr<const vvl::RenderPass> rp,
uint32_t subpass, VkPipelineLayout layout);
FragmentShaderState(const vvl::Pipeline &pipeline_state, const ValidationStateTracker &dev_data,
std::shared_ptr<const vvl::RenderPass> rp, uint32_t subpass, VkPipelineLayout layout);

template <typename CreateInfo>
FragmentShaderState(const vvl::Pipeline &p, const ValidationStateTracker &dev_data, const CreateInfo &create_info,
FragmentShaderState(const vvl::Pipeline &pipeline_state, const ValidationStateTracker &dev_data, const CreateInfo &create_info,
std::shared_ptr<const vvl::RenderPass> rp, spirv::StatelessData *stateless_data)
: FragmentShaderState(p, dev_data, rp, create_info.subpass, create_info.layout) {
: FragmentShaderState(pipeline_state, dev_data, rp, create_info.subpass, create_info.layout) {
if (create_info.pMultisampleState) {
ms_state = ToSafeMultisampleState(*create_info.pMultisampleState);
}
if (create_info.pDepthStencilState) {
ds_state = ToSafeDepthStencilState(*create_info.pDepthStencilState);
}
FragmentShaderState::SetFragmentShaderInfo(*this, dev_data, create_info, stateless_data);
FragmentShaderState::SetFragmentShaderInfo(pipeline_state, *this, dev_data, create_info, stateless_data);
}

static inline VkShaderStageFlags ValidShaderStages() { return VK_SHADER_STAGE_FRAGMENT_BIT; }
Expand All @@ -172,10 +172,11 @@ struct FragmentShaderState : public PipelineSubState {
std::shared_ptr<const spirv::EntryPoint> fragment_entry_point;

private:
static void SetFragmentShaderInfo(FragmentShaderState &fs_state, const ValidationStateTracker &state_data,
const VkGraphicsPipelineCreateInfo &create_info,
static void SetFragmentShaderInfo(const vvl::Pipeline &pipeline_state, FragmentShaderState &fs_state,
const ValidationStateTracker &state_data, const VkGraphicsPipelineCreateInfo &create_info,
spirv::StatelessData stateless_data[kCommonMaxGraphicsShaderStages]);
static void SetFragmentShaderInfo(FragmentShaderState &fs_state, const ValidationStateTracker &state_data,
static void SetFragmentShaderInfo(const vvl::Pipeline &pipeline_state, FragmentShaderState &fs_state,
const ValidationStateTracker &state_data,
const vku::safe_VkGraphicsPipelineCreateInfo &create_info,
spirv::StatelessData stateless_data[kCommonMaxGraphicsShaderStages]);
};
Expand Down
2 changes: 1 addition & 1 deletion scripts/generate_source.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def RunGenerators(api: str, registry: str, grammar: str, directory: str, styleFi
},
'chassis.cpp' : {
'generator' : LayerChassisOutputGenerator,
'genCombined': False,
'genCombined': True,
},
'chassis_dispatch_helper.h' : {
'generator' : LayerChassisOutputGenerator,
Expand Down

0 comments on commit 65f986c

Please sign in to comment.