-
-
Notifications
You must be signed in to change notification settings - Fork 291
Revised geometry, child / parent APIs; ditto object boxing #658
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
Conversation
…ather than upvalues
…an codebase, for more general consistency with other renderer code In general, moved #include of CoronaGraphics.h out of other headers Sometimes a forward declaration was enough Shader's DrawState now uses a const CoronaShaderDrawParams* (and points to a static dummy when none is assigned) The geometry extension attribute type is stored as a U16 (rather than an enum), and cast back in the cpp Similary, IsFloat()'s definition is moved to cpp-side A program's extra uniforms are now retrieved through a reference rather than on the stack, the type being defined on the cpp-side Command buffers' AddCommand() now takes a pointer, and stores the result in a LightPtrArray (owned by the renderer) A program's shell transform cleanup is stored in non-typedef's form (equality with the typedef is asserted on an is-same trait State blocks and commands are now stored in PtrArray values, themselves held in a forward-declared structure defined on the cpp-side The pending commands are now a LightPtrArray Whoops, scratch that :D There is now a synced counter, rather than a command count, and values after the synced counter are forwarded to the other buffer (then the counter is updated) Removed CustomOp from Renderer (wasn't used any more) Bug found in Array's PadToSize() method, where old data wasn't copied over and then removed on growing the buffer
…e and Group subtypes), converting it from a struct to class with private members Using debug new on Windows (in debug mode, of course) Fixed leak in ShaderData
Redid some stuff with std vectors and strings to use Array and String types, in a couple list functions (need to test compiles and runs) Added new files to projects
… the library's PushImage() calls CoronaGraphics.h includes revised for Apple projects
32-bit version used by format extension lists
…es, now using a hash to enforce validity Brought in wyhash (right now just inlined in CoronaObjects.cpp) for updates to same and an RNG Revised CoronaGroupObjectGetParent() and *GetChild() revised to accept stack object as out parameter, in keeping with no-storage policy A couple state blocks functions revised to early-out if list empty (to avoid setting up box scope)
Objects all passed by value No CoronaStackObject, since now superfluous Types are lazily gathered No actual box object, only reified in array And of course, all of it crashes anyhow :(
Now using hijacked pointers rather than pushed-by-value structs These contain a xor'd-with-hash version of the value in a scope's array (quick to check and robust against absence) Using wyhash, now in external Rename of ObjectBoxList to ObjectHandle Slimming down of previous details External refs and engine-supplied ones use same array, but the former set a flag
Rather than return a handle, often the same one, and also due to the odd reference thing, just reserve a slot with CoronaObjectGetAvailableSlot() Same is now an out value in GetChild() and GetParent() These slots are now encoded in a bit of the box pointer, before mixed with the hash Some redesign around the above Added Clear() method and an RAII type that can invoke it This is for the possiblility of customizing some method scopes (restrict to if clause versus both before and after calls, say) On that side of things, made object params booleans full ints and reordered them (needed to fix offsetof() in GenericParams code too
Now have declarations and also deferred Add()'ing Using optional separated scopes for display objects, only loading them if their before or after is present (cuts down on spurious hashes) Those use the ClearIf RAII objects (initialized with a separateScopes member) OnMessage handlers may also opt to retain the current scope Switched some allStored checks to asserts (only really needs checking in open situations)
…ta) and replaced with one to set the geometry writer, a callback Same in Main.cpp and Rtt_ApplePlatform.mm Added writer list to renderer, which defaults to a memcpy-based one Can supply others (tested with text objects; should be in supplementary diff in PR) Minor changes in CoronaShaderDrawParams's before and after functions to allow injection of custom writers Vertex copying in renderer logic slightly adjusted for writer list approach
…ese as well as wyhash) Reworked how TypeNode was initialized since static member being shared among derived types was broken; seemed to work with Visual Studio build but maybe just not stress-tested enough (still untested) Returning nil if extended vertex data not present rather than creating it just to read it (still untested; related to _properties)
Rtt_STATIC_ASSERT() likewise Added Rtt_ObjectHandle to various platforms (and removed box list from Apple ones) Added a few that I missed in Linux projects (I assume these are IDE-related)
@ggcrunchy @Shchvova Hello, I am attempting to build for Linux. The Cmakelists.txt list on the main Master branch includes code only found on the Experimental Branch: [CoronaObjects.cpp]. So the Master branch for Linux wont compile, would I be better off working with the Experimental Branch. Maybe we have a merge back with Master ? I am new to this code base so I would be guessing at this stage. Cheers |
I believe this was the last blocker for experimental.
First things first, I removed experimental's
CoronaGeometryCopyData()
andCoronaGeometryGetMappingFromRenderData()
from theCoronaGraphics
API. In their place isCoronaGeometrySetComponentWriter()
, which takes a writer callback. The writer is run at a slightly different time (while batching vertices) but is able to achieve the same thing as those two APIs. There is now always a writer in effect: the default simply copies vertex data from incoming display objects into the output stream, i.e. the current behavior. Writers only produce the outgoing data, so have no effect on batching itself.These are assigned through the
Shader::Draw()
methods, which now accept zero or more writers. (Writers are run in sequence on the outgoing vertex stream data.)In addition to streamlining the two geometry APIs, which were already a bit too general for their narrow use case (see
CoronaGraphics.h
for docs), the writers arose from a couple earlier observations:When doing the emitter mapping PR, I saw that particles, which can be quite numerous, mostly repeat themselves and only need part of their vertex data. This can burn a lot of RAM at scale. The groundwork should now be there to whittle this down.
Text objects are doing a lot reallocation to deal with offsets, but a writer could stitch that in as vertices are merged. I did in fact use this as a test case (those changes are... somewhere; could scrounge them up on request).
Some other possiblities:
Translate()
method, I believe, that might be amenable to this.(These being ways to use these features in Solar proper, rather than API-using plugins with out-of-the-ordinary shader needs, say.)
Many APIs in experimental use an handle (an "object box") to native Solar objects. What I had been doing was keeping a list of the boxes, but the "get child" and "get parent" routines have no deterministic limit: a display object might have a great-great-great...-grandparent, for instance, or a group could have hundreds of children. The list could consume a lot of memory, on the one hand, as well as be difficult to search and validate.
There is now something like a call stack, and a
CoronaObjectGetAvailableSlot()
API that will reserve a spot in the current scope (there is a limit here, but it's more than 2, which is enough to make placeholders for a parent and child, respectively). The slot is represented by an "Any" object:CoronaObjectGetParent()
andCoronaGroupObjectGetChild()
both now expect one of these and temporarily box the appropriate object into it, after which it may be used like any other. Subsequent calls will evict the current object from the box, but this also avoids the need for an unbounded list.The handles are a hashed representation of their position in the scope, with a flag for the "get available" ones as well. The hashing makes it easy to invalidate objects when they go out of scope and also avoids accidental use, say versus the handles just being integer indices.
wyhash was used to perform the hashing. Although I'd been considering just going with one or the other, this seemed distinct enough from the FNV-1a-based
GetHash32()
andGetHash64()
methods I'd also recently added forRtt_String
, so they're both present now. (Neither should have much of a footprint.)Some of the data structures and macros in
CoronaObjects
were tidied up to account for all this.Handle types are now declared via macro (
CORONA_DECLARE_PUBLIC_TYPE()
), and "Any" included among them.The API files on Windows (
Main.cpp
) and Apple (ApplePlatform.mm
) have been updated to reflect the above changes.ObjectBoxList.h
was removed and the replacement functionality is in theObjectHandle.*
files andwyhash.h
, platform projects being updated accordingly. Looks like I forgot a file or two in the Linux projects before, too. :)Somebody in earlier testing had pointed out that he was getting errors when reading
_properties
on display objects. This was from format extension lists, and I had just been instantiating them on demand to account for it. This is pretty wasteful, since no code in the wild is yet using these, so the corresponding properties now returnnil
when a list has not been registered, and_properties
seems to be happy.