Pass a separate sampler pointer to the sampling routine
Previously only the texture pointer was passed to the sampling routine,
containing some sampler data which is (still) wrongly assumed to come
from a combined image-view and sampler descriptor. This change will
allow splitting them up correctly.
Also refactor getImageSampler to not start the Reactor function
generation, moving it entirely into emitSamplerFunction so that the
latter only has to be passed the pointers to the descriptor state that
will affect the code that's generated.
Name all the parameters according to their actual type.
Clamp sampler LOD on construction so we don't have to deal with it any
more later.
Bug: b/129523279
Test: dEQP-VK.glsl.texture_functions.*
Test: dEQP-VK.spirv_assembly.instruction.graphics.image_sampler.*
Change-Id: Id3a9f12f379cf65741198b732fe387ec6e24dd86
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/30350
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
diff --git a/src/Pipeline/SamplerCore.cpp b/src/Pipeline/SamplerCore.cpp
index e09f1c4..96999ac 100644
--- a/src/Pipeline/SamplerCore.cpp
+++ b/src/Pipeline/SamplerCore.cpp
@@ -16,6 +16,7 @@
#include "PixelRoutine.hpp"
#include "Constants.hpp"
+#include "Vulkan/VkSampler.hpp"
#include "Vulkan/VkDebug.hpp"
namespace
@@ -50,7 +51,7 @@
{
}
- Vector4f SamplerCore::sampleTexture(Pointer<Byte> &texture, Float4 &u, Float4 &v, Float4 &w, Float4 &q, Float4 &lodOrBias, Vector4f &dsx, Vector4f &dsy, Vector4f &offset, SamplerFunction function)
+ Vector4f SamplerCore::sampleTexture(Pointer<Byte> &texture, Pointer<Byte> &sampler, Float4 &u, Float4 &v, Float4 &w, Float4 &q, Float4 &lodOrBias, Vector4f &dsx, Vector4f &dsy, Vector4f &offset, SamplerFunction function)
{
Vector4f c;
diff --git a/src/Pipeline/SamplerCore.hpp b/src/Pipeline/SamplerCore.hpp
index 2aad2c9..b822299 100644
--- a/src/Pipeline/SamplerCore.hpp
+++ b/src/Pipeline/SamplerCore.hpp
@@ -60,7 +60,7 @@
public:
SamplerCore(Pointer<Byte> &constants, const Sampler::State &state);
- Vector4f sampleTexture(Pointer<Byte> &texture, Float4 &u, Float4 &v, Float4 &w, Float4 &q, Float4 &lodOrBias, Vector4f &dsx, Vector4f &dsy, Vector4f &offset, SamplerFunction function);
+ Vector4f sampleTexture(Pointer<Byte> &texture, Pointer<Byte> &sampler, Float4 &u, Float4 &v, Float4 &w, Float4 &q, Float4 &lodOrBias, Vector4f &dsx, Vector4f &dsy, Vector4f &offset, SamplerFunction function);
private:
void border(Short4 &mask, Float4 &coordinates);
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index 9f5246d..853c24d 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -4564,8 +4564,9 @@
auto coordinate = GenericValue(this, state->routine, coordinateId);
auto &coordinateType = getType(coordinate.type);
- auto sampler = *Pointer<Pointer<Byte>>(samplerDescriptor + OFFSET(vk::SampledImageDescriptor, sampler)); // vk::Sampler*
- auto imageView = *Pointer<Pointer<Byte>>(imageDescriptor + OFFSET(vk::SampledImageDescriptor, imageView)); // vk::ImageView*
+ Pointer<Byte> sampler = samplerDescriptor + OFFSET(vk::SampledImageDescriptor, sampler); // vk::Sampler*
+ Pointer<Byte> imageView = *Pointer<Pointer<Byte>>(imageDescriptor + OFFSET(vk::SampledImageDescriptor, imageView)); // vk::ImageView*
+ Pointer<Byte> texture = imageDescriptor + OFFSET(vk::SampledImageDescriptor, texture); // sw::Texture*
uint32_t imageOperands = spv::ImageOperandsMaskNone;
bool lodOrBias = false;
@@ -4698,7 +4699,7 @@
auto samplerFunc = Call(getImageSampler, instruction.parameters, imageView, sampler);
Array<SIMD::Float> out(4);
- Call<ImageSampler>(samplerFunc, imageDescriptor, &in[0], &out[0], state->routine->constants);
+ Call<ImageSampler>(samplerFunc, texture, sampler, &in[0], &out[0], state->routine->constants);
for (int i = 0; i < 4; i++) { result.move(i, out[i]); }
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 77da4d5..3423968 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -258,7 +258,7 @@
using InsnStore = std::vector<uint32_t>;
InsnStore insns;
- using ImageSampler = void(void* image, void* uvsIn, void* texelOut, void* constants);
+ using ImageSampler = void(void* texture, void *sampler, void* uvsIn, void* texelOut, void* constants);
using GetImageSampler = ImageSampler*(const vk::ImageView *imageView, const vk::Sampler *sampler);
/* Pseudo-iterator over SPIRV instructions, designed to support range-based-for. */
@@ -928,10 +928,7 @@
std::pair<SIMD::Float, SIMD::Int> Frexp(RValue<SIMD::Float> val) const;
static ImageSampler *getImageSampler(uint32_t instruction, const vk::ImageView *imageView, const vk::Sampler *sampler);
- static void emitSamplerFunction(
- ImageInstruction instruction,
- const vk::ImageView *imageView, const vk::Sampler *sampler,
- Pointer<Byte> image, Pointer<SIMD::Float> in, Pointer<Byte> out, Pointer<Byte> constants);
+ static ImageSampler *emitSamplerFunction(ImageInstruction instruction, const Sampler::State &samplerState);
// TODO(b/129523279): Eliminate conversion and use vk::Sampler members directly.
static sw::TextureType convertTextureType(VkImageViewType imageViewType);
diff --git a/src/Pipeline/SpirvShaderSampling.cpp b/src/Pipeline/SpirvShaderSampling.cpp
index 905551f..bd9222d 100644
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -50,25 +50,6 @@
auto it = cache.find(key);
if (it != cache.end()) { return it->second; }
- // TODO: Hold a separate mutex lock for the sampler being built.
- auto function = rr::Function<Void(Pointer<Byte> image, Pointer<SIMD::Float>, Pointer<SIMD::Float>, Pointer<Byte>)>();
- Pointer<Byte> image = function.Arg<0>();
- Pointer<SIMD::Float> in = function.Arg<1>();
- Pointer<SIMD::Float> out = function.Arg<2>();
- Pointer<Byte> constants = function.Arg<3>();
-
- emitSamplerFunction({instruction}, imageView, sampler, image, in, out, constants);
-
- auto fptr = reinterpret_cast<ImageSampler*>((void *)function("sampler")->getEntry());
- cache.emplace(key, fptr);
- return fptr;
-}
-
-void SpirvShader::emitSamplerFunction(
- ImageInstruction instruction,
- const vk::ImageView *imageView, const vk::Sampler *sampler,
- Pointer<Byte> image, Pointer<SIMD::Float> in, Pointer<Byte> out, Pointer<Byte> constants)
-{
Sampler::State samplerState = {};
samplerState.textureType = convertTextureType(imageView->getType());
samplerState.textureFormat = imageView->getFormat();
@@ -91,64 +72,83 @@
ASSERT(sampler->anisotropyEnable == VK_FALSE); // TODO(b/129523279)
ASSERT(sampler->unnormalizedCoordinates == VK_FALSE); // TODO(b/129523279)
- SamplerCore s(constants, samplerState);
+ auto fptr = emitSamplerFunction({instruction}, samplerState);
- Pointer<Byte> texture = image + OFFSET(vk::SampledImageDescriptor, texture); // sw::Texture*
- SIMD::Float uvw[3];
- SIMD::Float q(0); // TODO(b/129523279)
- SIMD::Float lodOrBias(0); // Explicit level-of-detail, or bias added to the implicit level-of-detail (depending on samplerMethod).
- Vector4f dsx;
- Vector4f dsy;
- Vector4f offset;
- SamplerFunction samplerFunction = instruction.getSamplerFunction();
+ cache.emplace(key, fptr);
+ return fptr;
+}
- uint32_t i = 0;
- for( ; i < instruction.coordinates; i++)
+SpirvShader::ImageSampler *SpirvShader::emitSamplerFunction(ImageInstruction instruction, const Sampler::State &samplerState)
+{
+ // TODO(b/129523279): Hold a separate mutex lock for the sampler being built.
+ Function<Void(Pointer<Byte>, Pointer<Byte>, Pointer<SIMD::Float>, Pointer<SIMD::Float>, Pointer<Byte>)> function;
{
- uvw[i] = in[i];
- }
+ Pointer<Byte> texture = function.Arg<0>();
+ Pointer<Byte> sampler = function.Arg<1>();
+ Pointer<SIMD::Float> in = function.Arg<2>();
+ Pointer<SIMD::Float> out = function.Arg<3>();
+ Pointer<Byte> constants = function.Arg<4>();
- // TODO(b/129523279): Currently 1D textures are treated as 2D by setting the second coordinate to 0.
- // Implement optimized 1D sampling.
- if(imageView->getType() == VK_IMAGE_VIEW_TYPE_1D ||
- imageView->getType() == VK_IMAGE_VIEW_TYPE_1D_ARRAY)
- {
- uvw[1] = SIMD::Float(0);
- }
+ SamplerCore s(constants, samplerState);
- if(instruction.samplerMethod == Lod || instruction.samplerMethod == Bias)
- {
- lodOrBias = in[i];
- i++;
- }
- else if(instruction.samplerMethod == Grad)
- {
- for(uint32_t j = 0; j < instruction.gradComponents; j++, i++)
+ SIMD::Float uvw[3];
+ SIMD::Float q(0); // TODO(b/129523279)
+ SIMD::Float lodOrBias(0); // Explicit level-of-detail, or bias added to the implicit level-of-detail (depending on samplerMethod).
+ Vector4f dsx;
+ Vector4f dsy;
+ Vector4f offset;
+ SamplerFunction samplerFunction = instruction.getSamplerFunction();
+
+ uint32_t i = 0;
+ for( ; i < instruction.coordinates; i++)
{
- dsx[j] = in[i];
+ uvw[i] = in[i];
}
- for(uint32_t j = 0; j < instruction.gradComponents; j++, i++)
+ // TODO(b/129523279): Currently 1D textures are treated as 2D by setting the second coordinate to 0.
+ // Implement optimized 1D sampling.
+ if(samplerState.textureType == TEXTURE_1D ||
+ samplerState.textureType == TEXTURE_1D_ARRAY)
{
- dsy[j] = in[i];
+ uvw[1] = SIMD::Float(0);
}
+
+ if(instruction.samplerMethod == Lod || instruction.samplerMethod == Bias)
+ {
+ lodOrBias = in[i];
+ i++;
+ }
+ else if(instruction.samplerMethod == Grad)
+ {
+ for(uint32_t j = 0; j < instruction.gradComponents; j++, i++)
+ {
+ dsx[j] = in[i];
+ }
+
+ for(uint32_t j = 0; j < instruction.gradComponents; j++, i++)
+ {
+ dsy[j] = in[i];
+ }
+ }
+
+ if(instruction.samplerOption == Offset)
+ {
+ for(uint32_t j = 0; j < instruction.offsetComponents; j++, i++)
+ {
+ offset[j] = in[i];
+ }
+ }
+
+ Vector4f sample = s.sampleTexture(texture, sampler, uvw[0], uvw[1], uvw[2], q, lodOrBias, dsx, dsy, offset, samplerFunction);
+
+ Pointer<SIMD::Float> rgba = out;
+ rgba[0] = sample.x;
+ rgba[1] = sample.y;
+ rgba[2] = sample.z;
+ rgba[3] = sample.w;
}
- if(instruction.samplerOption == Offset)
- {
- for(uint32_t j = 0; j < instruction.offsetComponents; j++, i++)
- {
- offset[j] = in[i];
- }
- }
-
- Vector4f sample = s.sampleTexture(texture, uvw[0], uvw[1], uvw[2], q, lodOrBias, dsx, dsy, offset, samplerFunction);
-
- Pointer<SIMD::Float> rgba = out;
- rgba[0] = sample.x;
- rgba[1] = sample.y;
- rgba[2] = sample.z;
- rgba[3] = sample.w;
+ return (ImageSampler*)function("sampler")->getEntry();
}
sw::TextureType SpirvShader::convertTextureType(VkImageViewType imageViewType)
diff --git a/src/Vulkan/VkDescriptorSetLayout.cpp b/src/Vulkan/VkDescriptorSetLayout.cpp
index b84079c..e769163 100644
--- a/src/Vulkan/VkDescriptorSetLayout.cpp
+++ b/src/Vulkan/VkDescriptorSetLayout.cpp
@@ -258,12 +258,12 @@
void SampledImageDescriptor::updateSampler(const vk::Sampler *sampler)
{
- this->sampler = sampler;
-
if (sampler)
{
- texture.minLod = sw::clamp(sampler->minLod, 0.0f, (float) (sw::MAX_TEXTURE_LOD));
- texture.maxLod = sw::clamp(sampler->maxLod, 0.0f, (float) (sw::MAX_TEXTURE_LOD));
+ memcpy(&this->sampler, sampler, sizeof(*sampler));
+
+ texture.minLod = sampler->minLod;
+ texture.maxLod = sampler->maxLod;
}
}
diff --git a/src/Vulkan/VkDescriptorSetLayout.hpp b/src/Vulkan/VkDescriptorSetLayout.hpp
index 627922a..e63bb4f 100644
--- a/src/Vulkan/VkDescriptorSetLayout.hpp
+++ b/src/Vulkan/VkDescriptorSetLayout.hpp
@@ -32,9 +32,9 @@
void updateSampler(const vk::Sampler *sampler);
// TODO(b/129523279): Minimize to the data actually needed.
- const vk::Sampler *sampler;
- const vk::ImageView *imageView;
+ vk::Sampler sampler;
+ const vk::ImageView *imageView;
alignas(16) sw::Texture texture;
};
diff --git a/src/Vulkan/VkSampler.hpp b/src/Vulkan/VkSampler.hpp
index 09acf4b..551621d 100644
--- a/src/Vulkan/VkSampler.hpp
+++ b/src/Vulkan/VkSampler.hpp
@@ -16,6 +16,8 @@
#define VK_SAMPLER_HPP_
#include "VkDevice.hpp"
+#include "Device/Config.hpp"
+#include "System/Math.hpp"
#include <atomic>
@@ -37,8 +39,8 @@
maxAnisotropy(pCreateInfo->maxAnisotropy),
compareEnable(pCreateInfo->compareEnable),
compareOp(pCreateInfo->compareOp),
- minLod(pCreateInfo->minLod),
- maxLod(pCreateInfo->maxLod),
+ minLod(ClampLod(pCreateInfo->minLod)),
+ maxLod(ClampLod(pCreateInfo->maxLod)),
borderColor(pCreateInfo->borderColor),
unnormalizedCoordinates(pCreateInfo->unnormalizedCoordinates)
{
@@ -51,6 +53,12 @@
return 0;
}
+ // Prevents accessing mipmap levels out of range.
+ static float ClampLod(float lod)
+ {
+ return sw::clamp(lod, 0.0f, (float)(sw::MAX_TEXTURE_LOD));
+ }
+
const uint32_t id = nextID++;
const VkFilter magFilter = VK_FILTER_NEAREST;
const VkFilter minFilter = VK_FILTER_NEAREST;
