PDA

View Full Version : Should I provide interleaving to end users?



DarioCiao!
08-11-2014, 04:02 AM
Should I provide Interleaving to end users?
My class actually is like that.


class MeshBuffer: public virtual HardwareBuffer{

public:

/** Default is true, you can set interleaving to false for elements
* that have to be updated separately and frequently. If provided
* name does not exist the program crash. */
virtual void setInterleave( bool Interleave, const String & name) = 0; //remove this?

//...

};


The implementation details are not important, actually for me is easy setting interleaving on/off (just question of computing stride, offset and size and store it somewhere ^^ )

Actually the API is very simple:
"provide 1 array of correct size for each vertex component". (so 1 for normals, 1 for positions etc..)

My main concern is that interleaving could be confusing. Actually a buffer can be updated (whole or in part) but for all vertex components. Allowing interleaving will require to specify HOW an user have to update only certain components, WHEN the buffer update will fail because of wrong components and then users must be aware WHY a buffer update failed. (even if those specification would be simple, those will be extra details for users, and so extra complexity)

On the other hand, I cannot simply "update just what users provide as new data, regardless of vertex layout" <-- I think this is totally a no-no.

Performance should not be a real issue, and anyway data that have to be updated separately could be provided in form of a texture in the vertex/geometry shader (texel fetch). So I see no real reason to allow interleaving, someone disagree why?

Yandersen
08-11-2014, 01:51 PM
I wouldn't recommend to dig into individual component updates. Let the user to add the data in it's native format and say that results will be undefined if the vertex format of given data and the one in the buffer do not match.

DarioCiao!
08-12-2014, 02:28 AM
I finally ended up with this:



#include <cstddef>
#define MyEngineAddVertexComponent(s,c,type) /* details*/

struct Diffuse{
vec3 position;
half3 normal;
short2 uv;
};

VertexDescriptor vd;
vd
.MyEngineAddVertexComponent(Diffuse,position, vec3)
.MyEngineAddVertexComponent(Diffuse,normal, half3)
.MyEngineAddVertexComponent(Diffuse,uv, short2);



note that this way I'm unable to declare a vertex at runtime (ex: loading a mesh with custom format is possible, but I cannot change it from code nor to convert individual components to lower precision types)

Yandersen
08-12-2014, 03:46 AM
If you need shader generation, you need to specify where the array of the attributes starts. If you add more items at runtime, that offset (byte offset) always changes. You need to supply that value to the shader.
From the other hand, using the interleaved data, you need to supply the offset to the desired element in the first structured item, and the size of the structure, and those values remain constant when more items added.

DarioCiao!
08-12-2014, 07:14 AM
The vertex format is immutable in my engine(I create it at run time, but once done I don't change it): once a mesh buffer is created from it, the mesh buffer layout can't be changed (unless you recreate a different mesh buffer and copy data from old while adding also new data).

Anyway I'll definitely follow your suggestion, providing also a pointer to an array/std::vector of "Diffuse" structs (previous sample) this would be much straight forward for users:



//"vd" is vertex descriptor from previous post
MeshBufferPtr mesh = meshFactory->createMeshBuffer(vd);
std::vector<Diffuse> meshData;
//...
mesh->upload<Diffuse>( meshData); //meshBuffer is ready for rendering




Do you mind being mentioned in credits somewhere? API decisions are very important to me.


P.S
I still need a mechanism to create formats also at runtime (this is already finished):



mesh->openWriteAccess(true); //true is "async" => closeAccess returns immediatly
mesh->uploadComponent<float3>("position",positionsArray);
mesh->uploadComponent<half3>("normal",normalsArray);
mesh->uploadComponent<short2>("uv",uvsArray);
mesh->closeAccess(); //error if some array is missing or is wrong lenght


Assume I serialize a mesh from a file someone created with an editor:
I can't do that


DiffuseArray = mesh->download<Diffuse>(); //Diffuse is declared in 3rd party binaries, not in mine


But I can do that:



mesh->openReadAccess();
if(vd.getType("position") == float3)
float3Array = mesh->download<float3>("position"); //crash if type is wrong (need to query it, but there's only a bunch of possible types).

Yandersen
08-13-2014, 03:45 AM
Do you mind being mentioned in credits somewhere?No problem, but I don't really see much contribution from my side, though. :)

I see the half-float x3 used for normals. That data requires 6 bytes of space, while the recommended size should be a multiple of 4. I would recommend to either add a dummy 4th component or switch the format to GL_INT_2_10_10_10_REV.

And I am not fully understand the task you are trying to solve. Is it an attempt to store ALL possible vertex attributes in file and then upload only a requested subset of them for rendering? If so, then the storage file does not need to have arrays of attributes interleaved - that is for sure. Instead, let it have the separate arrays, each of them defining the name and type of the item and the way it should be defined using the glVertexAttrib*Pointer function (ensure the 'normalized' flag is included as well as 'integer' to remove confusion between glVertexAttribPointer and glVertexAttribIPointer). But for rendering purposes, the data should be pulled and mixed. The size of the vertex structure will be the summ of the sizes of all individual attributes it is composed of. I would recommend to specify a palette of possible vertex structures, choosing the desired one the user gets a single {VAO+VBO} item in return. So the extraction function creates a VBO, places the extracted and interleaved data into it, creates a VAO and maps the attributes mixed inside the VBO. One function for every possible vertex type. Or you can make a universal function with enumerants as arguments where the user can flag each attribute it wants: GLM_POSITION|GLM_NORMAL|GLM_TEX_COORD|GLM_COLOR|.. .

DarioCiao!
08-13-2014, 05:40 AM
The task I'm trying to solve is to make a most possible easy to use API (well actually 3 lines of code compared to 10 lines of OGRE so I don't think there's room for much improvement), there's nothing in the implementation that is a particular problem.

Your suggestion of using a native struct is good and make things even easier for final users, but that is not enough to allow full control on vertices (even if that was not the point of the topic I appreciated it very much). Certain API features are limited by specific problems: passing native structs requires the struct to be declared in binaries and that is allowed only at build time, so If I want the ability to take an arbitrary mesh, deserialize and edit at runtime, I can't rely on native POD structs but I need (as you pointed out now) an enumeration: wich is however showed in my last snippet of code (float3 is a POD type and also VertexElement::float3 is an enum).

Going back to original issue: data needs mostly to be interleaved for rendering performance, in few cases data is needed with some components NOT interleaved (for example updating only position of particles). So the question was about "allowing users to specify wich data interleave and wich not", but in the end it seems just good idea to interleave everything and say goodbye.

(the 4th dummy component is added by the engine in half3, but I forgot is to add support for the 2_10_10_10 format I should add it)

So in short:


Allow to specify custom vertex format at run time(YES, done)
Ability to serialize/deserialize and edit mesh at run time, without ever touching void* or char* (YES, done)
Short cut to save time declaring a vertex format at build time, still allowing to serialize and deserialize it at run time (YES, WIP this is your idea;) )
Allow to specify interleaving or not to end users (NO)
Add the packed normal type to supported types (TODO)

Yandersen
08-13-2014, 12:20 PM
The task I'm trying to solve is to make a most possible easy to use API...API for what exactly? Is it a model converter pulling the individual data from one file format and storing it in the other format? Just guessing, sry. :)


passing native structs requires the struct to be declared in binaries and that is allowed only at build timeActually, the structure does not need to be declared at all. Using glGet* on VAO properties defines everything in full details. Every time the new attribute is being added, you pull all the data from all enabled vertex attributes into individual arrays, then mix them again along with the given attribute array, so the "structure" grows in size along with the VBO. Might not be the fastest way, but straightforward and robust. ;)

DarioCiao!
08-13-2014, 05:21 PM
API for what exactly? Is it a model converter pulling the individual data from one file format and storing it in the other format? Just guessing, sry. :)
a rendering engine (school project)



Actually, the structure does not need to be declared at all. Using glGet* on VAO properties defines everything in full details. Every time the new attribute is being added, you pull all the data from all enabled vertex attributes into individual arrays, then mix them again along with the given attribute array, so the "structure" grows in size along with the VBO. Might not be the fastest way, but straightforward and robust. ;)

That's not robust at all. If you allow Vertex Layout changethat would break all pre existing shaders and any code that operate on that mesh:

API design is also about prevent users from doing bad things (unless that have so many downsides that makes that worthless)

P.S. In my original code the structure is not declared at all (just see previous code snippets), declaring a struct was just a shortcut coming from your comment:


I wouldn't recommend to dig into individual component updates. Let the user to add the data in it's native format and say that results will be undefined if the vertex format of given data and the one in the buffer do not match.