Fix point rendering

- Point size limits were left at [0,0], forcing all points to zero coverage.
  Introduce a config value for the maximum, and plumb this through to drive
  physical device limits query.

- Fix all interpolants being replaced with pointcoord

Bug: b/124177079

Change-Id: I281dd3214537f15858afbd3890cf70f8850fa4aa
Reviewed-on: https://swiftshader-review.googlesource.com/c/25489
Tested-by: Chris Forbes <chrisforbes@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>

diff --git a/src/Device/Renderer.cpp b/src/Device/Renderer.cpp
index 405fe4b..a624597 100644 (file)
--- a/src/Device/Renderer.cpp
+++ b/src/Device/Renderer.cpp
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <VkConfig.h>
 #include "Renderer.hpp"
 
 #include "Clipper.hpp"
@@ -365,12 +366,6 @@ namespace sw
                        data->stencil[1] = stencilCCW;
                }
 
-               if(setupState.isDrawPoint)
-               {
-                       data->pointSizeMin = pointSizeMin;
-                       data->pointSizeMax = pointSizeMax;
-               }
-
                data->lineWidth = context->lineWidth;
 
                data->factor = factor;
@@ -1455,7 +1450,7 @@ namespace sw
 
                float pSize = v.builtins.pointSize;
 
-               pSize = clamp(pSize, data.pointSizeMin, data.pointSizeMax);
+               pSize = clamp(pSize, 1.0f, static_cast<float>(vk::MAX_POINT_SIZE));
 
                float4 P[4];
                int C[4];

diff --git a/src/Device/Renderer.hpp b/src/Device/Renderer.hpp
index e31e6de..013bc3d 100644 (file)
--- a/src/Device/Renderer.hpp
+++ b/src/Device/Renderer.hpp
@@ -145,9 +145,6 @@ namespace sw
                PS ps;
 
                int instanceID;
-
-               float pointSizeMin;
-               float pointSizeMax;
                float lineWidth;
 
                PixelProcessor::Stencil stencil[2];   // clockwise, counterclockwise

diff --git a/src/Device/VertexProcessor.cpp b/src/Device/VertexProcessor.cpp
index 49aa7cb..91e6c07 100644 (file)
--- a/src/Device/VertexProcessor.cpp
+++ b/src/Device/VertexProcessor.cpp
@@ -271,16 +271,6 @@ namespace sw
                else ASSERT(false);
        }
 
-       void VertexProcessor::setPointSizeMin(float pointSizeMin)
-       {
-               this->pointSizeMin = pointSizeMin;
-       }
-
-       void VertexProcessor::setPointSizeMax(float pointSizeMax)
-       {
-               this->pointSizeMax = pointSizeMax;
-       }
-
        void VertexProcessor::setRoutineCacheSize(int cacheSize)
        {
                delete routineCache;

diff --git a/src/Device/VertexProcessor.hpp b/src/Device/VertexProcessor.hpp
index 5c28dc1..62c8e2a 100644 (file)
--- a/src/Device/VertexProcessor.hpp
+++ b/src/Device/VertexProcessor.hpp
@@ -112,18 +112,12 @@ namespace sw
                void setMinLod(unsigned int sampler, float minLod);
                void setMaxLod(unsigned int sampler, float maxLod);
 
-               void setPointSizeMin(float pointSizeMin);
-               void setPointSizeMax(float pointSizeMax);
-
        protected:
                const State update(DrawType drawType);
                Routine *routine(const State &state);
 
                void setRoutineCacheSize(int cacheSize);
 
-               float pointSizeMin;
-               float pointSizeMax;
-
        private:
                Context *const context;
 

diff --git a/src/Pipeline/SetupRoutine.cpp b/src/Pipeline/SetupRoutine.cpp
index 67c861c..600bb0c 100644 (file)
--- a/src/Pipeline/SetupRoutine.cpp
+++ b/src/Pipeline/SetupRoutine.cpp
@@ -454,13 +454,17 @@ namespace sw
 
                        for (int interpolant = 0; interpolant < MAX_INTERFACE_COMPONENTS; interpolant++)
                        {
-                               // TODO: fix point, perspective, etc. Not convinced various edge cases are really correct here for either VK or GL.
+                               // TODO: fix `perspective` throughout interpolation code to consider NoPerspective-decorated interpolants,
+                               // which were not individually-controllable in the GLES implementation.
+                               // Note: `sprite` mode controls whether to replace this interpolant with the point sprite PointCoord value.
+                               // This was an interesting thing to support for old GL because any texture coordinate could be replaced in this way.
+                               // In modern GL and in Vulkan, the [gl_]PointCoord builtin variable to the fragment shader is used instead.
                                if (state.gradient[interpolant].Type != SpirvShader::ATTRIBTYPE_UNUSED)
                                        setupGradient(primitive, tri, w012, M, v0, v1, v2,
                                                        OFFSET(Vertex, v[interpolant]),
                                                        OFFSET(Primitive, V[interpolant]),
                                                        state.gradient[interpolant].Flat,
-                                                       point,
+                                                       false /* is pointcoord */,
                                                        state.perspective, 0);
                        }
 

diff --git a/src/Pipeline/VertexRoutine.cpp b/src/Pipeline/VertexRoutine.cpp
index 613ede3..803c24d 100644 (file)
--- a/src/Pipeline/VertexRoutine.cpp
+++ b/src/Pipeline/VertexRoutine.cpp
@@ -672,6 +672,17 @@ namespace sw
                *Pointer<Float4>(cacheLine + OFFSET(Vertex,projected) + sizeof(Vertex) * 1, 16) = v.y;
                *Pointer<Float4>(cacheLine + OFFSET(Vertex,projected) + sizeof(Vertex) * 2, 16) = v.z;
                *Pointer<Float4>(cacheLine + OFFSET(Vertex,projected) + sizeof(Vertex) * 3, 16) = v.w;
+
+               it = spirvShader->outputBuiltins.find(spv::BuiltInPointSize);
+               if (it != spirvShader->outputBuiltins.end())
+               {
+                       assert(it->second.SizeInComponents == 1);
+                       auto psize = routine.getValue(it->second.Id)[it->second.FirstComponent];
+                       *Pointer<Float>(cacheLine + OFFSET(Vertex,builtins.pointSize) + sizeof(Vertex) * 0) = Extract(psize, 0);
+                       *Pointer<Float>(cacheLine + OFFSET(Vertex,builtins.pointSize) + sizeof(Vertex) * 1) = Extract(psize, 1);
+                       *Pointer<Float>(cacheLine + OFFSET(Vertex,builtins.pointSize) + sizeof(Vertex) * 2) = Extract(psize, 2);
+                       *Pointer<Float>(cacheLine + OFFSET(Vertex,builtins.pointSize) + sizeof(Vertex) * 3) = Extract(psize, 3);
+               }
        }
 
        void VertexRoutine::writeVertex(const Pointer<Byte> &vertex, Pointer<Byte> &cache)

diff --git a/src/Vulkan/VkConfig.h b/src/Vulkan/VkConfig.h
index 70722b6..83c5e70 100644 (file)
--- a/src/Vulkan/VkConfig.h
+++ b/src/Vulkan/VkConfig.h
@@ -58,6 +58,11 @@ enum
        MAX_VERTEX_INPUT_BINDINGS = 16,
 };
 
+enum
+{
+       MAX_POINT_SIZE = 1,             // Large points are not supported. If/when we turn this on, must be >= 64.
+};
+
 }
 
 #endif // VK_CONFIG_HPP_

diff --git a/src/Vulkan/VkPhysicalDevice.cpp b/src/Vulkan/VkPhysicalDevice.cpp
index b9d29bf..2b0c71f 100644 (file)
--- a/src/Vulkan/VkPhysicalDevice.cpp
+++ b/src/Vulkan/VkPhysicalDevice.cpp
@@ -234,7 +234,7 @@ const VkPhysicalDeviceLimits& PhysicalDevice::getLimits() const
                8, // maxCullDistances
                8, // maxCombinedClipAndCullDistances
                2, // discreteQueuePriorities
-               { 1.0, 64.0 }, // pointSizeRange[2]
+               { 1.0, vk::MAX_POINT_SIZE }, // pointSizeRange[2]
                { 1.0, 1.0 }, // lineWidthRange[2] (unsupported)
                0.0, // pointSizeGranularity (unsupported)
                0.0, // lineWidthGranularity (unsupported)