fix(renderer): Interleave positive and negative feather atlas triangles (#11654) 37764336db We were rendering the atlas with all negative triangles first, followed by all positive triangles. This is the worst possible ordering for maintaining high precision in a 16-bit coverage buffer, since as the magnitude accumulates, we lose the lower bits. Instead, render the positive/negative triangles interleaved, in the same order naturally come in. This requires an extra check in the fragment shader for whether the triangle is clockwise. Co-authored-by: Chris Dalton <99840794+csmartdalton@users.noreply.github.com>
diff --git a/.rive_head b/.rive_head index bb099fd..06926e6 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +1 @@ -aa788cab462e8de4438e619a4047c7e6e39fcabb +37764336dbdb4b78d2be6842cd9cf34dacf996f1
diff --git a/renderer/include/rive/renderer/d3d11/render_context_d3d_impl.hpp b/renderer/include/rive/renderer/d3d11/render_context_d3d_impl.hpp index 1f61a45..f8136a9 100644 --- a/renderer/include/rive/renderer/d3d11/render_context_d3d_impl.hpp +++ b/renderer/include/rive/renderer/d3d11/render_context_d3d_impl.hpp
@@ -284,7 +284,8 @@ ComPtr<ID3D11SamplerState> m_samplerStates[rive::ImageSampler::MAX_SAMPLER_PERMUTATIONS]; - ComPtr<ID3D11RasterizerState> m_atlasRasterState; + ComPtr<ID3D11RasterizerState> m_atlasFillRasterState; + ComPtr<ID3D11RasterizerState> m_atlasStrokeRasterState; ComPtr<ID3D11RasterizerState> m_backCulledRasterState[2]; ComPtr<ID3D11RasterizerState> m_doubleSidedRasterState[2];
diff --git a/renderer/include/rive/renderer/gpu.hpp b/renderer/include/rive/renderer/gpu.hpp index 6b478b8..0b02f64 100644 --- a/renderer/include/rive/renderer/gpu.hpp +++ b/renderer/include/rive/renderer/gpu.hpp
@@ -1871,7 +1871,7 @@ .depthWriteEnabled = false, .stencilTestEnabled = false, .stencilWriteMask = 0, - .cullFace = CullFace::counterclockwise, + .cullFace = CullFace::none, .blendEquation = BlendEquation::plus, .colorWriteEnabled = true, };
diff --git a/renderer/src/d3d11/render_context_d3d_impl.cpp b/renderer/src/d3d11/render_context_d3d_impl.cpp index e9d3ca6..455ad56 100644 --- a/renderer/src/d3d11/render_context_d3d_impl.cpp +++ b/renderer/src/d3d11/render_context_d3d_impl.cpp
@@ -532,11 +532,18 @@ &rasterDesc, m_backCulledRasterState[0].ReleaseAndGetAddressOf())); - // ...And with scissor for the atlas. + // ...And with scissor and no culling for the atlas fill. + rasterDesc.CullMode = D3D11_CULL_NONE; rasterDesc.ScissorEnable = TRUE; VERIFY_OK(m_gpu->CreateRasterizerState( &rasterDesc, - m_atlasRasterState.ReleaseAndGetAddressOf())); + m_atlasFillRasterState.ReleaseAndGetAddressOf())); + + // ...And with culling back on for the atlas stroke. + rasterDesc.CullMode = D3D11_CULL_BACK; + VERIFY_OK(m_gpu->CreateRasterizerState( + &rasterDesc, + m_atlasStrokeRasterState.ReleaseAndGetAddressOf())); // ...And with wireframe for debugging. rasterDesc.ScissorEnable = FALSE; @@ -1664,7 +1671,6 @@ m_gpuContext->IASetIndexBuffer(m_patchIndexBuffer.Get(), DXGI_FORMAT_R16_UINT, 0); - m_gpuContext->RSSetState(m_atlasRasterState.Get()); D3D11_VIEWPORT viewport = {0, 0, @@ -1680,6 +1686,7 @@ if (desc.atlasFillBatchCount != 0) { + m_gpuContext->RSSetState(m_atlasFillRasterState.Get()); m_pipelineManager.setAtlasFillState(); m_gpuContext->OMSetBlendState(m_plusBlendState.Get(), NULL, @@ -1707,6 +1714,7 @@ if (desc.atlasStrokeBatchCount != 0) { + m_gpuContext->RSSetState(m_atlasStrokeRasterState.Get()); m_pipelineManager.setAtlasStrokeState(); m_gpuContext->OMSetBlendState(m_maxBlendState.Get(), NULL,
diff --git a/renderer/src/d3d12/d3d12_pipeline_manager.cpp b/renderer/src/d3d12/d3d12_pipeline_manager.cpp index e7505da..6d44159 100644 --- a/renderer/src/d3d12/d3d12_pipeline_manager.cpp +++ b/renderer/src/d3d12/d3d12_pipeline_manager.cpp
@@ -444,6 +444,7 @@ psoDesc.BlendState.RenderTarget[0].BlendOp = D3D12_BLEND_OP_ADD; psoDesc.BlendState.RenderTarget[0].BlendOpAlpha = D3D12_BLEND_OP_ADD; + psoDesc.RasterizerState.CullMode = D3D12_CULL_MODE_NONE; psoDesc.PS = {shader::atlas::fill::g_main, std::size(shader::atlas::fill::g_main)};
diff --git a/renderer/src/draw.cpp b/renderer/src/draw.cpp index 37946f6..26ade25 100644 --- a/renderer/src/draw.cpp +++ b/renderer/src/draw.cpp
@@ -635,7 +635,8 @@ if (det < 0) { m_contourDirections = - m_coverageType == CoverageType::msaa + (m_coverageType == CoverageType::msaa || + m_coverageType == CoverageType::atlas) ? gpu::ContourDirections::reverse : gpu::ContourDirections::forwardThenReverse; m_contourFlags |= NEGATE_PATH_FILL_COVERAGE_FLAG; // ignored by msaa @@ -643,7 +644,8 @@ else { m_contourDirections = - m_coverageType == CoverageType::msaa + (m_coverageType == CoverageType::msaa || + m_coverageType == CoverageType::atlas) ? gpu::ContourDirections::forward : gpu::ContourDirections::reverseThenForward; } @@ -657,12 +659,18 @@ // For clockwiseFill, this is also our opportunity to logically // reverse the winding of the path, if it is predominantly // counterclockwise. - m_contourDirections = gpu::ContourDirections::forwardThenReverse; + m_contourDirections = + (m_coverageType == CoverageType::atlas) + ? gpu::ContourDirections::reverse + : gpu::ContourDirections::forwardThenReverse; m_contourFlags |= NEGATE_PATH_FILL_COVERAGE_FLAG; } else { - m_contourDirections = gpu::ContourDirections::reverseThenForward; + m_contourDirections = + (m_coverageType == CoverageType::atlas) + ? gpu::ContourDirections::forward + : gpu::ContourDirections::reverseThenForward; } } else
diff --git a/renderer/src/metal/render_context_metal_impl.mm b/renderer/src/metal/render_context_metal_impl.mm index d56ac09..2e01674 100644 --- a/renderer/src/metal/render_context_metal_impl.mm +++ b/renderer/src/metal/render_context_metal_impl.mm
@@ -1390,10 +1390,10 @@ [atlasEncoder setVertexBuffer:m_pathPatchVertexBuffer offset:0 atIndex:0]; - [atlasEncoder setCullMode:MTLCullModeBack]; if (desc.atlasFillBatchCount != 0) { + [atlasEncoder setCullMode:MTLCullModeNone]; [atlasEncoder setRenderPipelineState:atlasFillpipelineState]; for (size_t i = 0; i < desc.atlasFillBatchCount; ++i) { @@ -1419,6 +1419,7 @@ if (desc.atlasStrokeBatchCount != 0) { + [atlasEncoder setCullMode:MTLCullModeBack]; [atlasEncoder setRenderPipelineState:atlasStrokepipelineState]; for (size_t i = 0; i < desc.atlasStrokeBatchCount; ++i) {
diff --git a/renderer/src/shaders/glsl.glsl b/renderer/src/shaders/glsl.glsl index 4f9a938..7463892 100644 --- a/renderer/src/shaders/glsl.glsl +++ b/renderer/src/shaders/glsl.glsl
@@ -550,6 +550,10 @@ layout(location = 0) out DATA_TYPE _fd; \ void main() +#define FRAG_DATA_MAIN_WITH_CLOCKWISE FRAG_DATA_MAIN + +#define _clockwise gl_FrontFacing + #define EMIT_FRAG_DATA(VALUE) _fd = VALUE #define _fragCoord gl_FragCoord.xy
diff --git a/renderer/src/shaders/hlsl.glsl b/renderer/src/shaders/hlsl.glsl index 19f6f91..1482ef7 100644 --- a/renderer/src/shaders/hlsl.glsl +++ b/renderer/src/shaders/hlsl.glsl
@@ -271,6 +271,11 @@ DATA_TYPE main(Varyings _varyings) : $SV_Target \ { +#define FRAG_DATA_MAIN_WITH_CLOCKWISE(DATA_TYPE, NAME) \ + DATA_TYPE main(Varyings _varyings, bool _clockwise : $SV_IsFrontFace) : \ + $SV_Target \ + { + #define EMIT_FRAG_DATA(VALUE) \ return VALUE; \ }
diff --git a/renderer/src/shaders/metal.glsl b/renderer/src/shaders/metal.glsl index afcbb1d..2325a03 100644 --- a/renderer/src/shaders/metal.glsl +++ b/renderer/src/shaders/metal.glsl
@@ -253,6 +253,13 @@ FragmentTextures _textures) \ { +#define FRAG_DATA_MAIN_WITH_CLOCKWISE(DATA_TYPE, NAME) \ + DATA_TYPE $__attribute__(($visibility("default"))) $fragment NAME( \ + Varyings _varyings [[$stage_in]], \ + FragmentTextures _textures, \ + bool _clockwise [[$front_facing]]) \ + { + #define EMIT_FRAG_DATA(VALUE) \ return VALUE; \ }
diff --git a/renderer/src/shaders/render_atlas.glsl b/renderer/src/shaders/render_atlas.glsl index 4c3dafb..8135e25 100644 --- a/renderer/src/shaders/render_atlas.glsl +++ b/renderer/src/shaders/render_atlas.glsl
@@ -59,6 +59,17 @@ #ifdef @FRAGMENT +#ifdef @ATLAS_FEATHERED_FILL +INLINE half signed_fill_coverage(float4 coverages, + bool clockwise TEXTURE_CONTEXT_DECL) +{ + half coverage = eval_feathered_fill(coverages TEXTURE_CONTEXT_FORWARD); + if (!clockwise) + coverage = -coverage; + return coverage; +} +#endif + #ifdef @ATLAS_RENDER_TARGET_R32UI_FRAMEBUFFER_FETCH // Store coverage as fp32 data bits in an r32ui color buffer, and use @@ -69,7 +80,8 @@ void main() { float coverage = uintBitsToFloat(_fragCoverage.r); - coverage += eval_feathered_fill(v_coverages); + coverage += signed_fill_coverage(v_coverages, + gl_FrontFacing TEXTURE_CONTEXT_FORWARD); _fragCoverage.r = floatBitsToUint(coverage); } #endif @@ -90,7 +102,12 @@ __pixel_localEXT PLS { layout(r32f) highp float _plsCoverage; }; #ifdef @ATLAS_FEATHERED_FILL -void main() { _plsCoverage += eval_feathered_fill(v_coverages); } +void main() +{ + _plsCoverage += + signed_fill_coverage(v_coverages, + gl_FrontFacing TEXTURE_CONTEXT_FORWARD); +} #endif #ifdef @ATLAS_FEATHERED_STROKE @@ -110,7 +127,8 @@ void main() { float coverage = uintBitsToFloat(pixelLocalLoadANGLE(_plsCoverage).r); - coverage += eval_feathered_fill(v_coverages); + coverage += signed_fill_coverage(v_coverages, + gl_FrontFacing TEXTURE_CONTEXT_FORWARD); pixelLocalStoreANGLE(_plsCoverage, uint4(floatBitsToUint(coverage))); } #endif @@ -138,7 +156,9 @@ #ifdef @ATLAS_FEATHERED_FILL void main() { - int coverage = fixedpoint_coverage(eval_feathered_fill(v_coverages)); + int coverage = fixedpoint_coverage( + signed_fill_coverage(v_coverages, + gl_FrontFacing TEXTURE_CONTEXT_FORWARD)); imageAtomicAdd(_atlasImage, image_coord(), coverage); } #endif @@ -157,10 +177,11 @@ // rare.). Just split up coverage across rgba8 components and hope for the best. #ifdef @ATLAS_FEATHERED_FILL -FRAG_DATA_MAIN(half4, @atlasFillFragmentMain) +FRAG_DATA_MAIN_WITH_CLOCKWISE(half4, @atlasFillFragmentMain) { VARYING_UNPACK(v_coverages, float4); - half coverage = eval_feathered_fill(v_coverages TEXTURE_CONTEXT_FORWARD); + half coverage = + signed_fill_coverage(v_coverages, _clockwise TEXTURE_CONTEXT_FORWARD); // i.e., is abs(coverage) ~= FEATHER(1), allowing for some sub-8-bit slop in // the texture unit performing a clamp to edge. if (abs(coverage) > MAX_FEATHER - 1e-3) @@ -207,10 +228,11 @@ // hardware count the coverage. #ifdef @ATLAS_FEATHERED_FILL -FRAG_DATA_MAIN(float, @atlasFillFragmentMain) +FRAG_DATA_MAIN_WITH_CLOCKWISE(float, @atlasFillFragmentMain) { VARYING_UNPACK(v_coverages, float4); - EMIT_FRAG_DATA(eval_feathered_fill(v_coverages TEXTURE_CONTEXT_FORWARD)); + EMIT_FRAG_DATA( + signed_fill_coverage(v_coverages, _clockwise TEXTURE_CONTEXT_FORWARD)); } #endif
diff --git a/renderer/src/vulkan/common_layouts.hpp b/renderer/src/vulkan/common_layouts.hpp index 617803c..c21fecd 100644 --- a/renderer/src/vulkan/common_layouts.hpp +++ b/renderer/src/vulkan/common_layouts.hpp
@@ -156,6 +156,14 @@ .lineWidth = 1.f, }; +constexpr VkPipelineRasterizationStateCreateInfo RASTER_STATE_CULL_NONE_CW = { + .sType = VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO, + .polygonMode = VK_POLYGON_MODE_FILL, + .cullMode = VK_CULL_MODE_NONE, + .frontFace = VK_FRONT_FACE_CLOCKWISE, + .lineWidth = 1.f, +}; + constexpr VkPipelineMultisampleStateCreateInfo MSAA_DISABLED = { .sType = VK_STRUCTURE_TYPE_PIPELINE_MULTISAMPLE_STATE_CREATE_INFO, .rasterizationSamples = VK_SAMPLE_COUNT_1_BIT,
diff --git a/renderer/src/vulkan/render_context_vulkan_impl.cpp b/renderer/src/vulkan/render_context_vulkan_impl.cpp index c39d555..fd131e8 100644 --- a/renderer/src/vulkan/render_context_vulkan_impl.cpp +++ b/renderer/src/vulkan/render_context_vulkan_impl.cpp
@@ -674,7 +674,7 @@ .pVertexInputState = &layout::PATH_VERTEX_INPUT_STATE, .pInputAssemblyState = &layout::INPUT_ASSEMBLY_TRIANGLE_LIST, .pViewportState = &layout::SINGLE_VIEWPORT, - .pRasterizationState = &layout::RASTER_STATE_CULL_BACK_CW, + .pRasterizationState = &layout::RASTER_STATE_CULL_NONE_CW, .pMultisampleState = &layout::MSAA_DISABLED, .pColorBlendState = &blendStateCreateInfo, .pDynamicState = &layout::DYNAMIC_VIEWPORT_SCISSOR,
diff --git a/renderer/src/webgpu/render_context_webgpu_impl.cpp b/renderer/src/webgpu/render_context_webgpu_impl.cpp index 4c8a3a3..daf94af 100644 --- a/renderer/src/webgpu/render_context_webgpu_impl.cpp +++ b/renderer/src/webgpu/render_context_webgpu_impl.cpp
@@ -864,7 +864,7 @@ { .topology = wgpu::PrimitiveTopology::TriangleList, .frontFace = RIVE_FRONT_FACE, - .cullMode = wgpu::CullMode::Back, + .cullMode = wgpu::CullMode::None, }, .fragment = &fragmentState, };
diff --git a/tests/add_gm_image.sh b/tests/add_gm_image.sh new file mode 100644 index 0000000..6e77ae4 --- /dev/null +++ b/tests/add_gm_image.sh
@@ -0,0 +1,6 @@ +set -x + +for f in `find ../../../zzzgold/ -name emptyclear.png`; do + cp $1 `dirname $f`/ + git add `dirname $f`/`basename $1` +done
diff --git a/tests/gm/feathertext.cpp b/tests/gm/feathertext.cpp index 0316a5f..59481f7 100644 --- a/tests/gm/feathertext.cpp +++ b/tests/gm/feathertext.cpp
@@ -26,8 +26,10 @@ class FeatherTextGM : public GM { public: - FeatherTextGM(rive::Span<uint8_t> fontBytes) : - GM(1600, 1840), m_text(TestingWindow::Get()->factory()) + FeatherTextGM(rive::Span<uint8_t> fontBytes, bool mirrored) : + GM(1600, 1840), + m_text(TestingWindow::Get()->factory()), + m_mirrored(mirrored) { m_paint->color(0xff000000); m_text.maxWidth(720); @@ -46,6 +48,14 @@ void onDraw(rive::Renderer* renderer) override { + if (m_mirrored) + { + // Draw with a negative determinant to ensure the atlas accounts for + // this in its signed coverage calculations. + renderer->translate(width(), 0); + renderer->scale(-1, 1); + } + renderer->translate(40, 40); for (int i = 0; i < 3; ++i) @@ -63,11 +73,16 @@ private: rive::RawText m_text; Paint m_paint; + const bool m_mirrored; }; GMREGISTER(feathertext_roboto, - return new FeatherTextGM(assets::roboto_flex_ttf())) + return new FeatherTextGM(assets::roboto_flex_ttf(), false)) GMREGISTER(feathertext_montserrat, - return new FeatherTextGM(assets::montserrat_ttf())) + return new FeatherTextGM(assets::montserrat_ttf(), false)) +GMREGISTER(feathertext_roboto_mirrored, + return new FeatherTextGM(assets::roboto_flex_ttf(), true)) +GMREGISTER(feathertext_montserrat_mirrored, + return new FeatherTextGM(assets::montserrat_ttf(), true)) #endif
diff --git a/tests/gm/gmmain.cpp b/tests/gm/gmmain.cpp index 3c8c2bc..a069626 100644 --- a/tests/gm/gmmain.cpp +++ b/tests/gm/gmmain.cpp
@@ -44,6 +44,8 @@ MAKE_GM(lots_of_images_sampled) MAKE_GM(feathertext_roboto) MAKE_GM(feathertext_montserrat) + MAKE_GM(feathertext_roboto_mirrored) + MAKE_GM(feathertext_montserrat_mirrored) // Add the normal (not slow) gms last. MAKE_GM(atlastypes)