Add background border-radius to text layer #9447
Conversation
| float isBorder = smoothedge(distToEdge, vLineWidth); | ||
| fragColor = mix(vFillColor, vLineColor, isBorder); | ||
| float distToEdge = -dist; | ||
| float isBorder = smoothstep(vLineWidth - 1.0, vLineWidth + 1.0, distToEdge); |
There was a problem hiding this comment.
Why remove smoothedge?
There was a problem hiding this comment.
Yes, I replicate smoothedge. I will use it instead.
There was a problem hiding this comment.
With shader code, it's best to avoid making unnecessary changes as each might introduce some bug
| float maxBorderRadius = min(dimensions.x, dimensions.y) * 0.5; | ||
| vec4 borderRadius = vec4(min(textBackground.borderRadius, maxBorderRadius)); | ||
| float dist = round_rect(pixelPosition, dimensions * 0.5, borderRadius); |
There was a problem hiding this comment.
Better to pass uv, dimensions & borderRadius as parameters to function to better encapsulate
There was a problem hiding this comment.
Did you mean adding a function like that to compute the distance ?
float get_dist(vec2 uv, vec2 dimensions, vec4 borderRadius) {
// Convert UV to center-based coordinates [-0.5, 0.5]
vec2 pixelPosition = (uv - 0.5) * dimensions;
float maxBorderRadius = min(dimensions.x, dimensions.y) * 0.5;
return round_rect(pixelPosition, dimensions * 0.5, vec4(min(borderRadius, maxBorderRadius)));
}
There was a problem hiding this comment.
No, rather than implement round_rect(pixelPosition, size, radii) have round_rect(uv, dimensions, radii) and then calculate pixelPosition, maxBorderRadius etc in the function
| void main(void) { | ||
| geometry.uv = uv; | ||
| vec2 pixelPosition = uv * dimensions; | ||
| // Convert UV to center-based coordinates [-0.5, 0.5] |
There was a problem hiding this comment.
Add an if block to only run if border radius is specified
There was a problem hiding this comment.
I'm not sure to understand why. By adding this borderRadius: {type: 'object', value: 0}, to TextBackgroundLayer default props, I thought that the default value of the props will be 0 and thus will never be undefined
There was a problem hiding this comment.
Yes, but all that code will run (on every pixel) even for value=0, by putting it in a branch the GPU can skip it
| @@ -87,6 +87,12 @@ type _TextLayerProps<DataT> = { | |||
| * @default 0 | |||
| */ | |||
| getBorderWidth?: Accessor<DataT, number>; | |||
| /** The border radius of the background. | |||
| * If a number is supplied, it is the same border radius in pixel for all corner. | |||
There was a problem hiding this comment.
| * If a number is supplied, it is the same border radius in pixel for all corner. | |
| * If a number is supplied, it is the same border radius in pixel for all corners. |
There was a problem hiding this comment.
Ok, I will fix that and will do it for the doc too.
| @@ -16,21 +16,37 @@ in vec2 dimensions; | |||
| out vec4 fragColor; | |||
| float round_rect(vec2 p, vec2 size, vec4 radii) { | |||
There was a problem hiding this comment.
Match formatting with rest of shader code
test/render/test-cases/text-layer.js
Outdated
| fontFamily: 'Arial', | ||
| background: true, | ||
| backgroundPadding: [10, 10], | ||
| backgroundBorderRadius: [10, 10, 10, 10], |
There was a problem hiding this comment.
Might as well make some labels have different radii to prove it works
|
Hello, |
Closes #9004
Change List
backgroundBorderRadiusprop toTextLayer.backgroundBorderRadiusprop.TextLayerapi-reference documentation.