gl: add direct shape blending path and shaders #4053
Conversation
906786c to
fcf2c3b
Compare
There was a problem hiding this comment.
Pull request overview
This PR replaces the render-to-texture approach for shape blending with direct blending, eliminating the separate compose pass and reducing visual artifacts (halo effects). The refactor extracts gradient computation logic into reusable functions and introduces shape-specific blend shaders that perform blending in a single draw call.
Key changes:
- Refactored gradient fragment shaders to separate computation functions from main entry points
- Added three new blend shader headers for shape blending (solid, linear gradient, radial gradient)
- Replaced
GlSimpleBlendTaskwithGlDirectBlendTaskthat copies only the required destination region and blends directly
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tvgGlShaderSrc.h |
Added declarations for gradient function strings and new shape blend shader headers |
tvgGlShaderSrc.cpp |
Refactored gradient shaders into reusable functions; added shape-specific blend shader headers with direct blending logic |
tvgGlRenderer.h |
Replaced separate blend enums with unified shape blend enums; added BlendSource enum for blend program selection |
tvgGlRenderer.cpp |
Updated shader initialization, modified drawPrimitive to use direct blending with region copying, simplified blend program selection with BlendSource |
tvgGlRenderTask.h |
Renamed GlSimpleBlendTask to GlDirectBlendTask and changed constructor signature |
tvgGlRenderTask.cpp |
Reimplemented blend task to copy destination region and blend directly without blend equations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@wenjieshen Please note that, when you really want to get a code review but the changes are not easy to read, separating commits is mandatory rather than mixing everything into a single commit. |
fcf2c3b to
828aa3b
Compare
I apologize for the inconvenience. I have tried separating commits. The modified version has redundant code, and I will create a PR to remove it. Thank you. |
828aa3b to
302b109
Compare
302b109 to
53c2eb8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/renderer/gl_engine/tvgGlShaderSrc.cpp:1
- The hardcoded buffer lengths lack explanation for how they were calculated. Consider adding a comment explaining how these values correspond to the shader component lengths, or consider computing them dynamically to avoid maintenance issues when shader strings change.
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
53c2eb8 to
4efcc17
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/renderer/gl_engine/tvgGlShaderSrc.cpp:1
- The hardcoded shader length constants (2831, 5315, 5290) are magic numbers that will require manual updates whenever shader code changes. Consider documenting how these values were calculated or add a comment explaining that these must be updated when modifying shader strings to prevent buffer overflows.
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace GlSimpleBlendTask with GlDirectBlendTask to perform blending directly in the shape shader without a separate compose pass. This copies only the required destination region and blends in a single draw.
Render to texture
There are two textures. The source texture has been anti-aliased, which causes a halo effect due to its softened edges.
Direct blending
There is only one textue. The MSAA is resolved after blending.
Result
Performance doesn't change, difference less than 0.1%
No regressions found druing execution of all examples.
issue: #4033, #3175