Refactor: Reuse OutlineFilters to fix allocation issue and potential GC overhead in StructureIconsLayer by Skigim · Pull Request #3339 · openfrontio/OpenFrontIO

Skigim · 2026-03-03T17:09:16Z

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 new outline filters instead of reusing them, which adds unnecessary allocations and some extra GC work.

The Fix:
The goal is for this to fix aforementioned allocation issue in [StructureIconsLayer] by reusing [OutlineFilter] instances instead.

In theory that should help performance a little by cutting some unnecessary allocation work, but the effect is small enough that I could not measure it reliably with browser profiling.

Pull outline filters into constants at the top of the file and initialize them once after imports.
Replace inline filter creation throughout the file with references to those shared constants.

Please complete the following:

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 · 2026-03-03T17:10:00Z

Walkthrough

Replaces inline new OutlineFilter(...) usages in src/client/graphics/layers/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 (`FILTER_OUTLINE_RED`, `FILTER_OUTLINE_GREEN`, `FILTER_OUTLINE_WHITE`) and replaces prior inline `new OutlineFilter(...)` constructions across rendering paths for 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 march in tidy rows,
Red, green, white where the ghost light glows,
Replaced the inline, now constants sing,
Simple lines make the renderings spring,
Small change, neat code — a quiet win.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title accurately describes the main change: refactoring to reuse OutlineFilter constants instead of creating new instances, which addresses the allocation and GC overhead issue mentioned in the PR objectives.
Description check	✅ Passed	The PR description clearly explains the problem (unnecessary OutlineFilter allocations), the solution (reusing filter instances via constants), and the expected outcome (reduced GC overhead).

_{✏️ 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 · 2026-03-03T17:12:35Z

@scamiv

scamiv · 2026-03-03T17:34:18Z

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 · 2026-03-03T19:06:05Z

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.

coderabbitai

🧹 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

# Conflicts: # src/client/graphics/layers/StructureIconsLayer.ts

Copilot

Pull request overview

Refactors StructureIconsLayer.ts to reduce late-game GC churn by reusing pixi-filters OutlineFilter instances instead of repeatedly allocating new ones during ghost/visibility updates.

Changes:

Introduces shared red/green/white OutlineFilter constants initialized once at module load.
Replaces per-use new OutlineFilter(...) allocations with references to the shared filter instances.

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

You can also share your feedback on Copilot code review. Take the survey.

coderabbitai

🧹 Nitpick comments (1)

src/client/graphics/layers/StructureIconsLayer.ts (1)

350-380: You still allocate fresh filter arrays in the hot branches.

The filter objects are reused now, but each [FILTER_OUTLINE_*] still creates a short-lived array. If profiling still shows GC here, hoisting the one-item arrays too would remove the last new allocations from these assignments.

♻️ Possible follow-up

 const FILTER_OUTLINE_RED = new OutlineFilter({
   thickness: 2,
   color: "rgba(255, 0, 0, 1)",
 });
 const FILTER_OUTLINE_GREEN = new OutlineFilter({
   thickness: 2,
   color: "rgba(0, 255, 0, 1)",
 });
 const FILTER_OUTLINE_WHITE = new OutlineFilter({
   thickness: 2,
   color: "rgb(255, 255, 255)",
 });
+const FILTERS_OUTLINE_RED = [FILTER_OUTLINE_RED];
+const FILTERS_OUTLINE_GREEN = [FILTER_OUTLINE_GREEN];
+const FILTERS_OUTLINE_WHITE = [FILTER_OUTLINE_WHITE];

-          this.ghostUnit.container.filters = [FILTER_OUTLINE_RED];
+          this.ghostUnit.container.filters = FILTERS_OUTLINE_RED;
…
-            this.potentialUpgrade.iconContainer.filters = [
-              FILTER_OUTLINE_GREEN,
-            ];
-            this.potentialUpgrade.dotContainer.filters = [FILTER_OUTLINE_GREEN];
+            this.potentialUpgrade.iconContainer.filters = FILTERS_OUTLINE_GREEN;
+            this.potentialUpgrade.dotContainer.filters = FILTERS_OUTLINE_GREEN;
…
-          this.ghostUnit.container.filters = [FILTER_OUTLINE_RED];
+          this.ghostUnit.container.filters = FILTERS_OUTLINE_RED;
…
-        render.iconContainer.filters = [FILTER_OUTLINE_WHITE];
-        render.dotContainer.filters = [FILTER_OUTLINE_WHITE];
+        render.iconContainer.filters = FILTERS_OUTLINE_WHITE;
+        render.dotContainer.filters = FILTERS_OUTLINE_WHITE;

Also applies to: 694-695

🤖 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 350 - 380,
The code still creates new one-item filter arrays inline (e.g., assignments like
this.ghostUnit.container.filters = [FILTER_OUTLINE_RED],
this.potentialUpgrade.iconContainer.filters = [FILTER_OUTLINE_GREEN], and
dotContainer.filters = [FILTER_OUTLINE_GREEN]), causing short-lived allocations;
fix by hoisting these one-item arrays into reusable constants (e.g.,
OUTLINE_RED_FILTER_ARRAY, OUTLINE_GREEN_FILTER_ARRAY) and use those constants in
StructureIconsLayer where you set container.filters and dotContainer.filters
(also update the similar occurrences around the other noted location at lines
694-695) so the same array instances are reused instead of allocating each time.

🤖 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 350-380: The code still creates new one-item filter arrays inline
(e.g., assignments like this.ghostUnit.container.filters = [FILTER_OUTLINE_RED],
this.potentialUpgrade.iconContainer.filters = [FILTER_OUTLINE_GREEN], and
dotContainer.filters = [FILTER_OUTLINE_GREEN]), causing short-lived allocations;
fix by hoisting these one-item arrays into reusable constants (e.g.,
OUTLINE_RED_FILTER_ARRAY, OUTLINE_GREEN_FILTER_ARRAY) and use those constants in
StructureIconsLayer where you set container.filters and dotContainer.filters
(also update the similar occurrences around the other noted location at lines
694-695) so the same array instances are reused instead of allocating each time.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 82164b00-f787-49d1-a67c-15ec2b419337

📥 Commits

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

📒 Files selected for processing (1)

src/client/graphics/layers/StructureIconsLayer.ts

github-actions · 2026-03-23T02:51:33Z

This pull request is stale because it has been open for fourteen days with no activity. If you want to keep this pull request open, add a comment or update the branch.

scamiv · 2026-03-23T14:46:58Z

I think this is good if you add this so it actually removes the allocations

🧹 Nitpick comments (1)
src/client/graphics/layers/StructureIconsLayer.ts (1)> 350-380: You still allocate fresh filter arrays in the hot branches.
The filter objects are reused now, but each [FILTER_OUTLINE_*] still creates a short-lived array. If profiling still shows GC here, hoisting the one-item arrays too would remove the last new allocations from these assignments.

♻️ Possible follow-up
 const FILTER_OUTLINE_RED = new OutlineFilter({
   thickness: 2,
   color: "rgba(255, 0, 0, 1)",
 });
 const FILTER_OUTLINE_GREEN = new OutlineFilter({
   thickness: 2,
   color: "rgba(0, 255, 0, 1)",
 });
 const FILTER_OUTLINE_WHITE = new OutlineFilter({
   thickness: 2,
   color: "rgb(255, 255, 255)",
 });
+const FILTERS_OUTLINE_RED = [FILTER_OUTLINE_RED];
+const FILTERS_OUTLINE_GREEN = [FILTER_OUTLINE_GREEN];
+const FILTERS_OUTLINE_WHITE = [FILTER_OUTLINE_WHITE];

github-actions · 2026-04-11T02:50:40Z

This pull request is stale because it has been open for fourteen days with no activity. If you want to keep this pull request open, add a comment or update the branch.

VariableVince · 2026-04-11T12:27:42Z

Comment to prevent staleness. I think if the above CodeRabbit feedback would be dealt with, we could really use this merge for the better perf? Plus it opens the way for next steps in the issue to be addressed after

github-actions · 2026-04-26T03:39:47Z

This pull request is stale because it has been open for fourteen days with no activity. If you want to keep this pull request open, add a comment or update the branch.

VariableVince · 2026-04-28T22:38:57Z

This proposal is now incorporated in #3654, i'll close this PR but again hat tip to you @Skigim

@Skigim

…r context loss, use WebGL/WebGPU/Canvas, and some improvements (#3654) ## Description: StructureIconsLayer and StructureDrawingUtils fixes and improvements. Most notably have it restore structure icons after webGL context loss. Inspired by @Skigim's #3339, #3480. Fixes his #3207, contains only those fixes from the Issue that are actually valid and needed fixes, on top of his earlier merged PR. ### CONTAINS (partly written by AI, excuse the exaggerated language) **1.** * ** AutoDetectRenderer: ** now, if Hardware Acceleration is unavailable or disabled, Structure Icons will be displayed using Canvas renderer. Otherwise it will use either WebGL or WebGPU, depeding on which is available. PixiJS currently prefers WebGL but it will switch this to WebGPU at one point. We can also force it to WebGPU as explained in the comment. * ** Canvas: ** on Canvas, what doesn't work is gracefully skipped. The non-working parts will be fixed, see this issue in their repo, but until then it will work fine for us anyway: pixijs/pixijs#11981 * **WebGPU Context Loss:** PixiJS doesn't restore this context itself. Instead we do it by calling setupRenderer again on device loss. * **WebGL Context Loss:** To know when we need to restore the layer, don't use native event (`webglcontextrestored`) but use PixiJS's internal hook (`this.renderer.runners.contextChange`). This prevents our cache-clearing commands from interrupting Pixi while it's still busy rebuilding its internal GL State Machine buffer. With links severed, we need to clear and rebuild all icons to restore them. * **WebGL Context existance Check (`this.renderer.context?.isLost`):** This prevents a crash in PixiJS. Fixes black map background and all graphics frozen, which has been reported a few times. Issue created in their repo: pixijs/pixijs#12032. * **Redraw:** for Canvas context restore or on Alt-R, a call from GameRenderer now actually restores icons. Also called for WebGPU device loss and after contextChange WebGL restoration. Checks for WebGL context.isLost so a calls from Alt-R etc won't meddle while GL context is lost. * **Orphaned Object Leaks:** In PixiJS v8, `Container.destroy()` does *not* recursively destroy its children. This PR explicitly adds `.destroy({ children: true })` inside icon deletion states. This stops thousands of `PIXI.Sprite` and `PIXI.BitmapText` child nodes from leaking and choking Pixi when it attempts a WebGL restore. * **Texture Lifecycle:** Invalidate caching logic in `SpriteFactory` now correctly executes `.destroy(true)` on `PIXI.Texture` objects. Previously, they were only deleted from the textureCache Map, retaining a phantom grip on GPU memory buffers. * **Don't remove PIXI.Texture.EMPTY from textureCache: `createTexture()` in `SpriteFactory` stores `PIXI.Texture.EMPTY` (a singleton) in `textureCache` when a structure type has no known shape. When not preventing removal of the EMPTY texture, `clearCache()` would call `texture.destroy(true)` on PixiJS's shared global empty texture, breaking all sprites in the renderer that fall back to it. **2. Small Memory/Perf Optimizations** * **The Shared 2D Canvas Optimization:** To prevent allocating endless tiny `<canvas>` elements every time a structure color is loaded, `SpriteFactory` now utilizes a cleanly shared `colorCanvas` singleton. To keep this safe from hardware acceleration crashes (where the 2D context dies alongside WebGL), it accurately nullifies itself in `clearCache()` and lazily instantiates on the next call (`getImageColored()`). * **Bypassing Inefficient Textures Cache:** Now passing the `skipCache: true` argument implicitly to dynamic UI elements via `PIXI.Texture.from(structureCanvas, true)`. * **Zero-Allocation Filters (No more GC Stutters):** `renderGhost()` previously spawned numerous `new OutlineFilter(...)` WebGL shaders when hovering over invalid tiles, compounding to many leaked Shader Programs. We hoisted these filters to static class properties initialized once, and went a step further: hoisted the wrapping Array structures too (`filterRedArray`, `filterGreenArray`). This eliminates many pointless micro-allocations and GC sweeps entirely. **BEFORE, for webGL:** https://youtu.be/durJxNFNePs **AFTER, for WebGL:** https://youtu.be/VnYEFMx4gfM **AFTER, for Canvas:** https://youtu.be/zT720oKxcaI **AFTER, for WebGPU:** https://youtu.be/J09Wee2qTs8 The performance optimizations weren't well measurable in my tests but there's no downgrade at least. WebGPU should bee better than WebGL when we would force it but again, currently PixiJS prefers WebGL hardcoded so only if we disallow WebGL will it use WebGPU if it is available, otherwise fallback gracefully to Canvas still. Canvas skips parts gracefully, as long as the non-breaking issue exists in PixiJS (as explained above): <img width="952" height="705" alt="AFTER Canvas in Firefox skips non-supported gracefully" src="https://github.com/user-attachments/assets/17e8d8ab-05dc-47cb-ab11-f0f4d015a42a" /> ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] 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: tryout33 --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Perf: Reuse OutlineFilters to reduce lategame GC overhead

d436c26

github-project-automation Bot added this to OpenFront Release Management Mar 3, 2026

github-project-automation Bot moved this to Triage in OpenFront Release Management Mar 3, 2026

github-actions Bot assigned Skigim Mar 3, 2026

coderabbitai Bot previously approved these changes Mar 3, 2026

View reviewed changes

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

Merge branch 'main' into perf/reuse-outline-filters

5389677

coderabbitai Bot reviewed Mar 6, 2026

View reviewed changes

Skigim changed the title ~~Refactor/Perf: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer~~ Refactor: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer Mar 8, 2026

Merge branch 'main' into perf/reuse-outline-filters

3b4caa8

# Conflicts: # src/client/graphics/layers/StructureIconsLayer.ts

Skigim dismissed coderabbitai[bot]’s stale review via 3b4caa8 March 8, 2026 06:10

Skigim marked this pull request as ready for review March 8, 2026 06:12

Skigim requested a review from a team as a code owner March 8, 2026 06:12

Copilot AI review requested due to automatic review settings March 8, 2026 06:12

Skigim changed the title ~~Refactor: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer~~ Refactor: Reuse OutlineFilters to fix allocation issue and potential GC overhead in StructureIconsLayer Mar 8, 2026

Copilot started reviewing on behalf of Skigim March 8, 2026 06:13 View session

Copilot AI reviewed Mar 8, 2026

View reviewed changes

coderabbitai Bot reviewed Mar 8, 2026

View reviewed changes

coderabbitai Bot approved these changes Mar 8, 2026

View reviewed changes

github-actions Bot added the Stale PRs that haven't been touched for over two weeks. label Mar 23, 2026

github-actions Bot removed the Stale PRs that haven't been touched for over two weeks. label Mar 24, 2026

github-actions Bot added the Stale PRs that haven't been touched for over two weeks. label Apr 11, 2026

github-actions Bot removed the Stale PRs that haven't been touched for over two weeks. label Apr 12, 2026

VariableVince mentioned this pull request Apr 12, 2026

Fix/refactor/optim(StructureIconsLayer): restore structure icons after context loss, use WebGL/WebGPU/Canvas, and some improvements #3654

Merged

4 tasks

github-actions Bot added the Stale PRs that haven't been touched for over two weeks. label Apr 26, 2026

VariableVince closed this Apr 28, 2026

github-project-automation Bot moved this from Triage to Complete in OpenFront Release Management Apr 28, 2026

Conversation

Skigim commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Please complete the following:

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

Uh oh!

coderabbitai Bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

Uh oh!

Skigim commented Mar 3, 2026

Uh oh!

scamiv commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Skigim commented Mar 3, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 23, 2026

Uh oh!

scamiv commented Mar 23, 2026

Uh oh!

github-actions Bot commented Apr 11, 2026

Uh oh!

VariableVince commented Apr 11, 2026

Uh oh!

github-actions Bot commented Apr 26, 2026

Uh oh!

VariableVince commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Skigim commented Mar 3, 2026 •

edited

Loading

coderabbitai Bot commented Mar 3, 2026 •

edited

Loading

scamiv commented Mar 3, 2026 •

edited

Loading