Skip to content

Replace the AlphaBlending struct with separate attributes#4086

Merged
Keavon merged 3 commits intomasterfrom
split-apart-alphablending
May 1, 2026
Merged

Replace the AlphaBlending struct with separate attributes#4086
Keavon merged 3 commits intomasterfrom
split-apart-alphablending

Conversation

@Keavon
Copy link
Copy Markdown
Member

@Keavon Keavon commented May 1, 2026

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the alpha blending system by replacing the monolithic AlphaBlending struct with individual attribute columns for blend_mode, opacity, opacity_fill, and clipping_mask. This change improves data granularity and simplifies attribute handling across the codebase. The review identified an inconsistency in the GPU raster rendering logic where opacity_fill_attr was ignored and the layer decision logic differed from the CPU implementation; a suggestion was provided to unify this behavior.

Comment thread node-graph/libraries/rendering/src/renderer.rs Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 15 files

Confidence score: 2/5

  • There are concrete, user-visible rendering risks: in node-graph/libraries/rendering/src/renderer.rs, artboard clipping is read from the wrong attribute key (severity 7/10, high confidence), so clip settings can be ignored at render time.
  • node-graph/libraries/rendering/src/renderer.rs also has a GPU raster mismatch (severity 6/10, high confidence) where opacity handling skips ATTR_OPACITY_FILL and mask-mode composition, which can make output differ from other render paths.
  • The issue in node-graph/libraries/core-types/src/table.rs is low risk and documentation-only (comment references ATTR_FILL instead of ATTR_OPACITY_FILL), but it should still be corrected for clarity.
  • Pay close attention to node-graph/libraries/rendering/src/renderer.rs - clipping and opacity behavior are both flagged as likely incorrect at runtime.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/libraries/rendering/src/renderer.rs">

<violation number="1" location="node-graph/libraries/rendering/src/renderer.rs:514">
P1: Artboard clipping is read from the wrong attribute key, so artboard clip settings can be ignored at render time.</violation>

<violation number="2" location="node-graph/libraries/rendering/src/renderer.rs:1655">
P2: GPU raster layer opacity ignores `ATTR_OPACITY_FILL` (and mask-mode composition), causing incorrect opacity compared with other render paths.</violation>
</file>

<file name="node-graph/libraries/core-types/src/table.rs">

<violation number="1" location="node-graph/libraries/core-types/src/table.rs:23">
P3: The doc comment references `ATTR_FILL`, but the actual attribute constant is `ATTR_OPACITY_FILL`. Update the comment to the correct name to avoid confusion.

(Based on your team's feedback about consistent, precise terminology in names and docs.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread node-graph/libraries/rendering/src/renderer.rs Outdated
Comment thread node-graph/libraries/rendering/src/renderer.rs Outdated
Comment thread node-graph/libraries/core-types/src/table.rs Outdated
@Keavon Keavon force-pushed the split-apart-alphablending branch from 295f2a5 to d0e254c Compare May 1, 2026 09:55
@Keavon Keavon merged commit 4474de4 into master May 1, 2026
11 checks passed
@Keavon Keavon deleted the split-apart-alphablending branch May 1, 2026 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant