3drepo / 3drepobouncer

A C++ library providing 3D Repo Scene Graph definition, repository management logic and manipulation logic. It is is essentially the refactored 3DRepoCore and (parts of) 3DRepoGUI
GNU Affero General Public License v3.0
29 stars 13 forks source link

Decommission AssetBundle Support #682

Closed sebjf closed 3 months ago

sebjf commented 4 months ago

Description

Remove support for AssetBundles and make associated improvements as per this product ticket: https://github.com/3drepo/3D-Repo-Product-Team/issues/402

Goals

Tasks

Questions

  1. Should we leave the MISSING_BUNDLES UploadStatus flag, given that it is hooked up outside bouncer?

Related Resources

https://github.com/3drepo/3D-Repo-Product-Team/issues/402

sebjf commented 3 months ago

Hi @carmenfan,

A few questions on this ticket when you have a sec!

  1. I have made a SupermeshNode type that subclasses MeshNode. I will leave this even though it subclasses a Mongo BSON type as a supermesh is naturally a specialisation of submesh, and we can decide what to do about it inheriting from a Mongo type when we do the driver upgrade.

  2. Similarly, I've left both the stash and default graph as the same type. This is because accesses to these go through RepoScene anyway, so I don't see any type-safety benefits of making them separate types - for now.

Do you agree, or would you like to split either of them up further immediately?

  1. For flags like MISSING_BUNDLES, do you want me to leave the definitions in bouncer with a comment that they may exist in the database but are not written anymore, or just remove the definitions entirely?

  2. Do you want me to look into moving/duplicating texture entries(*) so we can sort out this properly, or are we not bothered?

(*) One simple approach for example may just be to make copies of .scenes, texture Nodes in the now in-memory only stash graph. They would remain in .scene in the database for the backend, but it would mean we can free the entire graph. Or, if we think we will soon do something like atlasing, we could create a new collection for textures.

carmenfan commented 3 months ago
  1. ok
  2. yes fine as well.
  3. Let's leave the definition - it will be confusing if we reuse the enum
  4. If this is simple enough to do yes, if not then we can address it as part of #683. With the ever growing model size, the more memory we can work with the better
sebjf commented 3 months ago

(Just a note for later, but it might be worth running a migration script so we can decommission the assetsMeta API in the near future. Such a script might be feasible since iterating over the stash graph is done every time a model loads anyway at the moment!)