Skip to content

Refactor/Perf: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer#3339

Draft
Skigim wants to merge 2 commits intoopenfrontio:mainfrom
Skigim:perf/reuse-outline-filters
Draft

Refactor/Perf: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer#3339
Skigim wants to merge 2 commits intoopenfrontio:mainfrom
Skigim:perf/reuse-outline-filters

Conversation

@Skigim
Copy link
Contributor

@Skigim Skigim commented Mar 3, 2026

This PR is the second slice of updates addressing #3207 by optimizing how we draw visual outlines for structures in StructureIconsLayer.ts.

The Problem:
Currently we create separate filters each time we need a new outline, which overloads GC exponentially as the game (read - number of structures) progresses.

The Fix:
The goal is to reuse OutlineFilter to reduce GC overhead instead of drawing fresh every frame by creating one outline for each color at game start and reusing them for each structure that needs them.

  • Pull Filters into constants at the top of the file and load them exactly once right after imports.
  • Replaced calls throughout the file with references to the new constants.
  • Largest gains should come from the white filter since it shows all related structures on the map in lategame - included red and green for consistency.

Please complete the following: (this is planning commit, to align with my agreement with scamiv and to ensure that I understand the bigger picture before proceeding. Will add profiler screenshots later before marking ready)

  • I have added screenshots for all UI updates (N/A - no UI changes included)
  • I process any text displayed to the user through translateText() and I've added it to the en.json file (N/A - no new text)
  • I have added relevant tests to the test directory (N/A - change is very limited and existing test cover the file already)
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

skigim

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 2026

Walkthrough

Replaces inline new OutlineFilter(...) usages in StructureIconsLayer.ts with three shared constants: FILTER_OUTLINE_RED, FILTER_OUTLINE_GREEN, and FILTER_OUTLINE_WHITE, used for ghost outlines, upgrade indicators, and hidden-structure highlights. No public APIs changed.

Changes

Cohort / File(s) Summary
Outline Filter Constants Consolidation
src/client/graphics/layers/StructureIconsLayer.ts
Adds three shared OutlineFilter constants (red, green, white) and substitutes prior inline new OutlineFilter(...) constructions across rendering paths affecting ghost outlines, upgrade visuals, and hidden-structure highlights.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related issues

  • Issue #3207: Matches this PR's consolidation of red/green/white OutlineFilter instances in StructureIconsLayer.ts.

Poem

Three outlines now sing in line,
Red, green, white — simple and fine,
Ghosts and upgrades dressed the same,
Code tidied up, no more inline name,
A tiny change that keeps things prime.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: refactoring OutlineFilter reuse in StructureIconsLayer to reduce GC overhead, which matches the changeset's core objective.
Description check ✅ Passed The description thoroughly explains the problem, solution, and implementation details of reusing OutlineFilters, directly aligned with the changes in StructureIconsLayer.ts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Skigim
Copy link
Contributor Author

Skigim commented Mar 3, 2026

@scamiv

@scamiv
Copy link
Contributor

scamiv commented Mar 3, 2026

didn't have time to look deeper, but from a quick glance:
the change in itself seems to make sense from a code structure point of view.
it makes me wonder thou, is the filter actually reapplied each time or do we already cache the result up the chain?
also wouldn't claim this to be a perf improvement unless its actually very hot.

@Skigim
Copy link
Contributor Author

Skigim commented Mar 3, 2026

didn't have time to look deeper, but from a quick glance: the change in itself seems to make sense from a code structure point of view. it makes me wonder thou, is the filter actually reapplied each time or do we already cache the result up the chain? also wouldn't claim this to be a perf improvement unless its actually very hot.

So from what I can tell, the pixi library builds a programCache that covers the GPU side, but the js objects aren't cached on the CPU end so GC is still getting a workout with the memory overload.

I'll run profiler when I get home to see if there's a substantial benefit - if not I'll edit title to refactor.

@Skigim Skigim changed the title Perf: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer Refactor/Perf: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer Mar 6, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/client/graphics/layers/StructureIconsLayer.ts (1)

335-365: This still creates new filter arrays in the hot path.

The OutlineFilter objects are reused now, but each filters = [FILTER_OUTLINE_*] here still allocates a fresh array, and the repeated writes can keep some GC pressure in the exact path this PR is trying to cool down. A small helper that skips no-op assignments and reuses the common filter-array values would make the perf win more complete.

Also applies to: 640-641

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client/graphics/layers/StructureIconsLayer.ts` around lines 335 - 365,
The code repeatedly assigns new one-element arrays to sprite.container.filters
(e.g., this.ghostUnit.container.filters = [FILTER_OUTLINE_RED] and similar for
FILTER_OUTLINE_GREEN), causing garbage churn; add reusable const arrays like
FILTERS_OUTLINE_RED = [FILTER_OUTLINE_RED] and FILTERS_OUTLINE_GREEN =
[FILTER_OUTLINE_GREEN] and a small helper method (e.g., setFilters(container:
PIXI.Container, filters: readonly any[])) that checks if container.filters ===
filters and only assigns when different, then replace all inline assignments in
StructureIconsLayer (including the ghostUnit, potentialUpgrade.iconContainer,
potentialUpgrade.dotContainer and the other occurrences mentioned) to call
setFilters with the shared arrays to avoid allocating fresh arrays on the hot
path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/client/graphics/layers/StructureIconsLayer.ts`:
- Around line 335-365: The code repeatedly assigns new one-element arrays to
sprite.container.filters (e.g., this.ghostUnit.container.filters =
[FILTER_OUTLINE_RED] and similar for FILTER_OUTLINE_GREEN), causing garbage
churn; add reusable const arrays like FILTERS_OUTLINE_RED = [FILTER_OUTLINE_RED]
and FILTERS_OUTLINE_GREEN = [FILTER_OUTLINE_GREEN] and a small helper method
(e.g., setFilters(container: PIXI.Container, filters: readonly any[])) that
checks if container.filters === filters and only assigns when different, then
replace all inline assignments in StructureIconsLayer (including the ghostUnit,
potentialUpgrade.iconContainer, potentialUpgrade.dotContainer and the other
occurrences mentioned) to call setFilters with the shared arrays to avoid
allocating fresh arrays on the hot path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 508f6847-9e59-4f54-9678-8f3234af88e4

📥 Commits

Reviewing files that changed from the base of the PR and between d436c26 and 5389677.

📒 Files selected for processing (1)
  • src/client/graphics/layers/StructureIconsLayer.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

2 participants