Skip to content

fix: TextMeshPro materials used by TextShape are not properly destroyed#879

Merged
sandrade-dcl merged 1 commit intomasterfrom
fix/TextShape-memory-leaks
Aug 2, 2021
Merged

fix: TextMeshPro materials used by TextShape are not properly destroyed#879
sandrade-dcl merged 1 commit intomasterfrom
fix/TextShape-memory-leaks

Conversation

@sandrade-dcl
Copy link
Copy Markdown
Contributor

@sandrade-dcl sandrade-dcl commented Jul 30, 2021

What does this PR change?

Fix #772

There were lots of mesh count related to TextMeshPro on unity's profiler.
We should analyze if the TextShape component is destroyed correctly

Implemented solution

  1. Each time a TextShape object is created, we store a copy of the shared material in a private variable.
  2. Instead of using text.fontMaterial..., we use text.fontSharedMaterial... in order to not to provoke new copies of the material.
  3. Once the TextShape object is destroyed, we destroy our stored material.

NOTE: Making different tests with the TextShape components loading isolated, I have not seen a huge memory saving (the most important issue that I've detected with the number of materials in the profiler has been telated to this other ISSUE). Anyway, in order to avoid problems with the material handling, I think it is the best way to manage the cleaning of font materials.

Test link

https://play.decentraland.zone/index.html?renderer=urn:decentraland:off-chain:renderer-artifacts:fix/TextShape-memory-leaks&ENV=org

Our Code Review Standards

https://github.com/decentraland/unity-renderer/blob/master/docs/code-review-standards.md

@sandrade-dcl sandrade-dcl self-assigned this Jul 30, 2021
@github-actions
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor

@BrianAmadori BrianAmadori left a comment

Choose a reason for hiding this comment

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

LGTM

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.

TextMeshPro materials used by TextShape are not properly destroyed

3 participants