Skip to content

WebGL: apply compound opacity inheritance to child node bodies and borders#3448

Merged
maxkfranz merged 7 commits intounstablefrom
copilot/fix-child-nodes-opacity-inheritance
Apr 27, 2026
Merged

WebGL: apply compound opacity inheritance to child node bodies and borders#3448
maxkfranz merged 7 commits intounstablefrom
copilot/fix-child-nodes-opacity-inheritance

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 14, 2026 •
edited
Loading

WebGL node rendering was ignoring compound-parent opacity for child nodes, so compound graphs looked different from canvas mode. This updates the WebGL simple-shape path to respect inherited element opacity for child node fills and borders.

  • Renderer

    • Use node.effectiveOpacity() in the WebGL simple-shape node path.
    • Multiply inherited element opacity into:
      • node body alpha (background-opacity)
      • node border alpha (border-opacity)
    • Preserve picking behavior by keeping picking renders fully opaque.

  • Regression coverage

    • Add a focused WebGL unit test for drawNode() that verifies inherited opacity is applied to both body and border output colors.

  • Behavioral alignment

    • Match existing canvas semantics for compound opacity inheritance without changing unrelated WebGL rendering paths.
const effectiveOpacity = this.renderTarget.picking ? 1 : node.effectiveOpacity();
const opacity = effectiveOpacity * node.pstyle(props.opacity).value;
const borderOpacity = effectiveOpacity * node.pstyle('border-opacity').value;
  • Screenshot
    • Compound nodes in WebGL with parent opacity applied:
      WebGL compound opacity inheritance

Copilot AI linked an issue Apr 14, 2026 that may be closed by this pull request
WebGL: child nodes don't inherit their opacity from the parent node #3409
Closed
6 tasks
Copilot AI changed the title [WIP] Fix child nodes opacity inheritance in WebGL mode WebGL: apply compound opacity inheritance to child node bodies and borders Apr 15, 2026
Copilot AI requested a review from maxkfranz April 15, 2026 00:07
@maxkfranz maxkfranz marked this pull request as ready for review April 15, 2026 18:12
@maxkfranz
Copy link
Copy Markdown
Member

maxkfranz commented Apr 20, 2026 •
edited
Loading

@mikekucera, @chrtannus, @d2fong

The AI was kind of on the right track, but it couldn't get the details right. It did at least isolate the problematic function. As expected, the AI isn't great at rendering-related stuff. Its automated test could be bunk; I haven't really looked at it yet.

I made a few manual commits to clean things up with a proper fix. You can verify in this branch on the debug/webgl page.

Webgl with :parent { opacity: 0.5 }:

Screenshot 2026-04-20 at 14 39 35

Canvas with :parent { opacity: 0.5 }:

Screenshot 2026-04-20 at 14 39 39
  • Improved livereload support on the webgl debug page. Much better for testing.
  • I had to handle some edge cases.
    • It looks like the function was a bit off w.r.t. picking when the compound parent is background-opacity:0.
    • Every type other than "node-body" gets exempt from opacity inheritance/stacking. E.g. an opacity: 0 node can still get a visible overlay.

maxkfranz
maxkfranz reviewed Apr 20, 2026
View reviewed changes
Comment thread test/modules/webgl-drawing-elements.mjs
Copy link
Copy Markdown
Member

@maxkfranz maxkfranz Apr 20, 2026

Choose a reason for hiding this comment

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

This test needs to be evaluated to see if it's worth keeping.

Copy link
Copy Markdown
Contributor

@mikekucera mikekucera Apr 24, 2026

Choose a reason for hiding this comment

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

It validates that the webgl buffers get loaded with the correct data. It's actually a half decent way to test the renderer without it actually rendering. It basically tests that the node's visual properties correctly get interpreted and loaded for webgl.

The line: expect(colorBuffer.view[3]).to.be.closeTo(0.2, 0.000001); is doing the actual test for the inerited opacity. The [3]rd element is the alpha channel. The node's background-opacity is 0.5, and the effectiveOpacity() mock returns 0.4.. 0.4 * 0.5 = 0.2 so yes its being multiplied properly. I'd say its worth keeping. The mocks are ad-hock, might be worth using a real mocking library in the future, but its fine.

@maxkfranz
Copy link
Copy Markdown
Member

maxkfranz commented Apr 20, 2026

@d2fong, if this affects Cyweb, then you may want to take a look at this. Otherwise, it might not be interesting.

chrtannus
chrtannus approved these changes Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@chrtannus chrtannus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

When I set the compound node's 'opacity' to a transparent value (< 1.0), the child nodes are now rendered with the same opacity as the parent, as expected. Setting a 0.5 'opacity' to both the parent and child nodes look correct to me as well--the child node still gets its own opacity, and it's affected by the parent's transparency.

@maxkfranz
Copy link
Copy Markdown
Member

maxkfranz commented Apr 24, 2026

@chrtannus, great. Thanks. I'll merge this in next week for a release then -- unless we notice any issues in the meanwhile.

mikekucera
mikekucera approved these changes Apr 24, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@mikekucera mikekucera left a comment

Choose a reason for hiding this comment

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

Looks good.

Comment thread test/modules/webgl-drawing-elements.mjs
Copy link
Copy Markdown
Contributor

@mikekucera mikekucera Apr 24, 2026

Choose a reason for hiding this comment

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

It validates that the webgl buffers get loaded with the correct data. It's actually a half decent way to test the renderer without it actually rendering. It basically tests that the node's visual properties correctly get interpreted and loaded for webgl.

The line: expect(colorBuffer.view[3]).to.be.closeTo(0.2, 0.000001); is doing the actual test for the inerited opacity. The [3]rd element is the alpha channel. The node's background-opacity is 0.5, and the effectiveOpacity() mock returns 0.4.. 0.4 * 0.5 = 0.2 so yes its being multiplied properly. I'd say its worth keeping. The mocks are ad-hock, might be worth using a real mocking library in the future, but its fine.

@maxkfranz
Copy link
Copy Markdown
Member

maxkfranz commented Apr 27, 2026

Thanks, merging

@maxkfranz maxkfranz merged commit 0dc8f82 into unstable Apr 27, 2026
2 checks passed
maxkfranz added a commit that referenced this pull request Apr 27, 2026
@maxkfranz
Cherry-pick to master: Merge pull request #3448 from cytoscape/copilo…
b6df1d3 
…t/fix-child-nodes-opacity-inheritance

WebGL: apply compound opacity inheritance to child node bodies and borders
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxkfranz maxkfranz maxkfranz left review comments

@chrtannus chrtannus chrtannus approved these changes

@d2fong d2fong Awaiting requested review from d2fong

@mikekucera mikekucera Awaiting requested review from mikekucera

Assignees

@maxkfranz maxkfranz

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

WebGL: child nodes don't inherit their opacity from the parent node

4 participants

@maxkfranz @chrtannus @mikekucera