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

Adminotech - AssetAPI asset bundle support + ArchivePlugin for zip bundle implementation #508

Merged
merged 21 commits into from Oct 23, 2012

Conversation

jonnenauha
Copy link
Member

This pull request encapsulates two things. First proper support for asset bundles to AssetAPI and secondly ArchivePlugin that implements zip bundle support agains the new functionality in AssetAPI.

I recognize that this is very big commit and touches lots of places in AssetAPI. I'd be happy to receive code review and suggestion how to take the work forward/do fixes if there is need. Especially from people who have been working with the AssetAPI internals before.

AssetAPI

AssetAPI now has a consept of asset bundles. It understands what they are and that they are there to provide IAssets. IAssetBundle is a new interface that needs to be implemented when someone wants to introduce a new asset bundle type. There is also similarly IAssetBundleTypeFactory just like IAssets have factories. These factories can be registered from 3rd party plugins (like ArchivePlugin). The factories create asset bundles that again provide sub assets.

AssetAPI has been modified to understand the difference of bundles and sub assets inside of them. This is totally transparent to the code that actually calls AssetAPI::RequestAsset(...). Asset references in Tundra can now be bundle#subAsset eg. assets.zip#materials/my.material. When a request like this comes to the asset system it will detect that it first needs the asset bundle in front of the # separator. It will request the bundle normally via an asset transfer that will be materialized into the IAssetBundle with the bundle factory. Once the bundle has been loaded successfully all the pending sub asset requests to it will be loaded from the bundle and their asset transfers will succeed or fail depending on weather the bundle can provide them. Again this is totally transparent to 3rd party code that calls RequestAsset(bundle#subAsset) and the flow is exactly the same.

Relative references will work inside a sub asset like they do normally. If "assets.zip#materials/my.material" depends on my.png the context will be correctly resolved to "assets.zip#materials/" if the dependent sub asset is in the root it will resolve correctly to "assets.zip#".

As a last note: IAssetBundle was made for the zip implementation but it is meant to be for generic Tundra bundle support. This means other modules can implement what ever they please with this feature. One that has been talked a lot is the collada file support, that actually is a bundle that can contain multiple meshes/materials/textures. Following the example from ArhiveModule this should now be a trivial task in a 3rd part plugin instead of hacking around AssetAPI limitation because it does not understand the notion of bundles.

Feature summary

  • New IAssetBundle and IAssetBundleTypeFactory for plugins to implement and register their bundles.
  • Fixed the hardcoding away from AssetAPI::GetResourceTypeFromAssetRef. This made AssetAPI aware of all the potentially supported asset types even if their factories were in 3rd party modules. This function is no longer static and it will query all the registered IAsset and IAssetBundle factories if they can handle the file extension of the requested reference. There was a long standing todo in the function that the type should be asked from the factories and not be hardcoded.
  • Fixed local asset provider not to block if there are lots of failing requests. This made Tundra unresponsive if there was eg. 500 local refs to disk that did not exist for a long time. Essentially adds the same throtling check when the requests fail like it is in the succeeded code path. This was particularly a problem for scene that used EC_Materials output name as local://generatedname.material with hundreds of objects. These generated ones first go to the local provider, fail with nasty error prints and are reloaded via AssetRefListener once they are programmitically generated in EC_Material.
  • Assets windows (shift+a) has been modified to show all IAssetBundles just like we do with storages. It will show the zip and all sub assets loaded into memory from it under it. You can reload/unload/refetch/modify/open cache file for the sub assets with the right click menu as you would expect.

ArchivePlugin

This new Tundra plugin provides asset bundle support for ZIP files. It is a optional module but my proposal is to make it part of the core distribution. In this pull request it is turned on by default for windows and can be enabled to linux/mac as soon as their build scripts satisfy the zziplib dependency. Windows full build script now has support for building zziplib and zlib (dependency for zziplib) from sources.

Motivation for the work

ZIP file support is important for Tundra as we start to get bigger and bigger scenes. Lets take a example scene where we have 600 http asset references to the web. Now we already have our AssetAPI and AssetCache aware of last modified checks and it will only download the files from the web if they have been changed. But the last modified is always done to the server never the less (so we know if it has changed). Jukka and friends at Ludo have already shows some measurements that show that AssetAPI takes significant time to process these in our example 600 last modified web requests, its a huge amount of web requests even if they are very small in size.

ZIP support mitigate this problem to as many last modified requests as there are zip bundles in the scene. If we pack all the static content that is not expected to change when the scene is ready to a single zip file, we end up with one request check, no re-download for the zip from the web, threaded unpacking of the zip file and asynch loading (for those types that support it eg. ogre assets) for the sub assets.

Implementation details

ArhivePlugin is now supporting zip files via zziplib library. Current implementation extracts the whole file into the asset cache as is with the full asset reference. I decided to go with this route as it seems nicer to do this rather than for every sub asset request open the zip file in memory and read the data for the sub asset. Additional benefit from this is that we get perfectly valid DiskSource() for the sub assets that get loaded. Which makes OgreMeshAsset and friends load the file asynchronously as there is a valid AssetCache entry for them.

Update: I made the worker only extract files that have changed. Meaning if the zip was not re-downloaded from source, only missing sub assets will be extracted. This will save a lot of time for big zip files.

Why its called ArchivePlugin is that my idea is to expand support to .7z, .rar or any other "archive" file format into this plugin. This ways we don't have to make small plugins for each file format. The factory has been designed so that it can provide multiple bundles in its CreateEmptyAssetBundle() by detecting the file suffix. Only thing that needs to be made is a IAssetBundle implementation and a few lines to ArchiveBundleFactory.

Feature summary

  • Registers "Archive" type bundle factory to AssetAPI.
  • Deserializes the zip to asset cache in a worker thread that won't block the main loop.
  • Provides sub asset data from the bundle to AssetAPI to make the IAssets.

Please contact me here if you need a test scene with a txml and zip to test this. You can roll up your own easily from any existing scene. Just zip the assets file structure and put "myzip.zip#" in front of every reference in the scene!

Jonne Nauha added 4 commits July 22, 2012 06:47
…es to AssetAPI. This is a generic way to AssetAPI internals so that you can request sub assets from bundles, may it be a collada bundle or a zip file. ArchivePlugin now implements a working zip bundle type and factory. Other file types can be supported by simply adding the IAssetBunle implementations to the plugin. You request sub assets normally via ref eg. '<bundleref>#path/to/subasset.mesh', you don't need to request the bundle before hand or worry about anything like that. You will get a asset transfer as before and AssetAPI loads the bundle first then queries your sub asset from it. Additionally implemented Assets Window to show bundles and loaded assets from it. Current zip implementation unpacks the whole zip file to disk and provides the sub asset data from disk. TODO: Support live reloading of bundles and thread the asset bundle unpack stage so it wont block the main loop. Note: This is not final code, will be worked on more.
…actories for file extensions they can create assets from. This removes the long time hardcode hack that made AssetAPI aware of the types that each factory can produce, which is quite bad. Similarly ask asset bundle factories the same. AssetBundles: Added proper fullbuild deps for zziplib and zlib (dependency of zziplib).
…Fix ref resolving when the context is a bundle. Fixed local provider not to block if there are lots of failed requests, failing 1000 requests in a row made tundra unresponsive for that whole perioid (frustrating especially in debug mode with all the log spam). ZipBundle: Implemented threaded worker for unzipping so it wont block the main loop. Fixed cache path to be a proper file in asset cache, meaning eg. ogre threaded loading will now work seamlessly when using sub assets in a zip file. Added ArchivePlugin to be default on in windows (linux/mac needs scripts to get the zziplib dep first).
@Stinkfist0
Copy link
Contributor

As a minor note, the ArchivePlugin class seems like a class providing no functionality whatsoever and could be deleted.

… check if the zip files need to be extracted. This will save us from always uncompressing the full zip every time. Even if that is threded, with big zip files it will delay us getting the assets from the bundle. Now we check if sub asset has same cache time stamp than its parent zip. If not we extract it and set the last modified stamp to same. This will save us from downloading big zip files (AssetAPI does that automatically) and we dont have to extract a single file for the bundle to be ready if the source was not redownloaded. AssetAPI: Remove some left debug prints and tweak some prints.
@jonnenauha
Copy link
Member Author

@Stinkfist0 yeah thanks, seems I'm too fixated to IModule even if its not used for anything :) Fixed in the new commit.

Additional features in the new commit: Don't extract files again that are already up to date in the cache. The sub assets are now marked with the parent zip bundles time stamp on extraction. If the zip file is not redownloaded from source (eg. http ref) and has not changed, we dont need to extract a single file. Which is a huge time saver for big zip files. If you have manually or via Tundra cleared asset cache, then the missing sub assets (and only them) will be extracted.

Jonne Nauha and others added 4 commits August 1, 2012 18:13
@jonnenauha
Copy link
Member Author

Ok updated with fix to unload crash. I'm not sure why that actually crashed as the shared ptrs in the local vector should have been freed automatically. But ofc better to properly handle the uninit code. Seems that the list had shared_ptrs and it crashed to ~AssetAPI() :P

@jonnenauha
Copy link
Member Author

Thanks cvetan for the mac support :) I think this is now ready to go in.

…infinite loop in there (tired + copy pasting is bad combo). Now its fixed and properly tested to work. Good thing we did more testing on this thing and caught this before ended up merged to main repo. Yey for testing.
framework->Asset()->AssetTransferFailed(transfer.get(), reason);
// Also throttle asset loading here. This is needed in the case we have a lot of failed refs.
Copy link
Member Author

Choose a reason for hiding this comment

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

We have now found that this throttling on failing local:// requests broke scenes that have large amount of EC_Materials. EC_Material "broken" flow changes timings because all of the output material refs did not fails at once when the scene came to the client. When some of the EC_Material output material request in EC_Mesh failed here with a delay it broken the "hacked/to be removed" logic of AssetRefListener to hook to AssetCreated when the request failed. Now when the request failed AssetRefListener was too late to connect to AssetCreated, as the generated output material had already been created and that signal for the failed ref never came. Hence no reloading after the creation was done and EC_Materials broke.

The EC_Material logic is completely flawed and I'm working on a fix right now that would not require us removing these throttling lines. But it seems that it may be required. Once fix im doing is creating a notion of "generated://" refs that are never requested from the AssetAPI in AssetRefListener but it waits of AssetCreated signal and then monitors Loaded for that generated material, I also had to add proper Loaded signal emitting after dependencies have been fetched for cloned material assets.

P.S Scenes with ~low amount of EC_Material output request fails will not break because there are so little of them that this throttling never hits in. Once it does you are screwed, eg. hundreds of EC_Materials in scene.

Jonne Nauha added 3 commits September 15, 2012 01:28
…ning it via QThreadPool. This give the additional benefit of Qt running them where the is room, not just spinning N threads once the zip needs unpacking.
@jonnenauha
Copy link
Member Author

@juj I added QThreadPool into the picture, as requested from you. This is not Tundra wide mechanism but could be made one imo. People could implement QRunnable and push them to our ThreadAPI that would push them to QThreadPool, or we just document that this is the way people need to use asynch stuff. QThreadPool guards us from running too many threads at the same time across Tundra.

Additonally fixed ove 260 char filenames not being opened via windows api:s in AssetCache, leave me a note if the fix is not elegant enough, at least gets the job done. Fixes #347, actually does not fix the unicode part as there is still the QString::toLocal8bit() but dunno how many asset refs are doing to include unicode chars (urls basically as they are only things cached to asset cache atm)? :P

Jonne Nauha added 6 commits September 19, 2012 13:41
…urce, avoids unneccesary data shuffling in the load from, this is how normal transfer do it also. Dont start the threaded unzip worker if there is nothing to uncompress.
…undra. New callback to AssetAPI for providers to call on aborted transfers, so that we keep currentTransfers up to date. Fix bad iter crashes when forgetting all assets (abort all trasnfers step), make sure we wont end up to infinite loop there with the new logic.
…is could have been fixed hackisly with rputting the virtual transfers to 'readyTransfers', but that does not guarantee loading. The proper way is to delay LoadSubAssetToTransfer call to the next update so that the calling code has time to hook into the virtual transfer before any of the load path signals are emitted. Bug was noticed on mac where async ogre loading was disabled, hence all signals fired before calling code could hook.
@jonnenauha
Copy link
Member Author

  • Merged against current rex/tunda2
  • Added commit that remove throttling of failed local:// requests.

@juj
Copy link
Contributor

juj commented Oct 16, 2012

Are .js files inside .zip files supported? I tried to package the avatar example scene as a zip file, available here:

https://dl.dropbox.com/u/40949268/Tundra/bugs/avatarzip/avatarzip.zip
https://dl.dropbox.com/u/40949268/Tundra/bugs/avatarzip/scene.txml

But I get the following error when .js files are being loaded:

Error: AssetAPI: Failed to load sub asset 'local://avatarzip.zip#avatarzip.zip#simpleavatar.js from bundle 'local://avatarzip.zip': Sub asset does not exist.
Error: AssetAPI: Failed to load sub asset 'local://avatarzip.zip#avatarzip.zip#exampleavataraddon.js from bundle 'local://avatarzip.zip': Sub asset does not exist.

@jonnenauha
Copy link
Member Author

Yes any Tundra supported asset should work transparently from inside zip files. I think the actual bug here is the default local storage. I did try with local:// refs a lot but seems there is some slight bug. Clearly the ref is somehow broken what its trying to find inside the zip local://avatarzip.zip#avatarzip.zip#simpleavatar.js the avatarzip.zip is there two times.

Let me get back to you on this. Are you in a rush to evaluate this? Also could you try putting them to dropbox or similar to get the default storage for relative refs as a HTTP storage. I'd be interested if everything is fine then.

@juj
Copy link
Contributor

juj commented Oct 18, 2012

I'm currently evaluating this for regressions. Fixing the above is not critical for merging, but would be good to figure out what's going on there. Also an example scene would be nice to have.

@juj juj merged commit 15e3f5b into realXtend:tundra2 Oct 23, 2012
@ghost ghost assigned juj Oct 23, 2012
@juj
Copy link
Contributor

juj commented Oct 23, 2012

Tested and works good. Great work!

An additional commit on top of this was added to have AssetAPI::ParseAssetRef recognize both "," and "#" characters to delimit subasset names. Canonicalizing an assetref will produce asset refs with the "#" delimiter.

I'm ok to migrating to using "#" as the delimiter at some point, and dropping support for ",", but that will require time to allow CyberLightning and LudoCraft to migrate. For now, both forms are supported.

In the future, I hope we can unify the asset dependency tracking so that zip bundles won't require a separate dependency tracking mechanism, but a single unified system is used. Conceptually it looks like that is doable, and would simplify the now-grown Asset API quite a bit.

Also, in the future, I hope that we'd get a unified threaded loading mechanism that governs the loading of all types of assets, so that we won't need to use thread pools specific to zips. Then loading all the assets (that can enable it) can utilize background loader threads.

@jonnenauha
Copy link
Member Author

Supporting both sub asset delims sounds reasonable. Thanks for the review. Hopefully we can get the rest of the pull requests merged and fixed for this. I'll check my own at some point. At least CRN will need a brief commit to fix the extension to be fed into the OgreTexture factory instead of being inside AssetAPI.

For the unified queuing of normal and bundle transfers: I'm sure it can be done somehow neatly. Right now the main bundle fetch to the eg. zip is found in asset.GetAllTransfers(). The bending sub assets for that zip is not exposed to outside code. Maybe if we could make public AssetBundleTransfer : IAssetTransfer class that has the additional tracking in it for pending sub assets that are fired when the zip fetch completes. This way we could still return QList<IAssetTransfer> AssetAPI::GetAllTransfers() but some of them could be bundle transfers and you could digg out all the needed information from there. The AssetBundleMonitor could probably be somehow tied also the AssetBundleTransfer. This could bring the complexity down a bit, but would require some dynamic casting inside AssetAPI to detect things once the transfer finishes.

For the common threading/thread pool mechanism I agree. This would be sweet. Maybe we can jump on these things when we want to refactor the asset factories too.

Hopefully @Stinkfist0 can get going on his single TundraCore dynamic lib now that this is in!

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