Klaim wrote:Sorry for the delay, here is my review of the document:
1. I think that a sequence diagram of a typical multi-threaded loading of resource would help clarify how the resource loading implementation communicate with Ogre (even if it's almost clear in the current document).
2. "The general rule is that modifications to a resource may only be made upon the main thread (that is, the one driving the Ogre rendering process), with one exception: the loading state."
This sentence might be ambiguous. Do you mean: "The general rule is that once ready, modifications to a resource may only be made by the thread driving Ogre rendering process (which should always be the same), with one exception: the loading state, which is the communication device between the Ogre thread and the resource loader."
Best clarification would be "with the exception that the resource loader may modify it from another thread while it is in the LOADING state"
3. "Once startedLoadingResource function has been called, " -> +"by the loading thread"?
By the resource system on the main thread
startedLoadingResource is the function you call to tell Ogre that you're about to start doing background stuff on another thread.
You call aboutToLoadResource to notify it that you're done with that and going to finalize up your work on the main thread
You finish everything off by calling loadedResource.
All of these are state transitions, they must execute on the main thread.
4. "resource; up to that point, the Resource object is confined such that the only operations which may be performed on it from other threads are queries as to its’ state and reference count modifications."
What are "other threads"? Ogre threads? That part is a bit confusing (to me at least), certainly because of the lack of clarification of the role of the different actors in the whole system.
The ONLY things which are safe to do from a background thread are manipulate the reference count (through IntrusivePtr<T>) and read the state of the object (the Resource::LoadState enum), except for if you are the resource loader and are doing a background load from another thread
5. A list of the different classes of the new resource system, with a short description, might help too.
Resource and ResourceLoader. Add in ResourceCodec and ScriptLoader if you want to hook into the "standard" loaders.
6. "If you’re implementing your own resource system, you’ll need to consult the documentation to determine in what phase(s) to make what calls."
Which documentation exactly? The API documentation of the specific resource types?
The Doxygen comments for Resource and ResourceLoader
7. The explainations of how to setup your own resource system should be gathered in a separate dedicated section.
Agreed
8. "For these cases, the loaders that the default resource system uses are exposed as callbacks. [NB. At this time this isn’t implemented. The existing codecs are largely being stripped out and will be reappearing in the next phase]"
No
9. "If you’re not interested in writing a new resource system, then the changes for you are much simpler."
Here by "the changes", you mean "upgrading your Ogre user code to this new system". Am I correct?
Yes
10. Titles to the different sections would help getting to the part interesting to the different users, like the last part which is what you have to know for migrating to the new system if I'm correct.
Indeed. It was an early draft
I hope to try the new resource system as soon as it's stabilized enough that you, Owen, think users can begin to experiment with it.
Experimenting it is definitely possible now, but expect bugs and little areas to not be working. Compositors are definitely busted, for example, as are overlays.
