Skip to content

Commit

Permalink
Fix integer precision issues with pathID in vertex shaders
Browse files Browse the repository at this point in the history 
pathID was stored as a ushort, but could overflow when multiplied by 4 to access path data. Convert it to a uint.

Diffs=
423301a95b Fix integer precision issues with pathID in vertex shaders (#8689)

Co-authored-by: Chris Dalton <99840794+csmartdalton@users.noreply.github.com>
  • Loading branch information
@csmartdalton
csmartdalton and csmartdalton committed Dec 5, 2024
1 parent 91b3e5c commit d771676
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .rive_head
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
b1c0e806252b66d81bfb79113a83decc00ddccd0
423301a95b5312d1d13dc6f394acc36a7b7a8607
8 changes: 6 additions & 2 deletions renderer/src/shaders/atomic_draw.glsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,16 @@ VERTEX_MAIN(@drawVertexMain, Attrs, attrs, _vertexID, _instanceID)
VARYING_INIT(v_pathID, ushort);

float4 pos;
uint pathID;
float2 vertexPosition;
if (unpack_tessellated_path_vertex(@a_patchVertexData,
@a_mirroredVertexData,
_instanceID,
v_pathID,
pathID,
vertexPosition,
v_edgeDistance VERTEX_CONTEXT_UNPACK))
{
v_pathID = cast_uint_to_ushort(pathID);
pos = RENDER_TARGET_COORD_TO_CLIP_COORD(vertexPosition);
}
else
Expand Down Expand Up @@ -72,10 +74,12 @@ VERTEX_MAIN(@drawVertexMain, Attrs, attrs, _vertexID, _instanceID)
VARYING_INIT(v_windingWeight, half);
VARYING_INIT(v_pathID, ushort);

uint pathID;
float2 vertexPosition =
unpack_interior_triangle_vertex(@a_triangleVertex,
v_pathID,
pathID,
v_windingWeight VERTEX_CONTEXT_UNPACK);
v_pathID = cast_uint_to_ushort(pathID);
float4 pos = RENDER_TARGET_COORD_TO_CLIP_COORD(vertexPosition);

VARYING_PACK(v_windingWeight);
Expand Down
19 changes: 12 additions & 7 deletions renderer/src/shaders/draw_clockwise_path.glsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,18 @@ VERTEX_MAIN(@drawVertexMain, Attrs, attrs, _vertexID, _instanceID)
VARYING_INIT(v_coverageCoord, float2);

float4 pos;
uint pathID;
float2 vertexPosition;
if (unpack_tessellated_path_vertex(@a_patchVertexData,
@a_mirroredVertexData,
_instanceID,
v_pathID,
pathID,
vertexPosition,
v_edgeDistance VERTEX_CONTEXT_UNPACK))
{
uint4 coverageData =
STORAGE_BUFFER_LOAD4(@pathBuffer, v_pathID * 4u + 2u);
STORAGE_BUFFER_LOAD4(@pathBuffer, pathID * 4u + 2u);
v_pathID = pathID;
v_coveragePlacement = coverageData.xy;
v_coverageCoord = vertexPosition + uintBitsToFloat(coverageData.zw);
pos = RENDER_TARGET_COORD_TO_CLIP_COORD(vertexPosition);
Expand Down Expand Up @@ -85,11 +87,13 @@ VERTEX_MAIN(@drawVertexMain, Attrs, attrs, _vertexID, _instanceID)
VARYING_INIT(v_coveragePlacement, uint2);
VARYING_INIT(v_coverageCoord, float2);

uint pathID;
float2 vertexPosition =
unpack_interior_triangle_vertex(@a_triangleVertex,
v_pathID,
pathID,
v_windingWeight VERTEX_CONTEXT_UNPACK);
uint4 coverageData = STORAGE_BUFFER_LOAD4(@pathBuffer, v_pathID * 4u + 2u);
uint4 coverageData = STORAGE_BUFFER_LOAD4(@pathBuffer, pathID * 4u + 2u);
v_pathID = pathID;
v_coveragePlacement = coverageData.xy;
v_coverageCoord = vertexPosition + uintBitsToFloat(coverageData.zw);
float4 pos = RENDER_TARGET_COORD_TO_CLIP_COORD(vertexPosition);
Expand Down Expand Up @@ -282,7 +286,8 @@ FRAG_DATA_MAIN(half4, @drawFragmentMain)
VARYING_UNPACK(v_coverageCoord, float2);

half4 paintColor;
uint2 paintData = STORAGE_BUFFER_LOAD2(@paintBuffer, v_pathID);
uint pathID = v_pathID;
uint2 paintData = STORAGE_BUFFER_LOAD2(@paintBuffer, pathID);
uint paintType = paintData.x & 0xfu;
if (paintType <= SOLID_COLOR_PAINT_TYPE) // CLIP_UPDATE_PAINT_TYPE or
// SOLID_COLOR_PAINT_TYPE
Expand All @@ -294,9 +299,9 @@ FRAG_DATA_MAIN(half4, @drawFragmentMain)
// IMAGE_PAINT_TYPE
{
float2x2 M =
make_float2x2(STORAGE_BUFFER_LOAD4(@paintAuxBuffer, v_pathID * 4u));
make_float2x2(STORAGE_BUFFER_LOAD4(@paintAuxBuffer, pathID * 4u));
float4 translate =
STORAGE_BUFFER_LOAD4(@paintAuxBuffer, v_pathID * 4u + 1u);
STORAGE_BUFFER_LOAD4(@paintAuxBuffer, pathID * 4u + 1u);
float2 paintCoord = MUL(M, _fragCoord) + translate.xy;
if (paintType != IMAGE_PAINT_TYPE)
{
Expand Down
9 changes: 6 additions & 3 deletions renderer/src/shaders/draw_path.glsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ VERTEX_MAIN(@drawVertexMain, Attrs, attrs, _vertexID, _instanceID)
#endif

bool shouldDiscardVertex = false;
ushort pathID;
uint pathID;
float2 vertexPosition;
#ifdef @RENDER_MODE_MSAA
ushort pathZIndex;
Expand Down Expand Up @@ -187,8 +187,11 @@ VERTEX_MAIN(@drawVertexMain, Attrs, attrs, _vertexID, _instanceID)
// - 2 if the gradient ramp spans an entire row.
// - x0 of the gradient ramp in normalized space, if it's a simple
// 2-texel ramp.
if (paintTranslate.z >
.9) // paintTranslate.z is either ~1 or ~1/GRAD_TEXTURE_WIDTH.
float gradientSpan = paintTranslate.z;
// gradientSpan is either ~1 (complex gradients span the whole width
// of the texture minus 1px), or 1/GRAD_TEXTURE_WIDTH (simple
// gradients span 1px).
if (gradientSpan > .9)
{
// Complex ramps span an entire row. Set it to 2 to convey this.
v_paint.b = 2.;
Expand Down
50 changes: 24 additions & 26 deletions renderer/src/shaders/draw_path_common.glsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ INLINE float manhattan_pixel_width(float2x2 M, float2 normalized)
INLINE bool unpack_tessellated_path_vertex(float4 patchVertexData,
float4 mirroredVertexData,
int _instanceID,
OUT(ushort) o_pathID,
OUT(float2) o_vertexPosition
OUT(uint) outPathID,
OUT(float2) outVertexPosition
#ifndef @RENDER_MODE_MSAA
,
OUT(half2) o_edgeDistance
OUT(half2) outEdgeDistance
#else
,
OUT(ushort) o_pathZIndex
OUT(ushort) outPathZIndex
#endif
VERTEX_CONTEXT_DECL)
{
Expand All @@ -66,18 +66,18 @@ INLINE bool unpack_tessellated_path_vertex(float4 patchVertexData,
STORAGE_BUFFER_LOAD4(@contourBuffer,
contour_data_idx(contourIDWithFlags));
float2 midpoint = uintBitsToFloat(contourData.xy);
uint pathID = cast_uint_to_ushort(contourData.z & 0xffffu);
outPathID = contourData.z & 0xffffu;
uint vertexIndex0 = contourData.w;

// Fetch and unpack the path.
float2x2 M = make_float2x2(
uintBitsToFloat(STORAGE_BUFFER_LOAD4(@pathBuffer, pathID * 4u)));
uint4 pathData = STORAGE_BUFFER_LOAD4(@pathBuffer, pathID * 4u + 1u);
uintBitsToFloat(STORAGE_BUFFER_LOAD4(@pathBuffer, outPathID * 4u)));
uint4 pathData = STORAGE_BUFFER_LOAD4(@pathBuffer, outPathID * 4u + 1u);
float2 translate = uintBitsToFloat(pathData.xy);

float strokeRadius = uintBitsToFloat(pathData.z);
#ifdef @RENDER_MODE_MSAA
o_pathZIndex = cast_uint_to_ushort(pathData.w);
outPathZIndex = cast_uint_to_ushort(pathData.w);
#endif

// Fix the tessellation vertex if we fetched the wrong one in order to
Expand Down Expand Up @@ -164,7 +164,7 @@ INLINE bool unpack_tessellated_path_vertex(float4 patchVertexData,
// Calculate the AA distance to both the outset and inset edges of the
// stroke. The fragment shader will use whichever is lesser.
float x = outset * (strokeRadius + aaRadius);
o_edgeDistance = cast_float2_to_half2(
outEdgeDistance = cast_float2_to_half2(
(1. / (aaRadius * 2.)) * (float2(x, -x) + strokeRadius) + .5);
#endif

Expand Down Expand Up @@ -268,19 +268,19 @@ INLINE bool unpack_tessellated_path_vertex(float4 patchVertexData,
(bisectPixelWidth * (AA_RADIUS * 2.));
#ifndef @RENDER_MODE_MSAA
if ((contourIDWithFlags & LEFT_JOIN_CONTOUR_FLAG) != 0u)
o_edgeDistance.y = cast_float_to_half(clipDistance);
outEdgeDistance.y = cast_float_to_half(clipDistance);
else
o_edgeDistance.x = cast_float_to_half(clipDistance);
outEdgeDistance.x = cast_float_to_half(clipDistance);
#endif
}

#ifndef @RENDER_MODE_MSAA
o_edgeDistance *= globalCoverage;
outEdgeDistance *= globalCoverage;

// Bias o_edgeDistance.y slightly upwards in order to guarantee
// o_edgeDistance.y is >= 0 at every pixel. "o_edgeDistance.y < 0" is
// Bias outEdgeDistance.y slightly upwards in order to guarantee
// outEdgeDistance.y is >= 0 at every pixel. "outEdgeDistance.y < 0" is
// used to differentiate between strokes and fills.
o_edgeDistance.y = max(o_edgeDistance.y, make_half(1e-4));
outEdgeDistance.y = max(outEdgeDistance.y, make_half(1e-4));
#endif

postTransformVertexOffset = MUL(M, outset * vertexOffset);
Expand All @@ -306,9 +306,9 @@ INLINE bool unpack_tessellated_path_vertex(float4 patchVertexData,
}

#ifndef @RENDER_MODE_MSAA
// "o_edgeDistance.y < 0" indicates to the fragment shader that this is
// "outEdgeDistance.y < 0" indicates to the fragment shader that this is
// a fill.
o_edgeDistance = make_half2(fillCoverage, -1.);
outEdgeDistance = make_half2(fillCoverage, -1.);
#endif

// If we're actually just drawing a triangle, throw away the entire
Expand All @@ -318,25 +318,23 @@ INLINE bool unpack_tessellated_path_vertex(float4 patchVertexData,
return false;
}

o_pathID = cast_uint_to_ushort(pathID);
o_vertexPosition = MUL(M, origin) + postTransformVertexOffset + translate;
outVertexPosition = MUL(M, origin) + postTransformVertexOffset + translate;
return true;
}
#endif // @DRAW_PATH

#ifdef @DRAW_INTERIOR_TRIANGLES
INLINE float2
unpack_interior_triangle_vertex(float3 triangleVertex,
OUT(ushort) o_pathID,
OUT(half) o_windingWeight VERTEX_CONTEXT_DECL)
OUT(uint) outPathID,
OUT(half) outWindingWeight VERTEX_CONTEXT_DECL)
{
uint pathID = floatBitsToUint(triangleVertex.z) & 0xffffu;
outPathID = floatBitsToUint(triangleVertex.z) & 0xffffu;
float2x2 M = make_float2x2(
uintBitsToFloat(STORAGE_BUFFER_LOAD4(@pathBuffer, pathID * 4u)));
uint4 pathData = STORAGE_BUFFER_LOAD4(@pathBuffer, pathID * 4u + 1u);
uintBitsToFloat(STORAGE_BUFFER_LOAD4(@pathBuffer, outPathID * 4u)));
uint4 pathData = STORAGE_BUFFER_LOAD4(@pathBuffer, outPathID * 4u + 1u);
float2 translate = uintBitsToFloat(pathData.xy);
o_pathID = cast_uint_to_ushort(pathID);
o_windingWeight = cast_int_to_half(floatBitsToInt(triangleVertex.z) >> 16);
outWindingWeight = cast_int_to_half(floatBitsToInt(triangleVertex.z) >> 16);
return MUL(M, triangleVertex.xy) + translate;
}
#endif // @DRAW_INTERIOR_TRIANGLES
Expand Down

0 comments on commit d771676

Please sign in to comment.