Skip to content

S
swiftshader
Commit dbd02755 by Nicolas Capens Committed by Nicolas Capens
Options

Fix samplerless image fetch

OpImageFetch takes an Image operand, not a SampledImage, and thus we don't explicitly have a sampler descriptor. However, our vk::SampledImageDescriptor structure includes both image view descriptor data and sampler data, and we were relying on the sampler descriptor data to be present for setting the addressing mode. Other state would also inadvertently be affected. This change ensures that the sampler descriptor for fetch operations is null and we set the addressing mode to repeat but leave other unused state at their defaults. Bug: b/139401791 Change-Id: I9f1af35940c2fa9c7d30e771ebdb244f72f5bdbe Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/35348 Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarChris Forbes <chrisforbes@google.com>
parent 02d4c0d3
Showing with 67 additions and 23 deletions
src/Pipeline/SpirvShader.cpp
...@@ -4910,7 +4910,7 @@ namespace sw ...@@ -4910,7 +4910,7 @@ namespace sw
{ {
Type::ID resultTypeId = insn.word(1); Type::ID resultTypeId = insn.word(1);
Object::ID resultId = insn.word(2); Object::ID resultId = insn.word(2);
Object::ID sampledImageId = insn.word(3); Object::ID sampledImageId = insn.word(3); // For OpImageFetch this is just an Image, not a SampledImage.
Object::ID coordinateId = insn.word(4); Object::ID coordinateId = insn.word(4);
auto &resultType = getType(resultTypeId); auto &resultType = getType(resultTypeId);
...@@ -4928,6 +4928,14 @@ namespace sw ...@@ -4928,6 +4928,14 @@ namespace sw
Pointer<Byte> sampler = samplerDescriptor + OFFSET(vk::SampledImageDescriptor, sampler); // vk::Sampler* Pointer<Byte> sampler = samplerDescriptor + OFFSET(vk::SampledImageDescriptor, sampler); // vk::Sampler*
Pointer<Byte> texture = imageDescriptor + OFFSET(vk::SampledImageDescriptor, texture); // sw::Texture* Pointer<Byte> texture = imageDescriptor + OFFSET(vk::SampledImageDescriptor, texture); // sw::Texture*
// Above we assumed that if the SampledImage operand is not the result of an OpSampledImage,
// it must be a combined image sampler loaded straight from the descriptor set. For OpImageFetch
// it's just an Image operand, so there's no sampler descriptor data.
if(getType(sampledImage.type).opcode() != spv::OpTypeSampledImage)
{
sampler = Pointer<Byte>(nullptr);
}
uint32_t imageOperands = spv::ImageOperandsMaskNone; uint32_t imageOperands = spv::ImageOperandsMaskNone;
bool lodOrBias = false; bool lodOrBias = false;
Object::ID lodOrBiasId = 0; Object::ID lodOrBiasId = 0;
......
src/Pipeline/SpirvShader.hpp
...@@ -1265,7 +1265,7 @@ namespace sw ...@@ -1265,7 +1265,7 @@ namespace sw
// TODO(b/129523279): Eliminate conversion and use vk::Sampler members directly. // TODO(b/129523279): Eliminate conversion and use vk::Sampler members directly.
static sw::FilterType convertFilterMode(const vk::Sampler *sampler); static sw::FilterType convertFilterMode(const vk::Sampler *sampler);
static sw::MipmapType convertMipmapMode(const vk::Sampler *sampler); static sw::MipmapType convertMipmapMode(const vk::Sampler *sampler);
static sw::AddressingMode convertAddressingMode(int coordinateIndex, VkSamplerAddressMode addressMode, VkImageViewType imageViewType); static sw::AddressingMode convertAddressingMode(int coordinateIndex, const vk::Sampler *sampler, VkImageViewType imageViewType);
// Returns 0 when invalid. // Returns 0 when invalid.
static VkShaderStageFlagBits executionModelToStage(spv::ExecutionModel model); static VkShaderStageFlagBits executionModelToStage(spv::ExecutionModel model);
......
src/Pipeline/SpirvShaderSampling.cpp
...@@ -34,9 +34,10 @@ namespace sw { ...@@ -34,9 +34,10 @@ namespace sw {
SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::SampledImageDescriptor const *imageDescriptor, const vk::Sampler *sampler) SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::SampledImageDescriptor const *imageDescriptor, const vk::Sampler *sampler)
{ {
ImageInstruction instruction(inst); ImageInstruction instruction(inst);
ASSERT(imageDescriptor->imageViewId != 0 && (sampler->id != 0 || instruction.samplerMethod == Fetch)); const auto samplerId = sampler ? sampler->id : 0;
ASSERT(imageDescriptor->imageViewId != 0 && (samplerId != 0 || instruction.samplerMethod == Fetch));
vk::Device::SamplingRoutineCache::Key key = {inst, imageDescriptor->imageViewId, sampler->id}; vk::Device::SamplingRoutineCache::Key key = {inst, imageDescriptor->imageViewId, samplerId};
ASSERT(imageDescriptor->device); ASSERT(imageDescriptor->device);
...@@ -59,34 +60,41 @@ SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::Sampl ...@@ -59,34 +60,41 @@ SpirvShader::ImageSampler *SpirvShader::getImageSampler(uint32_t inst, vk::Sampl
Sampler samplerState = {}; Sampler samplerState = {};
samplerState.textureType = type; samplerState.textureType = type;
samplerState.textureFormat = imageDescriptor->format; samplerState.textureFormat = imageDescriptor->format;
samplerState.textureFilter = (instruction.samplerMethod == Gather) ? FILTER_GATHER : convertFilterMode(sampler);
samplerState.border = sampler->borderColor;
samplerState.addressingModeU = convertAddressingMode(0, sampler->addressModeU, type); samplerState.addressingModeU = convertAddressingMode(0, sampler, type);
samplerState.addressingModeV = convertAddressingMode(1, sampler->addressModeV, type); samplerState.addressingModeV = convertAddressingMode(1, sampler, type);
samplerState.addressingModeW = convertAddressingMode(2, sampler->addressModeW, type); samplerState.addressingModeW = convertAddressingMode(2, sampler, type);
samplerState.mipmapFilter = convertMipmapMode(sampler); samplerState.mipmapFilter = convertMipmapMode(sampler);
samplerState.swizzle = imageDescriptor->swizzle; samplerState.swizzle = imageDescriptor->swizzle;
samplerState.gatherComponent = instruction.gatherComponent; samplerState.gatherComponent = instruction.gatherComponent;
samplerState.highPrecisionFiltering = false; samplerState.highPrecisionFiltering = false;
samplerState.compareEnable = (sampler->compareEnable == VK_TRUE);
samplerState.compareOp = sampler->compareOp;
samplerState.unnormalizedCoordinates = (sampler->unnormalizedCoordinates == VK_TRUE);
samplerState.largeTexture = (imageDescriptor->extent.width > SHRT_MAX) || samplerState.largeTexture = (imageDescriptor->extent.width > SHRT_MAX) ||
(imageDescriptor->extent.height > SHRT_MAX) || (imageDescriptor->extent.height > SHRT_MAX) ||
(imageDescriptor->extent.depth > SHRT_MAX); (imageDescriptor->extent.depth > SHRT_MAX);
if(sampler->ycbcrConversion) if(sampler)
{ {
samplerState.ycbcrModel = sampler->ycbcrConversion->ycbcrModel; samplerState.textureFilter = (instruction.samplerMethod == Gather) ? FILTER_GATHER : convertFilterMode(sampler);
samplerState.studioSwing = (sampler->ycbcrConversion->ycbcrRange == VK_SAMPLER_YCBCR_RANGE_ITU_NARROW); samplerState.border = sampler->borderColor;
samplerState.swappedChroma = (sampler->ycbcrConversion->components.r != VK_COMPONENT_SWIZZLE_R);
}
if(sampler->anisotropyEnable != VK_FALSE) samplerState.mipmapFilter = convertMipmapMode(sampler);
{
UNSUPPORTED("anisotropyEnable"); samplerState.compareEnable = (sampler->compareEnable == VK_TRUE);
samplerState.compareOp = sampler->compareOp;
samplerState.unnormalizedCoordinates = (sampler->unnormalizedCoordinates == VK_TRUE);
if(sampler->ycbcrConversion)
{
samplerState.ycbcrModel = sampler->ycbcrConversion->ycbcrModel;
samplerState.studioSwing = (sampler->ycbcrConversion->ycbcrRange == VK_SAMPLER_YCBCR_RANGE_ITU_NARROW);
samplerState.swappedChroma = (sampler->ycbcrConversion->components.r != VK_COMPONENT_SWIZZLE_R);
}
if(sampler->anisotropyEnable != VK_FALSE)
{
UNSUPPORTED("anisotropyEnable");
}
} }
routine = emitSamplerRoutine(instruction, samplerState); routine = emitSamplerRoutine(instruction, samplerState);
...@@ -249,6 +257,11 @@ sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler) ...@@ -249,6 +257,11 @@ sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler)
sw::MipmapType SpirvShader::convertMipmapMode(const vk::Sampler *sampler) sw::MipmapType SpirvShader::convertMipmapMode(const vk::Sampler *sampler)
{ {
if(!sampler)
{
return MIPMAP_POINT; // Samplerless operations (OpImageFetch) can take an integer Lod operand.
}
if(sampler->ycbcrConversion) if(sampler->ycbcrConversion)
{ {
return MIPMAP_NONE; // YCbCr images can only have one mipmap level. return MIPMAP_NONE; // YCbCr images can only have one mipmap level.
...@@ -264,7 +277,7 @@ sw::MipmapType SpirvShader::convertMipmapMode(const vk::Sampler *sampler) ...@@ -264,7 +277,7 @@ sw::MipmapType SpirvShader::convertMipmapMode(const vk::Sampler *sampler)
} }
} }
sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, VkSamplerAddressMode addressMode, VkImageViewType imageViewType) sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, const vk::Sampler *sampler, VkImageViewType imageViewType)
{ {
switch(imageViewType) switch(imageViewType)
{ {
...@@ -342,6 +355,27 @@ sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, VkSam ...@@ -342,6 +355,27 @@ sw::AddressingMode SpirvShader::convertAddressingMode(int coordinateIndex, VkSam
return ADDRESSING_WRAP; return ADDRESSING_WRAP;
} }
if(!sampler)
{
// OpImageFetch does not take a sampler descriptor, but still needs a valid,
// arbitrary addressing mode that prevents out-of-bounds accesses:
// "The value returned by a read of an invalid texel is undefined, unless that
// read operation is from a buffer resource and the robustBufferAccess feature
// is enabled. In that case, an invalid texel is replaced as described by the
// robustBufferAccess feature." - Vulkan 1.1
return ADDRESSING_WRAP;
}
VkSamplerAddressMode addressMode = VK_SAMPLER_ADDRESS_MODE_REPEAT;
switch(coordinateIndex)
{
case 0: addressMode = sampler->addressModeU; break;
case 1: addressMode = sampler->addressModeV; break;
case 2: addressMode = sampler->addressModeW; break;
default: UNSUPPORTED("coordinateIndex: %d", coordinateIndex);
}
switch(addressMode) switch(addressMode)
{ {
case VK_SAMPLER_ADDRESS_MODE_REPEAT: return ADDRESSING_WRAP; case VK_SAMPLER_ADDRESS_MODE_REPEAT: return ADDRESSING_WRAP;
......
src/Vulkan/vulkan.vcxproj
...@@ -80,7 +80,8 @@ ...@@ -80,7 +80,8 @@
mkdir "$(SolutionDir)build\$(Configuration)_$(Platform)\" mkdir "$(SolutionDir)build\$(Configuration)_$(Platform)\"
copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)out\$(Configuration)_$(Platform)\" copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)out\$(Configuration)_$(Platform)\"
copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)build\$(Configuration)_$(Platform)\" copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)build\$(Configuration)_$(Platform)\"
IF EXIST "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\" (copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\vulkan-1.dll")</Command> IF EXIST "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\Debug" (copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\Debug\vulkan-1.dll")
IF EXIST "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\Release" (copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\Release\vulkan-1.dll")</Command>
</PostBuildEvent> </PostBuildEvent>
</ItemDefinitionGroup> </ItemDefinitionGroup>
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">
...@@ -105,7 +106,8 @@ IF EXIST "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\" (copy ...@@ -105,7 +106,8 @@ IF EXIST "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\" (copy
mkdir "$(SolutionDir)build\$(Configuration)_$(Platform)\" mkdir "$(SolutionDir)build\$(Configuration)_$(Platform)\"
copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)out\$(Configuration)_$(Platform)\" copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)out\$(Configuration)_$(Platform)\"
copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)build\$(Configuration)_$(Platform)\" copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)build\$(Configuration)_$(Platform)\"
IF EXIST "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\" (copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\vulkan-1.dll")</Command> IF EXIST "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\Debug" (copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\Debug\vulkan-1.dll")
IF EXIST "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\Release" (copy "$(OutDir)vk_swiftshader.dll" "$(SolutionDir)..\deqp\build\external\vulkancts\modules\vulkan\Release\vulkan-1.dll")</Command>
</PostBuildEvent> </PostBuildEvent>
</ItemDefinitionGroup> </ItemDefinitionGroup>
<ItemGroup> <ItemGroup>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment