Skip to content

S
swiftshader
Commit 30847688 by Alexis Hetu Committed by Alexis Hétu
Options

Renderer alignment fix

The Renderer needs to be aligned when allocated, otherwise it leads to all sorts of hard to track issues in the 32 bit version of the library. In this instance, Angle's end to end tests on SwiftShader Vulkan had spurious failures due to having unaligned members in PixelProcessor, which wasn't aligned because it's used as a base class of the Renderer class, which wasn't aligned when allocated. Using an allocator to align the memory fixes the issue. I also did a small cleanup of PixelProcessor::setBlendConstant, as it was a bit hard to debug and changing it this way made it easier to track what was going on. Bug b/140888415 Change-Id: Ie5559642456ee4dcb73f9d07c777349f558879ba Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/36248 Presubmit-Ready: Alexis Hétu <sugoi@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com> Tested-by: 's avatarAlexis Hétu <sugoi@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
parent 6fce4a24
Showing with 85 additions and 81 deletions
src/Device/PixelProcessor.cpp
...@@ -62,96 +62,85 @@ namespace sw ...@@ -62,96 +62,85 @@ namespace sw
void PixelProcessor::setBlendConstant(const Color<float> &blendConstant) void PixelProcessor::setBlendConstant(const Color<float> &blendConstant)
{ {
// FIXME: Compact into generic function // FIXME: Clamp // TODO(b/140935644): Compact into generic function, cheack if clamp is required
short blendConstantR = static_cast<short>(iround(65535 * blendConstant.r)); factor.blendConstant4W[0][0] =
short blendConstantG = static_cast<short>(iround(65535 * blendConstant.g)); factor.blendConstant4W[0][1] =
short blendConstantB = static_cast<short>(iround(65535 * blendConstant.b)); factor.blendConstant4W[0][2] =
short blendConstantA = static_cast<short>(iround(65535 * blendConstant.a)); factor.blendConstant4W[0][3] = static_cast<uint16_t>(iround(65535.0f * blendConstant.r));
factor.blendConstant4W[0][0] = blendConstantR; factor.blendConstant4W[1][0] =
factor.blendConstant4W[0][1] = blendConstantR; factor.blendConstant4W[1][1] =
factor.blendConstant4W[0][2] = blendConstantR; factor.blendConstant4W[1][2] =
factor.blendConstant4W[0][3] = blendConstantR; factor.blendConstant4W[1][3] = static_cast<uint16_t>(iround(65535.0f * blendConstant.g));
factor.blendConstant4W[1][0] = blendConstantG; factor.blendConstant4W[2][0] =
factor.blendConstant4W[1][1] = blendConstantG; factor.blendConstant4W[2][1] =
factor.blendConstant4W[1][2] = blendConstantG; factor.blendConstant4W[2][2] =
factor.blendConstant4W[1][3] = blendConstantG; factor.blendConstant4W[2][3] = static_cast<uint16_t>(iround(65535.0f * blendConstant.b));
factor.blendConstant4W[2][0] = blendConstantB; factor.blendConstant4W[3][0] =
factor.blendConstant4W[2][1] = blendConstantB; factor.blendConstant4W[3][1] =
factor.blendConstant4W[2][2] = blendConstantB; factor.blendConstant4W[3][2] =
factor.blendConstant4W[2][3] = blendConstantB; factor.blendConstant4W[3][3] = static_cast<uint16_t>(iround(65535.0f * blendConstant.a));
factor.blendConstant4W[3][0] = blendConstantA; factor.invBlendConstant4W[0][0] =
factor.blendConstant4W[3][1] = blendConstantA; factor.invBlendConstant4W[0][1] =
factor.blendConstant4W[3][2] = blendConstantA; factor.invBlendConstant4W[0][2] =
factor.blendConstant4W[3][3] = blendConstantA; factor.invBlendConstant4W[0][3] = 0xFFFFu - factor.blendConstant4W[0][0];
// FIXME: Compact into generic function // FIXME: Clamp factor.invBlendConstant4W[1][0] =
short invBlendConstantR = static_cast<short>(iround(65535 * (1 - blendConstant.r))); factor.invBlendConstant4W[1][1] =
short invBlendConstantG = static_cast<short>(iround(65535 * (1 - blendConstant.g))); factor.invBlendConstant4W[1][2] =
short invBlendConstantB = static_cast<short>(iround(65535 * (1 - blendConstant.b))); factor.invBlendConstant4W[1][3] = 0xFFFFu - factor.blendConstant4W[1][0];
short invBlendConstantA = static_cast<short>(iround(65535 * (1 - blendConstant.a)));
factor.invBlendConstant4W[2][0] =
factor.invBlendConstant4W[0][0] = invBlendConstantR; factor.invBlendConstant4W[2][1] =
factor.invBlendConstant4W[0][1] = invBlendConstantR; factor.invBlendConstant4W[2][2] =
factor.invBlendConstant4W[0][2] = invBlendConstantR; factor.invBlendConstant4W[2][3] = 0xFFFFu - factor.blendConstant4W[2][0];
factor.invBlendConstant4W[0][3] = invBlendConstantR;
factor.invBlendConstant4W[3][0] =
factor.invBlendConstant4W[1][0] = invBlendConstantG; factor.invBlendConstant4W[3][1] =
factor.invBlendConstant4W[1][1] = invBlendConstantG; factor.invBlendConstant4W[3][2] =
factor.invBlendConstant4W[1][2] = invBlendConstantG; factor.invBlendConstant4W[3][3] = 0xFFFFu - factor.blendConstant4W[3][0];
factor.invBlendConstant4W[1][3] = invBlendConstantG;
factor.blendConstant4F[0][0] =
factor.invBlendConstant4W[2][0] = invBlendConstantB; factor.blendConstant4F[0][1] =
factor.invBlendConstant4W[2][1] = invBlendConstantB; factor.blendConstant4F[0][2] =
factor.invBlendConstant4W[2][2] = invBlendConstantB;
factor.invBlendConstant4W[2][3] = invBlendConstantB;
factor.invBlendConstant4W[3][0] = invBlendConstantA;
factor.invBlendConstant4W[3][1] = invBlendConstantA;
factor.invBlendConstant4W[3][2] = invBlendConstantA;
factor.invBlendConstant4W[3][3] = invBlendConstantA;
factor.blendConstant4F[0][0] = blendConstant.r;
factor.blendConstant4F[0][1] = blendConstant.r;
factor.blendConstant4F[0][2] = blendConstant.r;
factor.blendConstant4F[0][3] = blendConstant.r; factor.blendConstant4F[0][3] = blendConstant.r;
factor.blendConstant4F[1][0] = blendConstant.g; factor.blendConstant4F[1][0] =
factor.blendConstant4F[1][1] = blendConstant.g; factor.blendConstant4F[1][1] =
factor.blendConstant4F[1][2] = blendConstant.g; factor.blendConstant4F[1][2] =
factor.blendConstant4F[1][3] = blendConstant.g; factor.blendConstant4F[1][3] = blendConstant.g;
factor.blendConstant4F[2][0] = blendConstant.b; factor.blendConstant4F[2][0] =
factor.blendConstant4F[2][1] = blendConstant.b; factor.blendConstant4F[2][1] =
factor.blendConstant4F[2][2] = blendConstant.b; factor.blendConstant4F[2][2] =
factor.blendConstant4F[2][3] = blendConstant.b; factor.blendConstant4F[2][3] = blendConstant.b;
factor.blendConstant4F[3][0] = blendConstant.a; factor.blendConstant4F[3][0] =
factor.blendConstant4F[3][1] = blendConstant.a; factor.blendConstant4F[3][1] =
factor.blendConstant4F[3][2] = blendConstant.a; factor.blendConstant4F[3][2] =
factor.blendConstant4F[3][3] = blendConstant.a; factor.blendConstant4F[3][3] = blendConstant.a;
factor.invBlendConstant4F[0][0] = 1 - blendConstant.r; factor.invBlendConstant4F[0][0] =
factor.invBlendConstant4F[0][1] = 1 - blendConstant.r; factor.invBlendConstant4F[0][1] =
factor.invBlendConstant4F[0][2] = 1 - blendConstant.r; factor.invBlendConstant4F[0][2] =
factor.invBlendConstant4F[0][3] = 1 - blendConstant.r; factor.invBlendConstant4F[0][3] = 1 - blendConstant.r;
factor.invBlendConstant4F[1][0] = 1 - blendConstant.g; factor.invBlendConstant4F[1][0] =
factor.invBlendConstant4F[1][1] = 1 - blendConstant.g; factor.invBlendConstant4F[1][1] =
factor.invBlendConstant4F[1][2] = 1 - blendConstant.g; factor.invBlendConstant4F[1][2] =
factor.invBlendConstant4F[1][3] = 1 - blendConstant.g; factor.invBlendConstant4F[1][3] = 1 - blendConstant.g;
factor.invBlendConstant4F[2][0] = 1 - blendConstant.b; factor.invBlendConstant4F[2][0] =
factor.invBlendConstant4F[2][1] = 1 - blendConstant.b; factor.invBlendConstant4F[2][1] =
factor.invBlendConstant4F[2][2] = 1 - blendConstant.b; factor.invBlendConstant4F[2][2] =
factor.invBlendConstant4F[2][3] = 1 - blendConstant.b; factor.invBlendConstant4F[2][3] = 1 - blendConstant.b;
factor.invBlendConstant4F[3][0] = 1 - blendConstant.a; factor.invBlendConstant4F[3][0] =
factor.invBlendConstant4F[3][1] = 1 - blendConstant.a; factor.invBlendConstant4F[3][1] =
factor.invBlendConstant4F[3][2] = 1 - blendConstant.a; factor.invBlendConstant4F[3][2] =
factor.invBlendConstant4F[3][3] = 1 - blendConstant.a; factor.invBlendConstant4F[3][3] = 1 - blendConstant.a;
} }
......
src/Device/Renderer.cpp
...@@ -159,6 +159,18 @@ namespace sw ...@@ -159,6 +159,18 @@ namespace sw
drawTickets.take().wait(); drawTickets.take().wait();
} }
// Renderer objects have to be mem aligned to the alignment provided in the class declaration
void* Renderer::operator new(size_t size)
{
ASSERT(size == sizeof(Renderer)); // This operator can't be called from a derived class
return vk::allocate(sizeof(Renderer), alignof(Renderer), vk::DEVICE_MEMORY, VK_SYSTEM_ALLOCATION_SCOPE_DEVICE);
}
void Renderer::operator delete(void* mem)
{
vk::deallocate(mem, vk::DEVICE_MEMORY);
}
void Renderer::draw(const sw::Context* context, VkIndexType indexType, unsigned int count, int baseVertex, void Renderer::draw(const sw::Context* context, VkIndexType indexType, unsigned int count, int baseVertex,
TaskEvents *events, int instanceID, int viewID, void *indexBuffer, TaskEvents *events, int instanceID, int viewID, void *indexBuffer,
PushConstantStorage const & pushConstants, bool update) PushConstantStorage const & pushConstants, bool update)
......
src/Device/Renderer.hpp
...@@ -189,13 +189,16 @@ namespace sw ...@@ -189,13 +189,16 @@ namespace sw
static bool setupPoint(Primitive &primitive, Triangle &triangle, const DrawCall &draw); static bool setupPoint(Primitive &primitive, Triangle &triangle, const DrawCall &draw);
}; };
class Renderer : public VertexProcessor, public PixelProcessor, public SetupProcessor class alignas(16) Renderer : public VertexProcessor, public PixelProcessor, public SetupProcessor
{ {
public: public:
Renderer(vk::Device* device); Renderer(vk::Device* device);
virtual ~Renderer(); virtual ~Renderer();
void* operator new(size_t size);
void operator delete(void* mem);
bool hasOcclusionQuery() const { return occlusionQuery != nullptr; } bool hasOcclusionQuery() const { return occlusionQuery != nullptr; }
void draw(const sw::Context* context, VkIndexType indexType, unsigned int count, int baseVertex, void draw(const sw::Context* context, VkIndexType indexType, unsigned int count, int baseVertex,
......
src/Vulkan/VkCommandBuffer.cpp
...@@ -526,12 +526,12 @@ struct DrawBase : public CommandBuffer::Command ...@@ -526,12 +526,12 @@ struct DrawBase : public CommandBuffer::Command
// Apply either pipeline state or dynamic state // Apply either pipeline state or dynamic state
executionState.renderer->setScissor(pipeline->hasDynamicState(VK_DYNAMIC_STATE_SCISSOR) ? executionState.renderer->setScissor(pipeline->hasDynamicState(VK_DYNAMIC_STATE_SCISSOR) ?
executionState.dynamicState.scissor : pipeline->getScissor()); executionState.dynamicState.scissor : pipeline->getScissor());
executionState.renderer->setViewport(pipeline->hasDynamicState(VK_DYNAMIC_STATE_VIEWPORT) ? executionState.renderer->setViewport(pipeline->hasDynamicState(VK_DYNAMIC_STATE_VIEWPORT) ?
executionState.dynamicState.viewport : pipeline->getViewport()); executionState.dynamicState.viewport : pipeline->getViewport());
executionState.renderer->setBlendConstant(pipeline->hasDynamicState(VK_DYNAMIC_STATE_BLEND_CONSTANTS) ? executionState.renderer->setBlendConstant(pipeline->hasDynamicState(VK_DYNAMIC_STATE_BLEND_CONSTANTS) ?
executionState.dynamicState.blendConstants executionState.dynamicState.blendConstants : pipeline->getBlendConstants());
: pipeline->getBlendConstants());
if (pipeline->hasDynamicState(VK_DYNAMIC_STATE_DEPTH_BIAS)) if (pipeline->hasDynamicState(VK_DYNAMIC_STATE_DEPTH_BIAS))
{ {
// If the depth bias clamping feature is not enabled, depthBiasClamp must be 0.0 // If the depth bias clamping feature is not enabled, depthBiasClamp must be 0.0
......
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