Skip to content

Clean up duplicated code used for recursively flattening graphic types#3836

Merged
Keavon merged 5 commits intomasterfrom
graphic-flattening-cleanup
Feb 26, 2026
Merged

Clean up duplicated code used for recursively flattening graphic types#3836
Keavon merged 5 commits intomasterfrom
graphic-flattening-cleanup

Conversation

@Keavon
Copy link
Member

@Keavon Keavon commented Feb 26, 2026

No description provided.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @Keavon, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the core logic for flattening graphic tables by introducing a generic approach. Previously, separate functions existed for flattening different graphic types (vector, raster, color, gradient), leading to significant code duplication. The changes consolidate this logic into a single, reusable function and a new trait, making the codebase more maintainable and extensible for future graphic types.

Highlights

  • Refactored Graphic Flattening Logic: Replaced four duplicated into_flattened_*_table methods with a single generic into_flattened_table method within the IntoGraphicTable trait, significantly reducing code duplication.
  • Introduced TryFromGraphic Trait: Added a new TryFromGraphic trait and implemented it for Vector, Raster<CPU>, Color, and GradientStops to provide a standardized way to extract specific graphic variants from a Graphic enum.
  • Generic Flattening Function: Implemented a new private generic function flatten_graphic_table that handles the recursive flattening of graphic tables, composing transforms and opacity, and filtering elements based on the TryFromGraphic trait.
  • Updated Node Implementations: Modified flatten_vector, flatten_raster, flatten_color, flatten_gradient, flatten_path, and morph nodes to utilize the new generic into_flattened_table method, simplifying their implementations.
Changelog
  • node-graph/libraries/graphic-types/src/graphic.rs
    • Removed specific into_flattened_vector_table, into_flattened_raster_table, into_flattened_color_table, and into_flattened_gradient_table methods from IntoGraphicTable trait.
    • Added a new generic into_flattened_table<T: TryFromGraphic> method to the IntoGraphicTable trait.
    • Introduced a new private generic function flatten_graphic_table for recursive graphic table flattening.
    • Defined a new public trait TryFromGraphic with try_from_graphic method.
    • Implemented TryFromGraphic for Vector, Raster<CPU>, Color, and GradientStops.
  • node-graph/libraries/graphic-types/src/lib.rs
    • Exported the new TryFromGraphic trait.
  • node-graph/nodes/graphic/src/graphic.rs
    • Updated flatten_vector, flatten_raster, flatten_color, and flatten_gradient functions to use the new generic into_flattened_table method.
  • node-graph/nodes/vector/src/vector_nodes.rs
    • Refactored flatten_path to use into_flattened_table for processing vector content.
    • Updated morph function to use into_flattened_table for flattening vector content.
Activity
  • No explicit activity or comments were provided in the pull request description.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@Keavon Keavon requested a review from TrueDoctor February 26, 2026 11:51
@Keavon Keavon force-pushed the graphic-flattening-cleanup branch from 80584c8 to f1d87e0 Compare February 26, 2026 11:52
Copy link
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 is a great refactoring that significantly cleans up the code for recursively flattening graphic types. By introducing a generic flatten_graphic_table function and a TryFromGraphic trait, you've eliminated a large amount of duplicated code. This makes the implementation much cleaner, more maintainable, and easier to extend with new graphic types in the future. The changes in flatten_path are particularly nice, simplifying the logic considerably. I have one suggestion to further improve performance by avoiding an unnecessary clone.

Copy link

@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.

1 issue found across 4 files

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/graphic-types/src/graphic.rs">

<violation number="1" location="node-graph/libraries/graphic-types/src/graphic.rs:108">
P2: Doc comment says "Recursion through `Graphic::Graphic` sub-tables composes transforms and opacity" but the `Graphic::Graphic` recursive branch only composes transforms — opacity from intermediate `Graphic::Graphic` layers is silently dropped. The opacity composition only happens at the leaf level, meaning deeply nested `Graphic::Graphic` wrappers lose intermediate opacity. Consider either fixing the recursion to also compose opacity (by mutating `graphic.alpha_blending.opacity` alongside `graphic.transform` in the loop), or updating the doc comment to accurately reflect current behavior.

(Based on your team's feedback about aligning behavior with comments and TODO expectations.) [FEEDBACK_USED]</violation>
</file>

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

@Keavon Keavon merged commit 81c73d1 into master Feb 26, 2026
7 checks passed
@Keavon Keavon deleted the graphic-flattening-cleanup branch February 26, 2026 13:36
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