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.

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

Issue 682 - Decommission AssetBundleCreator #685

Merged
merged 49 commits into from
Jun 7, 2024
Merged

Issue 682 - Decommission AssetBundleCreator #685

merged 49 commits into from
Jun 7, 2024

Conversation

sebjf
Copy link
Contributor

@sebjf sebjf commented May 29, 2024

This fixes #682

Note: anything imported after this merge will require an updated viewer to see: https://github.com/3drepo/3drepounity/pull/534

Description

This PR removes support for AssetBundleCreator, and the written stash graph, since that is no longer needed with AssetBundleCreator gone.

This PR predominantly deletes decommissioned parts of the project, but also updates the RepoBundles export to write metadata that .io would have picked up from the stash graph, into the assets list document.

Additionally, the types have been cleaned up. SupermeshNode has been introduced as a subclass of MeshNode, and all supermesh specific properties are now members of this. The way these nodes are built throughout the code has been refactored to keep as much implementation detail in the factory methods as possible as well, instead of having classes modify the BSON objects themselves with the BSONBuilder & clone methods.

As part of this, a new class RepoMeshBSONBuilder has been added for internal use by the BSON factory, for building MeshNodes and SupermeshNodes.

These changes are "light touch" however, and only done where trivial. The expectation is that big changes will occur in #83, when we know exactly what needs to happen.
For example, though the stash graph is no longer written to disk, its types (i.e. based on RepoNodeSet) are still the same, and accessed via RepoScene in the same way.

MultipartOptimizer has been refactored so that it no longer creates stash material nodes. This is because since the stash graph is no longer written, there is no longer a need for this indirection. When exporting both textures and materials are now accessed directly from the Default graph.

The MeshMapReorganizer has been given a set of unit tests, and an outstanding bug fixed. After fixing the bug, the RepoBundles exporter has updated how it builds the mappings, and a check in the RepoBundles unit tests has been re-enabled.

Finally, the RepoBundleExport unit tests have been revised so that they no longer rely on the stash graph.

Specifically, this PR:

  1. Deletes the bouncer wrapper project & its unit tests
  2. Deletes AssetModelExport
  3. Deletes insertBinaryFileToDatabase
  4. Updates the logic of SceneManager so that it no longer writes the stash graph or Unity AssetBundles.
  5. Removes the ability to write the stash graph to the database, and deletes the CommitStash test
  6. Turns RepoUnityAssets into RepoAssets, and updates the schema to include metadata that would previously have been extracted from the stash graph on-demand.
  7. Introduces the RepoBSONMeshBuilder object for use by the BSON factory & reduces the direct use of BSON builder objects in the code in favour of factory methods
  8. Moves some utilities created for the MultipartOptimizer unit tests into a common resource for also testing MeshMapReorganizer
  9. Added unit tests for MeshMapReorganizer
  10. Fixed Large meshes are split across multiple submesh ids when they should be shared #678 & updated the RepoBundle exporter mappings logic accordingly
  11. Adds appendVector3d method to write a Vector3D as an object instead of an array
  12. Updates bouncer_worker to remove all references to the Unity Queue

Note, some of these changes are in https://github.com/3drepo/AssetGenerator/issues/3, which must be merged when this PR is.

This issue/PR also addresses:

#678
#683

@sebjf sebjf added the invalid label May 29, 2024
@carmenfan carmenfan self-assigned this May 29, 2024
bouncer/src/repo/core/model/bson/repo_bson_builder.h Outdated Show resolved Hide resolved
bouncer/src/repo/core/model/bson/repo_bson_factory.cpp Outdated Show resolved Hide resolved
bouncer/src/repo/core/model/bson/repo_bson_factory.cpp Outdated Show resolved Hide resolved
bouncer/src/repo/core/model/bson/repo_bson_factory.cpp Outdated Show resolved Hide resolved
bouncer/src/repo/core/model/bson/repo_bson_factory.h Outdated Show resolved Hide resolved
bouncer/src/repo/core/model/bson/repo_node_mesh.cpp Outdated Show resolved Hide resolved
bouncer/src/repo/core/model/bson/repo_node_mesh.h Outdated Show resolved Hide resolved
@carmenfan
Copy link
Member

@sebjf another thing that's missing here (not sure if you're planning to make it a separate ticket)

We need to change the bouncerworker so it doens't plonk the task into unityq after completing the work

@sebjf
Copy link
Contributor Author

sebjf commented Jun 3, 2024

@sebjf another thing that's missing here (not sure if you're planning to make it a separate ticket)

We need to change the bouncerworker so it doens't plonk the task into unityq after completing the work

Hi @carmenfan, done!

@sebjf sebjf removed the invalid label Jun 3, 2024
@sebjf
Copy link
Contributor Author

sebjf commented Jun 3, 2024

@carmenfan, ready for another look!

@carmenfan carmenfan self-requested a review June 4, 2024 08:43
Copy link
Member

@carmenfan carmenfan left a comment

Choose a reason for hiding this comment

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

looks fine, just 2 small organising related questions

bouncer/src/repo/core/model/bson/repo_bson_factory.h Outdated Show resolved Hide resolved
bouncer/src/repo/core/model/bson/repo_bson_factory.h Outdated Show resolved Hide resolved
@carmenfan carmenfan self-requested a review June 4, 2024 13:23
@carmenfan carmenfan merged commit 218f0c9 into staging Jun 7, 2024
9 of 13 checks passed
@carmenfan carmenfan deleted the ISSUE_682 branch June 7, 2024 11:27
@carmenfan carmenfan removed their assignment Jun 11, 2024
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.

None yet

3 participants