photo

Surface enabled default

Recommended Posts

Hello,

We are in the process of migrating from 2.6 to 2.9 and one thing that I've noticed is that if I create a static mesh object from C++ code and add a surface to it (addEmptySurface) this comes as enabled = 0 by default, while previously was 1. This also seems to happen for mesh clusters. Can you confirm that this is the case and I am not missing some aspects? I didn't found this behaviour documented in migration notes in docs.

Kind Regards,

Adrian L.

VStep

Share this post


Link to post

 Hi Adrian,

Thanks for the pointing it out!

More likely it was made intentionally, but we forget to mention this in the documentation. I will double-check this.

Share this post


Link to post

Btw, could you please provide a code snippets that started to work in a different way after update?

Share this post


Link to post

Hi,

For example:

Unigine::ObjectMeshClusterPtr engCluster;

engCluster = Unigine::ObjectMeshCluster::create(nullptr);

engCluster->setMesh(mesh);

After setMesh call I can see by debugging that Object::changed_mesh is called and the code enters in the 

if (id != -1)
        {
            s = surfaces[id];
        } else

where it takes the newly resized surfaces_new which is all filled with zero I guess, so the enabled is also zero. This, from what I can tell, used to be 1 in 2.5 version.

As for ObjectMeshStatic, surface are indeed enabled when calling it by providing a mesh:

Unigine::ObjectMeshStatic::create(mesh);

But by calling:

staticMesh = Unigine::ObjectMeshStatic::create(nullptr);

staticMesh->addEmptySurface(nullptr, vertsNo, indsNo);

and filling the data, will add the surface with enabled as zero (which again was 1 in 2.5 version).

Kind Regards,

Adrian L.

Share this post


Link to post

Thanks for the additional code samples.

Actually, that was a bug in 2.6: enabled flag contained some garbage:

image.png

Right now it's filled with zero. Which value do you prefer that enabled flag should get after adding new surfaces for the mesh? Maybe we can fix this behavior in the upcoming 2.10 update.

Thanks!

Share this post


Link to post

Hello,

It doesn't matter for me what value it is by default, whatever makes sense for engine/other applications. I am currently making sure to enable/disable the surface after initiating it. Thank you for looking into this.

Kind Regards,

Adrian

Share this post


Link to post

Hello,

Related to this (side effect of the fix) I've also noticed that if I just call:

mesh->setMaxVisibleDistance(some distance, surface index)

so I can just have a mesh fade out at a certain distance, and don't also setup the min visible distance this remains zero now (since the surface data doesn't have garbage into it), as I approach the mesh its surfaces start to disappear.

So now I also need to call:

mesh->setMinVisibleDistance(-INFINITY, surface index)

Maybe -inf should be default for min visible distance?

Regards,

Adrian

Share this post


Link to post

That's interesting case, thanks!

We will see how can we fix this.

Share this post


Link to post

Ok, another side effect:

If am using the above code (with addEmptySurface) sometimes, for first surface, I have the parent surface as zero. Shouldn't this be -1 as default? 

So then in RenderRenderer the following code:

					// asymmetry bounds
					else
					{
						// minimum bounds
						while (min_parent--)
						{
							if (min_surface_id != -1)
							{
								min_surface_id = object->getParent(min_surface_id);
								if (min_surface_id == -1)
									node = object;
							}
							else
							{
								if (node->getParent() == nullptr)
								{
									if (node->getPossessor())
										node->getPossessor();
									if (node->getParent() == nullptr)
										break;
								}
								node = node->getParent();
							}
						}

cycles with min_parent negatively for very long loops, probably resetting the int value back to zero due to overflow.

So after addEmptySurface I have to call:

						if(staticMesh->getParent(surfIdx) == surfIdx) // Usually for S == 0
						{
							staticMesh->setParent(-1, surfIdx);
						}

Can you also verify this?

Kind Regard,

Adrian

Share this post


Link to post

Sure thing! Thanks for the additional information.

 

Share this post


Link to post

Probably also related to this allocation data being all zero, my ObjectMeshCluster is no longer showing any data.

The cluster, previously (in 2.5) working, is created from code with something like:

engCluster = Unigine::ObjectMeshCluster::create(nullptr);

engCluster->setMesh(engineMesh);

Unigine::Vector<Unigine::Math::Mat4> insts;
insts.resize(static_cast<int>(instancesNo));
  
for(size_t I=0; I<instancesNo; I++)
{
	// Fill instances data with something like:                               
  	insts[idx].setRotateZ(orientation);
   	insts[idx].setColumn3(3, Unigine::Math::dvec3(pos.x, pos.y, pos.z));                                   
}
                               
engCluster->createMeshes(insts);

for(int S=0; S<engCluster->getNumSurfaces(); S++)
{
	// Since most of the surface data is now zeroed upon release I have to:
  
 	engCluster->setEnabled(1, S);
  	engCluster->setParent(-1, S);
  
  	engCluster->setViewportMask(mask);
  
	engCluster->setMinParent(1, S);
	engCluster->setMaxParent(1, S);  	
  
	engCluster->setMinVisibleDistance(-INFINITY, S);
	engCluster->setMaxVisibleDistance(INFINITY, S);
  	
  	engCluster->setMaterial(mat, S);  	
}
 

Any idea what can go wrong?

I can debug and find out that ObjectMeshCluster::create_spatial is called (at createMeshes) and creates some nodes. Then update_culling is called. But no render surfaces are finally rendered for the cluster from what I can figure out. Can you point me in the right direction in the code where to breakpoint and check what may go wrong?

Regards,

Adrian L.

Share this post


Link to post

Hello! 
I checked your code, it works correctly for me - the cluster is displayed.

there are some tips:

1. check if  existing:
-- engineMesh
-- mat
-- instancesNo > 0
-- mask correct with viewport.

2.

engCluster->setViewportMask(mask, S);

my code 
 

engCluster = Unigine::ObjectMeshCluster::create(nullptr);

	mesh = Unigine::Mesh::create("material_ball_2.mesh"); // my mesh
	MaterialPtr mat = Materials::get()->findMaterial("material_ball"); //my material

	engCluster->setMesh(mesh);
	
	int instancesNo = 10; //num instances

	Unigine::Vector<Unigine::Math::Mat4> insts;
	insts.resize(static_cast<int>(instancesNo));

	for (int I = 0; I<instancesNo; I++)
	{
		// Fill instances data with something like:                               
		insts[I].setTranslate(Unigine::Math::dvec3(I, I, 0)); // ?? just diagonal
	}

	engCluster->createMeshes(insts);

	for (int S = 0; S<engCluster->getNumSurfaces(); S++)
	{
		// Since most of the surface data is now zeroed upon release I have to:

		engCluster->setEnabled(1, S);
		engCluster->setParent(-1, S);

		// engCluster->setViewportMask(~0, S);  // set full vieport mask...  or...  it works for me without this line too.

		engCluster->setMinParent(1, S);
		engCluster->setMaxParent(1, S);

		engCluster->setMinVisibleDistance(-INFINITY, S);
		engCluster->setMaxVisibleDistance(INFINITY, S);

		engCluster->setMaterial(mat, S);
	}

 

Capture.PNG

Share this post


Link to post

Hi and thank you for looking into this.

In the end the error was quite stupid (I should have looked better). Seems that starting with Visual Studio 2017, somehow math.h was included last and INFINITY was there defined as

#define INFINITY   ((float)(_HUGE_ENUF * _HUGE_ENUF))

which is quite different than Unigine INFINITY. I've now used UNIGINE_INFINITY and all is fine.

Btw, this is part of Unigine source code is not very safe in general (MathLib.h):

// this is an internal include file
// so we can redefine common constants here safely, for brevity in our own sources
#ifdef INFINITY
#undef INFINITY
#endif

...

Kind Regards,

Adrian

Share this post


Link to post
Quote

Btw, this is part of Unigine source code is not very safe in general (MathLib.h):

Agree, we will also take a look into this (but more likely after 2.10 release).

Thanks for the update!

Share this post


Link to post

I think I found (one of) the cause(s) for all these issues: when adding a new surface to an object you call Object::changed_mesh which does the following:

	Vector<Surface> surfaces_new;
	surfaces_new.resize(num_surfaces);

	for (int i = 0; i < num_surfaces; i++)
	{
		String name = getSurfaceName(i);

		Surface &s = surfaces_new[i];

		if (id != -1)
		{
			s = surfaces[id];
		} else
		{
             ....

which is completely wrong if you don't have uniquely named surfaces (my case, where all the names where "", ie blank strings). So you get all new surfaces filled with zero datta (the newly allocated one) and then subsequently when the first surface data (again zero). 

You probably should have something like:

int id = name.size() ? surfaces_name.findIndex(name) : -1;

when searching for id.

Anyway, it is completely unsafe to search by name of the surface as this is not guaranteed as unique / not blank at setSurfaceName !!! This was not the case for 2.5 version.

In the case of the object cluster, you got visible now zero which is never touched for it and the clusters are nevere rendered.

Regards,

Adrian

Btw, it is not very nice (from optimization point of view) that for each set (min/max distance, fade distances) you call update_lods() internally. Maybe you should have an startUpdateObject(), ..., endUpdateObject(), asserts if any set is called outside, assert if any other calls (like render) are done inside etc.

Share this post


Link to post

Yep, more likely that's the one of the cases. We will for sure take a closer look in this code.

Thanks!

Share this post


Link to post
Posted (edited)

Hello,

I want to check about the status of this, since I've checked 2.10 and 2.11 source code and cannot find any fixed related to this.

I am especially interested in void Object::changed_mesh() which doesn't take previous surfaces data if they are not named:

        int id = surfaces_name.findIndex(name);

        Surface &s = surfaces_new;

        if (id != -1)
        {
            s = surfaces[id];
        } else
        {
            // create surfaces

It seems to me that if you add surfaces from code, don't give them names, then at each new surface, you lost what you've previously setup for surfaces (as findIndex returns -1 for all previous ones).

Do you have a solution for this? Or is (uniquely) naming the surface mandatory?

Currently I have the following "fix" which works for me since I only append surfaces:

        int id;
        if(name.size() == 0) // Unnamed surface.
        {
            // Use the same index. We assume we add to the end...
            // But if this is a new surface, we must fill it with default data.
            if(i >= begin)
                id = -1;
            else
                id = i;
        }
        else
            id = surfaces_name.findIndex(name);
        // Old code as just:
        //int id = surfaces_name.findIndex(name);

Regards,

Adrian L.

Edited by adrian.licuriceanu

Share this post


Link to post

Hi Adrian,

We've fixed couple of bugs (with INFINITY define and overall garbage that comes from constructor), but it looks like not all of them. If you can use workaround with non-empty name - it would be just great.

We will check what else we can do here after 2.11 release.

Thanks!

Share this post


Link to post

Hi Silent,

Thank you, looking forward for the future fixes.

Regards,

Adrian

Share this post


Link to post
Posted (edited)

Hi, I can see this (adding unnamed surfaces) fixed in 2.12, thanks. Will test if there are issues on our end but the fix seems about right.

Edited by adrian.licuriceanu
Message not clear enough.

Share this post


Link to post