Skip to content

⚡ Optimize material color provider lookup#236

Open
CanerKaraca23 wants to merge 1 commit intouser-grinch:mainfrom
CanerKaraca23:optimize-material-color-lookup-3207791642246816576
Open

⚡ Optimize material color provider lookup#236
CanerKaraca23 wants to merge 1 commit intouser-grinch:mainfrom
CanerKaraca23:optimize-material-color-lookup-3207791642246816576

Conversation

@CanerKaraca23
Copy link
Copy Markdown
Contributor

⚡ [performance improvement] Optimize material color provider lookup

💡 What:

Optimized the material color provider lookup in ModelInfoMgr by replacing a linear search with a type-indexed lookup.

🎯 Why:

The previous implementation iterated through every registered provider for every material being rendered, even if the provider was only intended for a specific material type (e.g., sirens). This was particularly inefficient as the number of providers grew.

📊 Measured Improvement:

In a simulated environment:

  • Hit Case (Specific type): ~3.3x faster (0.21s -> 0.06s for 10M iterations)
  • Miss Case (General type): ~9.4x faster (0.36s -> 0.038s for 10M iterations)

These improvements were achieved by reducing the number of lambda calls and iterations from O(N) to O(1) in the common case.

The previous implementation of `ModelInfoMgr::FetchMaterialCol` used a linear search through all registered `matColProviders`, regardless of the material type being requested. This led to unnecessary iterations and function calls, especially for features like sirens that register providers for specific material types.

This optimization implements a type-indexed lookup by:
1. Changing `matColProviders` from a single `std::vector` to an array of vectors indexed by `eMaterialType`.
2. Updating `RegisterMaterialColProvider` to accept a material type, defaulting to a "global" bucket (`eMaterialType::TotalMaterial`).
3. Modifying `FetchMaterialCol` to only iterate through providers registered for the specific type, followed by global providers if no match is found.
4. Updating `Sirens::Init` to use type-specific registration for siren materials.

Benchmark results on a simulated environment showed a performance improvement of approximately 3x in hits and 9x in misses.
Copilot AI review requested due to automatic review settings May 8, 2026 19:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Optimizes ModelInfoMgr material color provider selection by storing providers in per-eMaterialType buckets and checking the relevant bucket before falling back to a general bucket, avoiding a full linear scan across all providers per material.

Changes:

  • Replaced the single matColProviders vector with a fixed array of vectors indexed by eMaterialType (plus a general/fallback bucket).
  • Updated RegisterMaterialColProvider to accept an optional eMaterialType to register against.
  • Updated the siren material color provider to register specifically for eMaterialType::SirenLight.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/utils/modelinfomgr.h Changes storage and API to support type-indexed provider registration.
src/utils/modelinfomgr.cpp Implements type-indexed registration and lookup with a fallback provider bucket.
src/features/sirens.cpp Registers siren color provider under the siren material type bucket.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

void ModelInfoMgr::RegisterMaterialColProvider(MaterialColProviderCallback_t mat, eMaterialType type)
{
matColProviders.push_back(mat);
matColProviders[type].push_back(mat);
Comment thread src/features/sirens.cpp
@@ -607,22 +607,20 @@ void Sirens::Init()

ModelInfoMgr::RegisterMaterialColProvider([](CVehicle *pVeh, RpMaterial *pMat, eMaterialType type)
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.

2 participants