Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Adminotech Tundra fixes #480

merged 14 commits into from May 25, 2012


None yet
2 participants

jonnenauha commented May 24, 2012

  • Call AssetAPI::ForgetAllAssets() when client disconnects from a server. This needs to be done if we want Tundra to be able to login to multiple scenes in one instance/run. If this is not done, eventually client will run out of memory and crash. Fixes #472
  • Fix linking incorrectly to RenderSystem_Direct3D9 if CMakeLists.txt ENABLE_DIRECTX == false.
  • Protect against null service objects (framework property maps QObjects) while prepping JS engine.
  • Implement a Abort() mechanism for IAssetTransfer. This flag cannot be set from scripts, AssetAPI sets aborted when appropriate. This lets the provider know that once the load finishes (in http providers case for example) to not load anything into memory or store the cache file. Additionally AssetAPI can detect aborted flag and ignore the finished/failed transfer. Before it incorrectly did error logging for the transfer we didn't even expect anymore and constructed from its data an IAsset into memory.
  • Make the load path for AssetAPI uniform how AssetLoadFailed is called. Before there were situations where disk source had "a" path but the file was no present. In this scenario AssetAPI was never informed that the load actually failed which left dead transfers into the internal state. There might have been other obscure asset load paths that resulted in the same bug.
  • Fix crash that occurred if Ogre IAsset was loading the data asynchronously in Ogre and it was destroyed/unloaded. This left a dangling IAsset* into Ogre that it called back when the asynch load completed. Now if a asynch operation is ongoing the request is aborted in IAsset::DoUnload().
  • Try to recover from UI blit rects that have areas outside the scene rect. Add more information to error logging about the rects in play when DX surface subrect blit fails.
  • Enable CanvasPlugin into the default Tundra build and startup configs.
  • Fixed ~EC_RttTarget crash bug in headless server. Underlying bug reported in #481

jonnenauha and others added some commits May 18, 2012

@jonnenauha jonnenauha AssetAPI: Fix bug where failed asset loads left their asset transfers…
… into the transfer map incorrectly indefinately.
@jonnenauha jonnenauha JavascriptModule: Prevent null crashes on framework property QObjects…
… if the ptr has been freed, not a active bug bug a lurking one if some 3rd party module misuses the dyn obj registration to framework. On a side note: We should add a remove counterpart to RegisterDynamicObject(...).
@jonnenauha jonnenauha AssetAPI/AssetModule: Implement a Abort() mechanism to IAssetTransfer…
…. For now the abort is only used from AssetAPI itself in ForgetAllAssets() (when it also clears the shared_ptr transfer map, meaning its no longer expecting it), it cannot be called from scripts. AssetAPI will just ignore Aborted() transfer when they finish and make the appropriate callback to AssetAPI and not load anything into mem. Before the Abort() mechanism any 'forgotten' transfers finishing, meant we logged a warninig but still loaded the asset fully to memory (yes, to ogre aswell). This should never happen if AssetAPI is told to forget all assets and ongoing transfers.
@jonnenauha jonnenauha Client: After removing the client scene when disconnecting from a ser…
…ver, forget all assets in AssetAPI. This prepares Tundra to be more suitable for multiple scene logins on one Tundra instance. If we will not do this, all the visited scenes assets will stay in memory and after a while you will not be able to login to any scene without running into our of memory issues.
@jonnenauha jonnenauha Rendere: Add more useful information on rendering errors where sub re…
…ct of the DX surface could not be locked. Add code that will correct if the constructed blit subrect is (partly) outside our scene rect.
@jonnenauha jonnenauha Threaded ogre asset crash fixes: If the asynch load operation is ongo…
…ing in ogre make sure to abort it when unloading the IAsset (so also for dtor/freeing the asset). If this is not done, ogre will have a dangling callback ptr to our IAsset and will crash once the threaded asset load is completed. Now its safe to forget/unload/free ogre assets that are currently being loaded asynchronously.
@jonnenauha jonnenauha Add CanvasPlugin to the default build. 587cc43
@jonnenauha Cvetan Stefanovski AssetAPI: Use interator instead of const_iterator - GCC does not allo…
…w erase() with const_iterator as argument
@jonnenauha jonnenauha FindOgre.cmake: Fix that we do not link against DX render plugin if D…
…irectX is not enabled in the CMakeLists.txt:s defined.
@jonnenauha jonnenauha Add CanvasPlugin to viewer and default server startup configs. 2da8db8
@jonnenauha jonnenauha EC_RttTarget: Crashes server on dtor where asks ViewEnabled() if ogre…
… stuff should be unloaded. The bug is actually in ViewEnabled() as it returns true if parent entity is null (which it is in component dtor). fw->IsHeadless() is safe and is essentially same thing. Making a issue about the weird behaviour of ViewEnabled() in dtor.

juj commented May 25, 2012

Is it possible to implement the asset transfer abort mechanism without using a 'bool aborted;"? If Abort() is called on a transfer, can we just cancel the transfer right there and then, notify the provider about the abort request and have it handle it.

That would be a stateless design, and would cause the transfer itself to abort, so that abort wouldn't mean "finish transfer and discard.".


jonnenauha commented May 25, 2012

We'll the transfer itself is not "doing" the data trasnfer, its just waiting for the result from the provider. So its waiting in an aborted state. I guess we could replace the boolean with a signal that providers listen to and remove from their internal maps so the transfer would never go to AssetAPI::LoadFailed/Complete. Is this what you are looking for? There might just be some code paths that print errors if the completed transfer is not found in whatever internal bookkeeping the provider has. Just like assetapi had, before it knew they were aborted.

But do we really want another signal either to IAssetTransfer? I'll give it a look at work later.


juj commented May 25, 2012

Using an immediate path for halting the transfer sounds nicer than letting the transfer to finish to completion and using a boolean to track and kill it.

This does not have to be a signal, but can also be implemented via a virtual IAssetTransfer::Abort() call, which can call IAssetTransfer::provider->AbortTransfer(this); to route the abort handling to its own IAssetProvider object.

jonnenauha added some commits May 25, 2012

@jonnenauha jonnenauha AssetAPI: Refactor IAssetTransfer::Abort() to be stateless. Now by de…
…fault (if not overrided) it will calls its providers IAssetProvider::AbortTrasfer(). This is implemented now in both http and local providers. It will remove the transfer from the internal state (http also aborts the QNetorkReply). This way no caching or further downloading is done. Additionally automatically emits IAssetTransfer::Failed('Transfer aborted');.
@jonnenauha jonnenauha Typo fixes to prints and docs. ad4e18f

jonnenauha commented May 25, 2012

There we go, if you'd have time to review that before the next rex release id appreciate it. Thanks.

@juj juj merged commit fc30f57 into realXtend:tundra2 May 25, 2012


juj commented May 25, 2012

Looks good and tested to function well.

This functionality is not necessarily the strictly preferred behavior for all use cases and with the initial implementation we remained indifferent about the policy to use. This implementation is structured well to allow a configuration to select the policy on disconnect (keep assets in memory vs clear all assets from memory), that can be implemented in the future if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment