From 58c7df469dba9fbd102706984a2f38c477dfac51 Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Mon, 15 May 2023 14:45:38 -0400 Subject: [PATCH] Address review feedback * replace one_minus_focal_x and abs_one_minus_focal_x variables with the actual expressions * replace division by r^2-1 with multiplication by reciprocal * revert chain selects to branchy code for clarity. Branching is dynamically uniform so shouldn't affect performance * add suggested comment describing gradient kind/flags constants --- shader/draw_leaf.wgsl | 11 ++-- shader/fine.wgsl | 121 +++++++++++--------------------------- shader/shared/config.wgsl | 4 ++ 3 files changed, 42 insertions(+), 94 deletions(-) diff --git a/shader/draw_leaf.wgsl b/shader/draw_leaf.wgsl index ab27619..9845fdd 100644 --- a/shader/draw_leaf.wgsl +++ b/shader/draw_leaf.wgsl @@ -184,9 +184,7 @@ fn main( r1 = tmp_r; } focal_x = r0 / (r0 - r1); - let one_minus_focal_x = 1.0 - focal_x; - let cf = one_minus_focal_x * p0 + focal_x * p1; - let abs_one_minus_focal_x = abs(one_minus_focal_x); + let cf = (1.0 - focal_x) * p0 + focal_x * p1; radius = r1 / (distance(cf, p1)); let user_to_unit_line = transform_mul( two_point_to_unit_line(cf, p1), @@ -196,15 +194,16 @@ fn main( // When r == 1.0, focal point is on circle if abs(radius - 1.0) <= GRADIENT_EPSILON { kind = RAD_GRAD_KIND_FOCAL_ON_CIRCLE; - let scale = 0.5 * abs_one_minus_focal_x; + let scale = 0.5 * abs(1.0 - focal_x); user_to_scaled = transform_mul( Transform(vec4(scale, 0.0, 0.0, scale), vec2(0.0)), user_to_unit_line ); } else { let a = radius * radius - 1.0; - let scale_x = radius / a * abs_one_minus_focal_x; - let scale_y = sqrt(abs(a)) / a * abs_one_minus_focal_x; + let a_recip = 1.0 / a; + let scale_x = radius * a_recip * abs(1.0 - focal_x); + let scale_y = sqrt(abs(a)) * a_recip * abs(1.0 - focal_x); user_to_scaled = transform_mul( Transform(vec4(scale_x, 0.0, 0.0, scale_y), vec2(0.0)), user_to_unit_line diff --git a/shader/fine.wgsl b/shader/fine.wgsl index bab1b63..e6ddb2c 100644 --- a/shader/fine.wgsl +++ b/shader/fine.wgsl @@ -118,30 +118,20 @@ fn extend_mode(t: f32, mode: u32) -> f32 { let EXTEND_PAD = 0u; let EXTEND_REPEAT = 1u; let EXTEND_REFLECT = 2u; - // Branching version of the code below: - // - // switch mode { - // // EXTEND_PAD - // case 0u: { - // return clamp(t, 0.0, 1.0); - // } - // // EXTEND_REPEAT - // case 1u: { - // return fract(t); - // } - // // EXTEND_REFLECT - // default: { - // return abs(t - 2.0 * round(0.5 * t)); - // } - // } - let pad = clamp(t, 0.0, 1.0); - let repeat = fract(t); - let reflect = abs(t - 2.0 * round(0.5 * t)); - return select( - select(pad, repeat, mode == EXTEND_REPEAT), - reflect, - mode == EXTEND_REFLECT - ); + switch mode { + // EXTEND_PAD + case 0u: { + return clamp(t, 0.0, 1.0); + } + // EXTEND_REPEAT + case 1u: { + return fract(t); + } + // EXTEND_REFLECT + default: { + return abs(t - 2.0 * round(0.5 * t)); + } + } } #else @@ -309,16 +299,14 @@ fn main( case 7u: { let rad = read_rad_grad(cmd_ix); let focal_x = rad.focal_x; - let one_minus_focal_x = 1.0 - focal_x; let radius = rad.radius; let is_strip = rad.kind == RAD_GRAD_KIND_STRIP; let is_circular = rad.kind == RAD_GRAD_KIND_CIRCULAR; let is_focal_on_circle = rad.kind == RAD_GRAD_KIND_FOCAL_ON_CIRCLE; let is_swapped = (rad.flags & RAD_GRAD_SWAPPED) != 0u; - let is_greater = radius > 1.0; - let inv_r1 = select(1.0 / radius, 0.0, is_circular); - let less_scale = select(1.0, -1.0, is_swapped || one_minus_focal_x < 0.0); - let t_sign = sign(one_minus_focal_x); + let r1_recip = select(1.0 / radius, 0.0, is_circular); + let less_scale = select(1.0, -1.0, is_swapped || (1.0 - focal_x) < 0.0); + let t_sign = sign(1.0 - focal_x); for (var i = 0u; i < PIXELS_PER_THREAD; i += 1u) { let my_xy = vec2(xy.x + f32(i), xy.y); let local_xy = rad.matrx.xy * my_xy.x + rad.matrx.zw * my_xy.y + rad.xlat; @@ -326,65 +314,22 @@ fn main( let y = local_xy.y; let xx = x * x; let yy = y * y; - let x_inv_r1 = x * inv_r1; - // This is the branching version of the code implemented - // by the chained selects below: - // - // var t = 0.0; - // var is_valid = true; - // if is_strip { - // let a = radius - yy; - // t = sqrt(a) + x; - // is_valid = a >= 0.0; - // } else if is_focal_on_circle { - // t = (xx + yy) / x; - // is_valid = t >= 0.0; - // } else if radius > 1.0 { - // t = sqrt(xx + yy) - x_inv_r1; - // } else { - // let a = xx - yy; - // t = root_f * sqrt(a) - x_inv_r1; - // is_valid = a >= 0.0 && t >= 0.0; - // } - // - // The pattern is that these can all be computed with - // the expression: a * sqrt(b) + c - // - // The parameters to the expression are computed up front - // and chosen with chained selects based on their - // respective conditions. The same process is done - // for determining the validity of the resulting value. - var strip_params = vec3(1.0, radius - yy, x); - var foc_params = vec3(1.0, 0.0, (xx + yy) / x); - var greater_params = vec3(1.0, xx + yy, -x_inv_r1); - var less_params = vec3(less_scale, xx - yy, -x_inv_r1); - var params = select( - select( - select( - less_params, - greater_params, - is_greater, - ), - foc_params, - is_focal_on_circle, - ), - strip_params, - is_strip, - ); - var t = params.x * sqrt(params.y) + params.z; - let is_valid = select( - select( - select( - params.y >= 0.0 && t >= 0.0, - true, - is_greater - ), - t >= 0.0 && x != 0.0, - is_focal_on_circle, - ), - params.y >= 0.0, - is_strip, - ); + var t = 0.0; + var is_valid = true; + if is_strip { + let a = radius - yy; + t = sqrt(a) + x; + is_valid = a >= 0.0; + } else if is_focal_on_circle { + t = (xx + yy) / x; + is_valid = t >= 0.0 && x != 0.0; + } else if radius > 1.0 { + t = sqrt(xx + yy) - x * r1_recip; + } else { // radius < 1.0 + let a = xx - yy; + t = less_scale * sqrt(a) - x * r1_recip; + is_valid = a >= 0.0 && t >= 0.0; + } if is_valid { t = extend_mode(focal_x + t_sign * t, rad.extend_mode); t = select(t, 1.0 - t, is_swapped); diff --git a/shader/shared/config.wgsl b/shader/shared/config.wgsl index 9b1cd2e..4ac718c 100644 --- a/shader/shared/config.wgsl +++ b/shader/shared/config.wgsl @@ -50,6 +50,10 @@ let N_TILE = 256u; let BLEND_STACK_SPLIT = 4u; +// The following are computed in draw_leaf from the generic gradient parameters +// encoded in the scene, and stored in the gradient's info struct, for +// consumption during fine rasterization. + // Radial gradient kinds let RAD_GRAD_KIND_CIRCULAR = 1u; let RAD_GRAD_KIND_STRIP = 2u;