Skip to content

[RAT-2480] Removing unnecessary dictionary allocation#662

Open
lopezt-unity wants to merge 2 commits intomasterfrom
bugfix/rat-2480-removing-unnecessary-dictionary-allocation
Open

[RAT-2480] Removing unnecessary dictionary allocation#662
lopezt-unity wants to merge 2 commits intomasterfrom
bugfix/rat-2480-removing-unnecessary-dictionary-allocation

Conversation

@lopezt-unity
Copy link
Collaborator

Purpose of this PR

The ColorUtility.ColorNameLookup dictionary was taking too much space in builds, as reported by some users and does not have any other use than displaying a nice name in an optional notification in the SceneView when applying colors to a face, vertex or edge.

This PR remove that dictionary as the cost here seems unrelevant.

Links

Jira: https://jira.unity3d.com/browse/RAT-2480

Comments to Reviewers

[List known issues, planned work, provide any extra context for your code.]

Copy link

@u-pr u-pr bot left a comment

Choose a reason for hiding this comment

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

Great

The PR looks good overall. The changes significantly reduce binary size and memory overhead by removing large utility dictionaries. Two minor observations regarding public API breaking changes and documentation clarity were noted.

🤖 Helpful? 👍/👎

{ "Zaffre", CIELabFromRGB(0f, 8f, 66f, 100f) },
{ "Zinnwaldite Brown", CIELabFromRGB(17f, 9f, 3f, 100f) },
{ "Zomp", CIELabFromRGB(22f, 65f, 56f, 100f) }
};
}
Copy link

Choose a reason for hiding this comment

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

medium
While removing the ColorNameLookup dictionary is a significant performance win for binary size and runtime memory, note that removing the public static string GetColorName method is a breaking change for the public API. If this color-naming functionality remains useful for Editor-side tooling, have you considered moving the logic and dictionary to an Editor assembly instead of deleting it? This would preserve the feature for developers while keeping the Runtime build lean. Additionally, it may be worth verifying if supporting types like CIELabColor or XYZColor are now orphaned and should also be moved or removed.

🤖 Helpful? 👍/👎

## Unreleased

### Changes
- Removed a non-necessary utility dictionary that would allocate unnecessary large space in a build.
Copy link

Choose a reason for hiding this comment

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

low
Minor nit: The phrase 'allocate unnecessary large space' is slightly unclear. Have you considered rephrasing to 'Removed a large utility dictionary to reduce binary size and runtime memory overhead' or similar?

🤖 Helpful? 👍/👎

@codecov-github-com
Copy link

codecov-github-com bot commented Mar 18, 2026

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
Editor/EditorCore/VertexColorPalette.cs 0.00% 1 Missing ⚠️
@@           Coverage Diff           @@
##           master     #662   +/-   ##
=======================================
  Coverage   36.05%   36.06%           
=======================================
  Files         277      277           
  Lines       34909    34911    +2     
=======================================
+ Hits        12588    12589    +1     
- Misses      22321    22322    +1     
Flag Coverage Δ
probuilder_MacOS_6000.0 34.60% <0.00%> (+0.01%) ⬆️
probuilder_MacOS_6000.3 34.60% <0.00%> (+0.01%) ⬆️
probuilder_MacOS_6000.4 34.59% <0.00%> (+0.01%) ⬆️
probuilder_MacOS_6000.6 34.59% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
Runtime/Core/ColorUtility.cs 0.00% <ø> (ø)
Editor/EditorCore/VertexColorPalette.cs 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

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