Fixing tools icons not loading properly when importing the package#668
Conversation
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
The pull request overall looks solid, with only one potential issue identified.
Summary of Findings
- Performance / Caching in IconUtility: Not caching
nullvalues for missing or misspelled icons may lead to repetitive disk lookups viaEditorGUIUtility.LoadIconduring GUI repaint loops, potentially degrading performance.
🤖 Helpful? 👍/👎
|
|
||
| icon = EditorGUIUtility.LoadIcon(fullPath); | ||
| s_Icons.Add(iconName, icon); | ||
| if(icon != null) |
There was a problem hiding this comment.
While not caching null successfully prevents temporary startup/import load failures from being cached permanently, it introduces a potential performance regression. If an icon is permanently missing or misspelled, GetIcon will perform string operations and EditorGUIUtility.LoadIcon (which triggers disk and asset database searches) on every call. If this occurs within GUI repaint loops (like OnGUI), it can degrade Editor performance.
Have you considered keeping track of failed lookups in a separate static set and clearing it on a specific lifecycle event (e.g., when package compilation/import completes), or is the risk of permanently missing icons considered negligible enough to accept this trade-off?
🤖 Helpful? 👍/👎
modrimkus-unity
left a comment
modrimkus-unity
left a comment
There was a problem hiding this comment.
LGTM! In addition did a manual test pass on mac - icons show correctly on first import.
lopezt-unity
merged commit d836c7e
into
coreclr/uum-131032-fast-enter-playmode
* builtin mat and csg files changes * Updating Poly2Tri for fast enter playmode * fast enter playmode for materials, log, normals * moving the rest of the runtime scripts to be compatible with fast enter playmode * update ShowHoveredInfo * Updating editor menu actions for fast enter playmode * Fixing overlay not showing on the first action * Adding Runtime Init on load attribute to reset editor scripts part.1 * switch attribute to InitializeOnEnterPlaymode for the editor only code * best support of fast entering playmode for editor scripts * fixing errors after fast enter playmode * fixing tools states on enter fast playmode * changelog * fix error on enter playmode * Fixing tools icons not loading properly when importing the package (#668) * remove old tests not used anymore
2 participants
Purpose of this PR
Fixed an issue where the PolyShape and CreateCube tool buttons would show significant letters instead of the icons when loading the ProBuilder package in a project.
This would be corrected after a domain reload.
Switching from toolbarIcon to the Icon attribute fixes that.
Links
Jira: https://jira.unity3d.com/browse/UUM-142556
Comments to Reviewers
[List known issues, planned work, provide any extra context for your code.]