Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: textures memory leak #4455

Merged
merged 15 commits into from
Apr 25, 2023
Merged

fix: textures memory leak #4455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
3fa6f29
fix: textures memory leak
pbosio Feb 17, 2023
396871d
Merge branch 'dev' of https://github.com/decentraland/unity-renderer …
pbosio Feb 27, 2023
c55545e
Merge branch 'dev' of https://github.com/decentraland/unity-renderer …
pbosio Mar 2, 2023
6be7563
Merge branch 'dev' of https://github.com/decentraland/unity-renderer …
pbosio Apr 13, 2023
0217f19
change approach
pbosio Apr 14, 2023
ae8c9c5
missing asmdef
pbosio Apr 14, 2023
c751379
reset dispose state on cancel
pbosio Apr 14, 2023
5d07f84
Merge branch 'dev' of https://github.com/decentraland/unity-renderer …
pbosio Apr 14, 2023
91d5a1b
add tests
pbosio Apr 14, 2023
37b1fa3
fix video leaks
pbosio Apr 17, 2023
7c75780
skip some frames in tests to ensure texture destroy
pbosio Apr 17, 2023
286ce89
Merge branch 'dev' of https://github.com/decentraland/unity-renderer …
pbosio Apr 18, 2023
cbb934b
fix
pbosio Apr 18, 2023
c6854d7
Merge branch 'dev' of https://github.com/decentraland/unity-renderer …
pbosio Apr 18, 2023
f08d7c8
Merge branch 'dev' into fix/texture-leak
pbosio Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions unity-renderer/Assets/Scripts/MainScripts/DCL/Components/Materials/BasicMaterial.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
using Cysharp.Threading.Tasks;
using DCL.Controllers;
using DCL.Helpers;
using DCL.Models;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using UnityEngine;
using UnityEngine.Rendering;

Expand Down Expand Up @@ -34,6 +36,9 @@ public class Model : BaseModel
private static readonly int _Cutoff = Shader.PropertyToID("_Cutoff");
private static readonly int _ZWrite = Shader.PropertyToID("_ZWrite");

private readonly DCLTexture.Fetcher dclTextureFetcher = new DCLTexture.Fetcher();
private bool isDisposed;

public BasicMaterial()
{
material = new Material(Utils.EnsureResourcesMaterial("Materials/BasicShapeMaterial"));
Expand Down Expand Up @@ -78,19 +83,25 @@ public override IEnumerator ApplyChanges(BaseModel newModel)
{
if (dclTexture == null || dclTexture.id != model.texture)
{
yield return DCLTexture.FetchTextureComponent(scene, model.texture,
(downloadedTexture) =>
yield return dclTextureFetcher.Fetch(
scene,
model.texture,
texture =>
{
if ( dclTexture != null )
if (isDisposed)
return false;

if (dclTexture != null)
{
dclTexture.DetachFrom(this);
}

material.SetTexture(_BaseMap, downloadedTexture.texture);
dclTexture = downloadedTexture;
material.SetTexture(_BaseMap, texture.texture);
dclTexture = texture;
dclTexture.AttachTo(this);
}
);
return true;
}).ToCoroutine();
// using `ToCoroutine()` since using Task directly arise some component lifecycle issues
}
}
else
Expand Down Expand Up @@ -181,6 +192,8 @@ void OnMaterialDetached(IDCLEntity entity)

public override void Dispose()
{
isDisposed = true;

dclTexture?.DetachFrom(this);

while (attachedEntities != null && attachedEntities.Count > 0 )
Expand All @@ -189,6 +202,8 @@ public override void Dispose()
}

Object.Destroy(material);

dclTextureFetcher.Dispose();
base.Dispose();
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...derer/Assets/Scripts/MainScripts/DCL/Components/Materials/DCL.Components.Materials.asmdef
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
"GUID:f070f44586498a44c9e6d2ea3e166379",
"GUID:760a1d365aad58044916992b072cf2a6",
"GUID:08b2edf8f14cdd3408988fd7746b749c",
"GUID:f62ee1cb37ad7324797360ac42f7a4d9",
"GUID:28f74c468a54948bfa9f625c5d428f56",
"GUID:0b3e983b6c2fed54ebecf9d146c251ba"
"GUID:0b3e983b6c2fed54ebecf9d146c251ba",
"GUID:f51ebe6a0ceec4240a699833d6309b23"
],
"includePlatforms": [],
"excludePlatforms": [],
Expand Down
110 changes: 57 additions & 53 deletions unity-renderer/Assets/Scripts/MainScripts/DCL/Components/Materials/PBRMaterial.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Cysharp.Threading.Tasks;
using DCL.Helpers;
using DCL.Models;
using System.Collections;
Expand Down Expand Up @@ -48,18 +49,29 @@ enum TransparencyMode
AUTO
}

public Material material { get; set; }
private enum TextureType
{
Albedo,
Alpha,
Emissive,
Bump
}

public Material material { get; private set; }
private string currentMaterialResourcesFilename;

const string MATERIAL_RESOURCES_PATH = "Materials/";
const string PBR_MATERIAL_NAME = "ShapeMaterial";

DCLTexture albedoDCLTexture = null;
DCLTexture alphaDCLTexture = null;
DCLTexture emissiveDCLTexture = null;
DCLTexture bumpDCLTexture = null;
private readonly DCLTexture[] textures = new DCLTexture[4];

private List<Coroutine> textureFetchCoroutines = new List<Coroutine>();
private readonly DCLTexture.Fetcher[] dclTextureFetcher = new DCLTexture.Fetcher[]
{
new DCLTexture.Fetcher(),
new DCLTexture.Fetcher(),
new DCLTexture.Fetcher(),
new DCLTexture.Fetcher()
};

public PBRMaterial()
{
Expand Down Expand Up @@ -110,24 +122,16 @@ public override IEnumerator ApplyChanges(BaseModel newModel)


// FETCH AND LOAD EMISSIVE TEXTURE
var fetchEmission = FetchTexture(ShaderUtils.EmissionMap, model.emissiveTexture, emissiveDCLTexture);
var fetchEmission = FetchTexture(ShaderUtils.EmissionMap, model.emissiveTexture, (int)TextureType.Emissive);

SetupTransparencyMode();

// FETCH AND LOAD TEXTURES
var fetchBaseMap = FetchTexture(ShaderUtils.BaseMap, model.albedoTexture, albedoDCLTexture);
var fetchAlpha = FetchTexture(ShaderUtils.AlphaTexture, model.alphaTexture, alphaDCLTexture);
var fetchBump = FetchTexture(ShaderUtils.BumpMap, model.bumpTexture, bumpDCLTexture);
var fetchBaseMap = FetchTexture(ShaderUtils.BaseMap, model.albedoTexture, (int)TextureType.Albedo);
var fetchAlpha = FetchTexture(ShaderUtils.AlphaTexture, model.alphaTexture, (int)TextureType.Alpha);
var fetchBump = FetchTexture(ShaderUtils.BumpMap, model.bumpTexture, (int)TextureType.Bump);

textureFetchCoroutines.Add(CoroutineStarter.Start(fetchEmission));
textureFetchCoroutines.Add(CoroutineStarter.Start(fetchBaseMap));
textureFetchCoroutines.Add(CoroutineStarter.Start(fetchAlpha));
textureFetchCoroutines.Add(CoroutineStarter.Start(fetchBump));

yield return fetchBaseMap;
yield return fetchAlpha;
yield return fetchBump;
yield return fetchEmission;
yield return UniTask.WhenAll(fetchEmission, fetchBaseMap, fetchAlpha, fetchBump).ToCoroutine();

foreach (IDCLEntity entity in attachedEntities)
InitMaterial(entity);
Expand Down Expand Up @@ -272,50 +276,63 @@ private void OnMaterialDetached(IDCLEntity entity)
DataStore.i.sceneWorldObjects.RemoveMaterial(scene.sceneData.sceneNumber, entity.entityId, material);
}

IEnumerator FetchTexture(int materialPropertyId, string textureComponentId, DCLTexture cachedDCLTexture)
private UniTask FetchTexture(int materialPropertyId, string textureComponentId, int textureType)
{
if (!string.IsNullOrEmpty(textureComponentId))
{
if (!AreSameTextureComponent(cachedDCLTexture, textureComponentId))
if (!AreSameTextureComponent(textureType, textureComponentId))
{
yield return DCLTexture.FetchTextureComponent(scene, textureComponentId,
(fetchedDCLTexture) =>
{
if (material == null)
return;

material.SetTexture(materialPropertyId, fetchedDCLTexture.texture);
SwitchTextureComponent(cachedDCLTexture, fetchedDCLTexture);
});
return dclTextureFetcher[textureType]
.Fetch(scene, textureComponentId,
fetchedDCLTexture =>
{
if (material == null)
return false;

material.SetTexture(materialPropertyId, fetchedDCLTexture.texture);
SwitchTextureComponent(textureType, fetchedDCLTexture);
return true;
});
}
}
else
{
material.SetTexture(materialPropertyId, null);
cachedDCLTexture?.DetachFrom(this);
textures[textureType]?.DetachFrom(this);
textures[textureType] = null;
}

return new UniTask();
}

bool AreSameTextureComponent(DCLTexture dclTexture, string textureId)
bool AreSameTextureComponent(int textureType, string textureId)
{
DCLTexture dclTexture = textures[textureType];
if (dclTexture == null)
return false;
return dclTexture.id == textureId;
}

void SwitchTextureComponent(DCLTexture cachedTexture, DCLTexture newTexture)
void SwitchTextureComponent(int textureType, DCLTexture newTexture)
{
cachedTexture?.DetachFrom(this);
cachedTexture = newTexture;
cachedTexture.AttachTo(this);
DCLTexture dclTexture = textures[textureType];
dclTexture?.DetachFrom(this);
textures[textureType] = newTexture;
newTexture.AttachTo(this);
}

public override void Dispose()
{
albedoDCLTexture?.DetachFrom(this);
alphaDCLTexture?.DetachFrom(this);
emissiveDCLTexture?.DetachFrom(this);
bumpDCLTexture?.DetachFrom(this);
for (int i = 0; i < dclTextureFetcher.Length; i++)
{
dclTextureFetcher[i].Dispose();
}

for (int i = 0; i < textures.Length; i++)
{
textures[i]?.DetachFrom(this);
textures[i] = null;
}

if (material != null)
{
Expand All @@ -328,19 +345,6 @@ public override void Dispose()
Utils.SafeDestroy(material);
}

for (int i = 0; i < textureFetchCoroutines.Count; i++)
{
var coroutine = textureFetchCoroutines[i];

if ( coroutine != null )
CoroutineStarter.Stop(coroutine);
}

albedoDCLTexture = null;
alphaDCLTexture = null;
emissiveDCLTexture = null;
bumpDCLTexture = null;

base.Dispose();
}
}
Expand Down
3 changes: 2 additions & 1 deletion ...renderer/Assets/Scripts/MainScripts/DCL/Components/Textures/DCL.Components.Texture.asmdef
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"GUID:a4bf61c057a098f4ca05b539a6d8c0fe",
"GUID:0b3e983b6c2fed54ebecf9d146c251ba",
"GUID:8f18b4ef38b9d4637b8190231eb24e38",
"GUID:2995626b54c60644988f134a69a77450"
"GUID:2995626b54c60644988f134a69a77450",
"GUID:f51ebe6a0ceec4240a699833d6309b23"
],
"includePlatforms": [],
"excludePlatforms": [],
Expand Down
Loading