-
Notifications
You must be signed in to change notification settings - Fork 13
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 654 - gzipping asset bundles #656
Conversation
… the vector around. Updated the wrapper to store the encoding in the bson so it gets written to the db.
Hi @Charence, the build failed on this branch previously. I have started it again but both times I cannot see its progress: Do you know what's up with it? |
wrapper/src/c_sharp_wrapper.cpp
Outdated
in.push(inputStream); | ||
boost::iostreams::copy(in, ss); | ||
|
||
repo::core::model::RepoBSONBuilder builder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't do it this way. This is quite difficult to read and making the way we store metadata data about a file highly coupled with the functions/a classes that are simply passing in the files.
I think this could be a lot simpler (Unless i'm missing something, which I'm often am!). I think the file manager can manage the gzipping and it should just be a flag on the function https://github.com/3drepo/3drepobouncer/blob/master/bouncer/src/repo/core/handler/fileservice/repo_file_manager.h#L60
If this is set to true, it would know to append the gzip flag onto the metadata of the file ref.
We could just gzip all the geo files we export, or it could be a flag in the exporter to indicate whether we want to gzip this. (Persumably we benefit from gzipping the SRCs too, but I'm not sure how unreal handles those requests!)
And it would also make gzipping the JSON caches much easier to do if we want to implement it in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @carmenfan, this should be resolved now!
… to control it. Set all the webbuffers to be gzipped by default, because .io can decompress as required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebjf
This runs fine, works fine, but failing to build in linux
@carmenfan, this should be fixed now for when Travis comes back! |
/azp run |
No pipelines are associated with this pull request. |
This fixes #654
Description
This PR gzips the Unity Asset Bundles by default, and provides a metadata BSON so that the state of this encoding is stored in the db.
This PR exists for now just for feedback.