Feat: Materials #79

mikhail-dcl · 2023-05-29T10:02:58Z

Closes #37

Finalized "textures loading" as they were required for Materials, and the implementation of the latter revealed the flaws of the former
Added a group of systems for Materials processing:
- Created a cache that stores the instances of the Material based on the material data. Potentially it will help if the same material setup is used in a scene, and less likely across scenes
- Creation of the material "key" is separated from the loading so different systems can consume it
- "Basic" and "PBR" Material systems are separated
- Clean-up Material system takes care of Materials which entities are dying
- Loading material lies in textures promises (entities) creation
- Constructing material consists of waiting for them, instantiating the final instance and setting all required parameters to the material
- Apply Material System will apply materials to the renderers but Primitives PR should be merged first
A step towards a prod-ready scene setup made:
- Scenes can be loaded from the folder with their dependencies (like textures)
- ContentProvider is added to take care of permissions and URLs translation
- Scene Launcher is modified to support directories instead
- In the future, the same abstraction setup will be used to load scenes from realms
Added an ECS Plugins Concept:
- Instead of registering everything in the single factory, multiple plugins should be used
- Each plugin can contain a set of individual non-exposed dependencies that are used inside that plugin only
- Each plugin registers systems of its scope based on the private and shared dependencies
  - E.g. Material Cache is a private dependency
  - ContentProvider and Entities Map are shared dependencies
  - This structure has a perfect fit as all intersystem communication happens via Components so no other scoped dependencies are necessary to share in any case
Refactored Assemblies definition to decrease their number and make the setup simpler

Add Scene Content Provider and loading from the directory

Add tests coverage

Explorer/Assets/Scripts/ECS/StreamableLoading/Components/Common/ForgetLoading.cs

Explorer/Assets/Scripts/ECS/StreamableLoading/Systems/ConcludeTextureLoadingSystem.cs

Explorer/Assets/Scripts/ECS/StreamableLoading/Tests/ConcludeTextureLoadingSystemShould.cs

Explorer/Assets/Scripts/ECS/Unity/Materials/Components/MaterialData.cs

dalkia

Amazing PR. Left some comments/questions, but overall it looks good to me

dalkia · 2023-05-29T20:52:41Z

Explorer/Assets/Scripts/ECS/Unity/Materials/Systems/CreateBasicMaterialSystem.cs

+            HandleQuery(World);
+        }
+
+        [Query]


I ve seen you started using queries without descriptors. Is this intentional? Wouldnt this query all components in the ECS world?

By default the types corresponding to the arguments are included to the query, it will not query all entities

So, what you are saying, the All descriptor is not necessary when params are present in the method?

Explorer/Assets/Scripts/SceneRunner/ECSWorld/ECSWorldFactory.cs

Explorer/Assets/Scripts/ECS/Unity/Materials/Tests/CleanUpMaterialsSystemShould.cs

dalkia · 2023-05-30T03:27:03Z

Explorer/Assets/Scripts/ECS/Unity/Materials/Systems/CreateMaterialSystemBase.cs

+            if (!entityReference.IsAlive(World))
+                return;
+
+            World.Destroy(entityReference);


WDYT about adding a DeleteEntityIntention, and have just one system destroy everything that needs to be destroyed?

It's a good idea, we still don't have a system to destroy entities?! Do we?

I thought about it further, and I will leave it as is:

DeleteEntityIntention is really good for SDK Components as we don't drive their life-cycle

Structural changes are expensive and I would like to avoid them when possible

In this case, the loading process is fully finished to this moment so we need to read data and destroy the entity, it does not make much sense to postpone it.

The clean-up itself is signaled by ForgetLoadingIntent, otherwise clean-up should happen in ConcludeLoadingSystem

Explorer/Assets/Scripts/ECS/Unity/Materials/Tests/LoadMaterialFromCacheSystemShould.cs

# Conflicts: # Explorer/Assets/Scenes/SampleScene.unity # Explorer/Assets/Scripts/CrdtEcsBridge/Tests/CRDT.ECS.Bridge.Tests.asmdef # Explorer/Assets/Scripts/ECS/ComponentsPooling/Tests/ComponentsPooling.Tests.asmdef # Explorer/Assets/Scripts/SceneRunner/ECSWorld/ECSWorldFactory.cs

m3taphysics · 2023-05-30T09:03:37Z

Explorer/Assets/Scripts/SceneRunner/Scene/SceneContentProvider.cs

+                    return true;
+                }
+
+                Debug.LogError($"{nameof(SceneContentProvider)}: {url} not found in {nameof(fileToHash)}");


We should discuss how logging should be handled in the PoC - one major issue now is that we have nothing to distinguish between user error (from the scenes themselves) and errors that we should fix. Leading to millions of partially ignored errors! Ideally for performance we should not even have logging at runtime?

Let's discuss

mikhail-dcl · 2023-05-30T10:03:14Z

Known limitation:
#80 - found a solution

Add logic to handle components deletion

Explorer/Assets/Scripts/Utility/Primitives/DefaultMaterial.cs

Get rid of MaterialsCache as it performed very bad

* Introduce Materials and textures loading * Add Scene Content Provider and loading from the directory * Simplify asm distribution * Change Launcher to use Directories * Change default Shader to DCL

mikhail-dcl added 5 commits May 25, 2023 13:42

Introduce Materials and textures loading (without Tests)

55b6822

Add Scene Content Provider and loading from the directory

Simplify asm distribution

a108800

Add tests coverage

Change Launcher to use Directories

ee37d0d

Remove an experimental class

22ae93b

Fix the path to the cube scene in the test

175876d

mikhail-dcl requested review from dalkia and m3taphysics May 29, 2023 10:31

m3taphysics reviewed May 29, 2023

View reviewed changes

Explorer/Assets/Scripts/ECS/StreamableLoading/Components/Common/ForgetLoading.cs Outdated Show resolved Hide resolved

m3taphysics reviewed May 29, 2023

View reviewed changes

Explorer/Assets/Scripts/ECS/StreamableLoading/Systems/ConcludeTextureLoadingSystem.cs Show resolved Hide resolved

m3taphysics reviewed May 29, 2023

View reviewed changes

Explorer/Assets/Scripts/ECS/StreamableLoading/Tests/ConcludeTextureLoadingSystemShould.cs Show resolved Hide resolved

m3taphysics reviewed May 29, 2023

View reviewed changes

Explorer/Assets/Scripts/ECS/Unity/Materials/Components/MaterialData.cs Show resolved Hide resolved

dalkia reviewed May 30, 2023

View reviewed changes

m3taphysics reviewed May 30, 2023

View reviewed changes

Explorer/Assets/Scripts/ECS/Unity/Materials/Tests/LoadMaterialFromCacheSystemShould.cs Outdated Show resolved Hide resolved

m3taphysics reviewed May 30, 2023

View reviewed changes

m3taphysics approved these changes May 30, 2023

View reviewed changes

Fix assemblies and launcher after merge

5cb30dd

mikhail-dcl added 5 commits May 30, 2023 13:11

Add Apply Material System

782ead1

Add logic to handle components deletion

Partial keywords were deleted

ae5e8f8

Reset Renderer.IsDirty in ApplyMaterialSystem

afa5b6e

Rename ForgetLoading to ForgetLoadingIntent

0984970

Add DestroyEntitiesSystem

371b9c5

dalkia reviewed May 30, 2023

View reviewed changes

Explorer/Assets/Scripts/Utility/Primitives/DefaultMaterial.cs Outdated Show resolved Hide resolved

mikhail-dcl added 2 commits May 30, 2023 18:38

Address performance issues

f2c55a2

Get rid of MaterialsCache as it performed very bad

Change default Shader to DCL

b02f470

mikhail-dcl merged commit ec89eaf into main May 30, 2023

mikhail-dcl deleted the materials branch May 30, 2023 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Materials #79

Feat: Materials #79

mikhail-dcl commented May 29, 2023 •

edited

Loading

dalkia left a comment

dalkia May 29, 2023

mikhail-dcl May 30, 2023

dalkia May 30, 2023

mikhail-dcl May 30, 2023

dalkia May 30, 2023

mikhail-dcl May 30, 2023

mikhail-dcl May 30, 2023

m3taphysics May 30, 2023 •

edited

Loading

mikhail-dcl May 30, 2023

mikhail-dcl commented May 30, 2023 •

edited

Loading

Feat: Materials #79

Feat: Materials #79

Conversation

mikhail-dcl commented May 29, 2023 • edited Loading

dalkia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m3taphysics May 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikhail-dcl commented May 30, 2023 • edited Loading

mikhail-dcl commented May 29, 2023 •

edited

Loading

m3taphysics May 30, 2023 •

edited

Loading

mikhail-dcl commented May 30, 2023 •

edited

Loading