Re: Few serious questions for Ogre-Next, moving Stunt Rally
Okay, any chance for some answers or an update or hints? It's been 2 weeks, and it's weekend ![]()
Support and community hang-out spot for Ogre3D
https://forums.ogre3d.org/
Okay, any chance for some answers or an update or hints? It's been 2 weeks, and it's weekend ![]()
Crystal Hammer wrote: Sun May 21, 2023 10:03 amMore questions:
- Does Pbs not even use and pass vertex colors to shader? If not is it much to do to add? Or maybe would it be easier to use another UV set per vertex and new code in shader to use it to any purpose.
This has been answered in various posts: Everyone has their own notion of what vertex colours are meant to do in PBS (i.e. should they carry "colour" data? should it be a multiply? an addition? affect albedo? specular? something else?); thus PBS doesn't do anything with vertex colours.
You can however use custom shader pieces to define the in vec4 colour input (use inVs_colour macro in the code so it's compatible with all languages) and pass it from vertex to pixel shader.
- If I'm gonna need few extra parameters for car paint in my own datablock, and some other parameters for other use case, then should I create different datablocks and cpp code, or even other Hlms and register that? Or can I just use one own Pbs datablock and put all my parameters in it, regardless of use. Is this very wasteful?
HlmsPbsDatablock::mUserValue let's you register custom values (which you can set via setUserValue from C++).
From shader code you can access them via material.userValue[] defined in 500.Structs_piece_vs_piece_ps.any.
If you need more variables, then you'll have to fully override HlmsPbsDatablock (and HlmsPbs obviously, so that you can create your datablocks by overriding HlmsPbs::createDatablockImpl) and override HlmsPbsDatablock::uploadToConstBuffer which is where data is uploaded from C++ to GPU material shader buffers.
Okay. I actually started with my own Datablock2 now, and I have my HlmsPbs2 (registered instead of HlmsPbs).
Can I create both HlmsPbsDatablock2 or HlmsPbsDatablock in HlmsPbs2::createDatablockImpl? Like so:
Code: Select all
HlmsDatablock* HlmsPbs2::createDatablockImpl(
IdString name,
const HlmsMacroblock *macro,
const HlmsBlendblock *blend,
const HlmsParamVec ¶ms )
{
String val;
if( Hlms::findParamInVec( params, "paint", val ) )
{
//LogO("paint db2 new");
//LogO(name.getFriendlyText()); // hash-
//for (auto p : params)
// LogO(p.second);
return OGRE_NEW HlmsPbsDatablock2( name, this, macro, blend, params );
}else
return OGRE_NEW HlmsPbsDatablock( name, this, macro, blend, params );
}And what do I put inside uploadToConstBuffer first, is this correct?
Code: Select all
void HlmsPbsDatablock2::uploadToConstBuffer( char *dstPtr, uint8 dirtyFlags )
{
char* orgPtr = dstPtr;
Ogre::HlmsPbsDatablock::uploadToConstBuffer( dstPtr, dirtyFlags );
dstPtr = orgPtr + Ogre::HlmsPbsDatablock::MaterialSizeInGpu;
Vector4 paintClr(1,0,1,0);
float4 par4 = paintClr;
memcpy( dstPtr, &par4, sizeof(par4) );
dstPtr += sizeof(par4);
}I almost have it working, seems that I just don't fill those extra paint float4 values in shader somehow.
Looks almost right.
The only thing I just noticed is that HlmsPbs::HlmsPbs hardcodes the buffer size passed to ConstBufferPool which is a problem for customization. HlmsUnlit uses constructor overload to fix that problem by providing uint32 constBufferSize.
Although that's an OgreNext bug (HlmsPbs is missing a constructor overload) I suggest you just use mUserValue:
Code: Select all
void HlmsPbsDatablock2::uploadToConstBuffer( char *dstPtr, uint8 dirtyFlags )
{
mUserValue[0][0] = paintClr.x;
mUserValue[0][1] = paintClr.y; //...
Ogre::HlmsPbsDatablock::uploadToConstBuffer( dstPtr, dirtyFlags );
}Since those values are there for your use.
Hmm, doesn't work, or I can't get it to work.
I barely fit into float [3][4], it's exactly 12 floats I need, but okay I tried and now it seems all are 0.0?
So in my previous commit I did it all the 1st way. And then in my next commit I did it with mUserValues.
Either way I don't seem to have any of those values in shader.
And now if I comment out this line 803 in Media/Hlms/Pbs/Any/Main/800.PixelShader_piece_ps.any:
Code: Select all
if (material.userValue[0].w > _h( 0.0 )) // all blackthen I see mostly all black.
Same as with this test there
Code: Select all
pixelData.diffuse.xyz = material.userValue[0].xyz;
pixelData.specular.xyz = material.userValue[1].xyz;Do I need a custom piece? Because I just put my code inside Pbs/Any/Main/800.PixelShader_piece_ps.any etc.
Stepping on a debugger in HlmsPbsDatablock::uploadToConstBuffer shows paintClr & paintMul have uninitialized variables. Don't do that.
Second, when you do:
Code: Select all
db->paintClr[i] = Vector4(p.r, p.g, p.b, 1.f);You're never calling scheduleConstBufferUpdate()
I just spotted that HlmsPbsDatablock::setUserValue never calls scheduleConstBufferUpdate(), that's a bug I'll fix.
Edit: Fixed.
Third, there is an assert triggering:
Code: Select all
assert( (size_t)( passBufferPtr - startupPtr ) * 4u == mapSize );It appears that assert triggers because you advance the pointer in HlmsPbsTerraShadows::preparePassBuffer but you forgot to update HlmsPbsTerraShadows::getPassBufferSize
I told you multiple times. Run in Debug mode.
Do I need a custom piece? Because I just put my code inside Pbs/Any/Main/800.PixelShader_piece_ps.any etc.
You don't "need to" but it is recommended to eventually use pieces, because that makes upgrading less painful.
If you modify the source code directly, upgrading to a newer version of OgreNext means you have to transplant all your changes.
Code: Select all
db->paintClr[i] = Vector4(p.r, p.g, p.b, 1.f);You're never calling scheduleConstBufferUpdate()
That's why I moved db->setFresnel(.. at end so it calls scheduleConstBufferUpdate().
Allright thanks. Fixed both. It almost worked. It works when I don't clone:
Code: Select all
#if 1 // no clone
db = pDb;
#else
db = static_cast<HlmsPbsDatablock2*>(
pDb->clone( "CarBody" + sCarI + toStr(id) ) );
#endifSo, I see that I probably should have in HlmsPbsDatablock2, a cloneImpl, what should I put in it?
It gets cloneImpl( HlmsDatablock *datablock ) not HlmsPbsDatablock2 as argument, I guess I should upcast?
Yeah true I forgot Debug, But it's easy to say debug. I got few issues with it:
But surely I need to debug more.
I'm not sure if this is stable yet, I saw few odd crashes, also some freezes at quit, weird, and again that freaking driver crash or something that I need to hard reset PC after.
Well, all that makes me say: coding with Ogre-Next (vs old Ogre) for me feels closer to coding a kernel driver, not just my game/editor. This part is really not a good surprise.
Okay let me summarize my remaining issues since it's weekend ![]()
Problem 1: is with shadowed terrain in minimap RTT, when I create them as manual updated. And they're okay when auto-updated.
To test this:
src/editor/Render2tex.cpp
Code: Select all
//#define MANUAL_RTT_UPD // todo: terrain shadowed..?Problem 2: after continuous road editing Fps drops, seems it is leaking something.
I'm almost sure I destroy it properly and create again, in this case every frame.
So first destroy: SceneNode, then Item, and Meshes last, code is around line 420 in SplineRoad::DestroySeg in src/road/Road_Mesh.cpp
To test this:
Road update skip to 0. Now editing road will update instantly, each frame.Problem 3: I noticed that GPU RAM use value gets big after longer time when window is inactive.
So e.g. I:
float AppGui::GetGPUmem() in src/common/AppGui_Fps.cppWhy on earth can't I set new values in .material when I override something that was set in base material?
If I do this
Code: Select all
hlms wall_base pbs
{
diffuse 1 0 1
}
hlms red_wall pbs : wall_base
{
diffuse 1 0 0
}now the red_wall won't be red because it can't set new value to that diffuse from wall_base ![]()
This way I can't set any default values and change them later when I inherit if needed. So if I had 1 base material and 20 from it, then I need to specify all values in those 20 materials, not e.g. 5 which changed it. Or have to create a new base materials etc.
It's an annoying bug for me, was this on purpose?
Try abstract hlms wall_base pbs; which causes the inheritance to be offloaded to the script parser (since HlmsPbsDatablock doesn't implement the base stuff functionality; although maybe it's possible to implement it now that HlmsDatablock has been supporting the clone() routine for a few years now).
Nope, doesn't work.
I took a look at that problem. Fixed.
Since it's not a very popular feature (as most people have moved to JSON materials), this bug went under for a long time.
Cheers
Great, thanks, it works.
Any chance for my 3 bigger issues from here?
Something is wrong in Terra, for my case when I don't have roughness nor metalness maps, only values. But setting these as values doesn't do anything.
From Samples/Media/Hlms/Terra/Any/800.PixelShader_piece_ps.any
Code: Select all
@undefpiece( SampleRoughnessMap )
@piece( SampleRoughnessMap )
/// ROUGHNESS MAP
@foreach( 4, n )
@property( roughness_map@n )
midf roughness@n = SampleRoughness@n@property( detail_triplanar_roughness )Triplanar@end ( textureMaps@value( roughness_map@n_idx ),
....
@else
midf roughness@n = _h( 0 ); // <- 1.0 here is better
@end
@end
pixelData.perceptualRoughness = (roughness0 * detailWeights.x * midf_c( material.roughness.x ) +
roughness1 * detailWeights.y * midf_c( material.roughness.y )) +
....So in my case I don't SampleRoughness and it goes into midf roughness@n = _h( 0 );
which then sets roughness0..3 to 0 and later material.roughness doesn't matter, roughness will be 0 anyway.
I fixed this with midf roughness@n = _h( 1.0 );
But for metalness it doesn't exactly work, Same case midf metalness@n = _h( 0 ); does same issue. But if I change it to 1.0 I also get weird dark spots. It's that last line I think.
What is this supposed to do?
Code: Select all
pixelData.F0 = lerp( midf3_c( 0.03f, 0.03f, 0.03f ), // ?
pixelData.diffuse.xyz * _h( 3.14159f ), metalness );
pixelData.diffuse.xyz = pixelData.diffuse.xyz - pixelData.diffuse.xyz * metalness;And why is specular always full white? And not e.g. set from datablock specular color?
pixelData.specular.xyz = midf3_c( 1.0f, 1.0f, 1.0f );
Also why does terrain even have fresnel / reflection? Most natural terrains are rough and don't reflect anything, unless wet right? Sure, we do have now metal tarrains in SR3 Space tracks, that look better so, but that's an exception.
Kind of reminds me of that issue I have with setDetailTriplanarNormalEnabled(true); which I guess everybody forgot by now. Maybe it's also due to my Terra setup IDK.
Ah and I use tdb->setBrdf(TerraBrdf::BlinnPhongLegacyMath); because every other looks just worse, not to mention new ones had this fresnel reflection horror.
Anyways, it'd be great if you could just tell me: if you will look into my previous issues someday eventually? I seriously don't like hoping for things to happen here. I'd rather know you won't do something and so I'd have to take care of those myself sooner or later. But some hints would be nice though.
I create meshes like so, from /Samples/2.0/ApiUsage/DynamicGeometry/DynamicGeometryGameState.cpp
which does lots of creating and doing new, I'm filtering it:
Code: Select all
// From: createScene01(
mStaticMesh = createStaticMesh( false );
// From: createStaticMesh(
{
Ogre::MeshPtr mesh = Ogre::MeshManager::getSingleton().createManual(
..
Ogre::SubMesh *subMesh = mesh->createSubMesh();
..
CubeVertices *cubeVertices = reinterpret_cast<CubeVertices *>(
OGRE_MALLOC_SIMD( sizeof( CubeVertices ) * 8, Ogre::MEMCATEGORY_GEOMETRY ) );
..
vertexBuffer = vaoManager->createVertexBuffer(
vertexElements, 8, Ogre::BT_DEFAULT_SHARED, cubeVertices,
..
// From: createIndexBuffer(
{
Ogre::uint16 *cubeIndices = reinterpret_cast<Ogre::uint16 *>(
OGRE_MALLOC_SIMD( sizeof( Ogre::uint16 ) * 3 * 2 * 6, Ogre::MEMCATEGORY_GEOMETRY ) );
..
indexBuffer = vaoManager->createIndexBuffer( Ogre::IndexBufferPacked::IT_16BIT, 3 * 2 * 6,
}
// Back to: createStaticMesh(
..
Ogre::VertexArrayObject *vao =
vaoManager->createVertexArrayObject( vertexBuffers, indexBuffer, Ogre::OT_TRIANGLE_LIST );
..
subMesh->mVao[Ogre::VpNormal].push_back( vao );
}
// Back to: createScene01(
..
Ogre::Item *item = sceneManager->createItem( mStaticMesh, Ogre::SCENE_DYNAMIC );
Ogre::SceneNode *sceneNode = sceneManager->getRootSceneNode( Ogre::SCENE_DYNAMIC )
->createChildSceneNode( Ogre::SCENE_DYNAMIC );
node->attachObject(item);What is now the proper way to destroy it all? ![]()
So that I can create it again and again (after edit, every frame, thousands of times) and not leak memory and drop Fps?
Only thing I see in that Sample in void DynamicGeometryGameState::destroyScene() is
Code: Select all
mStaticMesh.reset();Surely this can't be enough? Why is nothing destroyed or deleted in this Sample, will it all be done automatically on quit?
Do I need to (when recreating this, every frame on edit, rebuild, or on track unload):
Code: Select all
sceneManager->destroySceneNode(sceneNode)
sceneManager->destroyItem(item)
MeshManager::getSingleton().remove(mesh)
OGRE_FREE_SIMD cubeVertices and cubeIndices ?
vaoManager->destroyVertexBuffer(vertexBuffer) ?
vaoManager->destroyIndexBuffer(indexBuffer) ?
vaoManager->destroyVertexArrayObject(vao) ?And in this order (or reversed as from create) or otherwise?
Hi!
OGRE_FREE_SIMD cubeVertices and cubeIndices ?
See the documentation of createVertexBuffer (which points to BufferPacked's constructor):
If keepAsShadow is false, caller is responsible for freeing the data
If keepAsShadow is true, we're responsible for freeing the pointer. We will free the pointer using
OGRE_FREE_SIMD( MEMCATEGORY_GEOMETRY ), in which case the pointer must* have been allocated usingOGRE_MALLOC_SIMD( MEMCATEGORY_GEOMETRY )
The same goes for index buffers.
MeshManager::getSingleton().remove(mesh)
You need to obviously call MeshManager::getSingleton().remove(mesh).
Because you handed the buffers and VertexArrayObject to a mesh, when the mesh is destroyed, it will automatically destroy:
The vertexBuffers (you don't have to call vaoManager->destroyVertexBuffer(vertexBuffer))
The indexBuffer (you don't have to call vaoManager->destroyIndexBuffer(indexBuffer))
The Vao (you don't have to call vaoManager->destroyVertexArrayObject(vao))
Please note that:
Mesh is a smart_ptr. Thus if you have any reference hanging around, it won't be destroyed until all references are unset
Destroy all Items referencing the Mesh before trying to destroy the Mesh (otherwise the nature of smart_ptrs means the Mesh won't be actually destroyed until you do).
Okay thanks for explanation. So it is actually how I did it.
One question though, there was at start this:
Code: Select all
Ogre::MeshPtr mesh = Ogre::MeshManager::getSingleton().createManual(Does it mean I should keep this mesh variable and later do mesh.reset();?
So destroying the above stuff like this?
Code: Select all
sceneManager->destroySceneNode(sceneNode);
sceneManager->destroyItem(item);
MeshManager::getSingleton().remove(mesh);
mesh.reset(); // or before ^ ?Allright I tracked it down, here is how I create:
Code: Select all
MeshPtr mesh = MeshManager::getSingleton().createManual(sMesh, "General");
..
// tangents v2 to v1 to v2 meh-
#ifdef V1tangents
String s1 = sMesh+"v1", s2 = sMesh+"v2";
sd.mesh1 = v1::MeshManager::getSingleton().create(s1, "General");
sd.mesh1->setVertexBufferPolicy( v1::HardwareBuffer::HBU_STATIC, false ); // true may decrease loading time..
sd.mesh1->setIndexBufferPolicy( v1::HardwareBuffer::HBU_STATIC, false );
sd.mesh1->importV2(mesh.get());
MeshManager::getSingleton().remove(mesh); // not needed
mesh.reset();
sd.mesh1->buildTangentVectors(); // todo: slow in ed, 24,30 Fps vs 60 Fps v2 only
sd.mesh = MeshManager::getSingleton().createByImportingV1(
s2, "General", sd.mesh1.get(), false,false,false);
#else
sd.mesh = mesh;
#endifI destroy it like so:
Code: Select all
auto mgr = mSceneMgr;
auto& ms = MeshManager::getSingleton();
auto& m1 = v1::MeshManager::getSingleton();
mgr->destroySceneNode(node);
mgr->destroyItem(item);
if (rd.mesh) ms.remove(rd.mesh);
rd.mesh.reset();
if (rd.mesh1) m1.remove(rd.mesh1);
rd.mesh1.reset();And it doesn't leak with V1tangents not defined (only V2 mesh is created).
It does leak GPU RAM for a mesh, when I define V1tangents.
So, am I doing something bad in this block above for V1tangents, that leaks a mesh? Or is there a leak inside buildTangentVectors or createByImportingV1?
Oh I see what's going on!
createByImportingV1() does this on purpose.
Here's the thing:
Given that you generate the meshes yourself (and you could run that again on device_lost, but you'd have to write more code to handle it), I think the sensible solution here is to replace createByImportingV1 with:
Code: Select all
sd.mesh = MeshManager::getSingleton().createManual( name, groupName, nullptr );
sd.mesh->importV1( sd.mesh1, params.halfPos, params.halfTexCoords, params.qTangents, params.halfPose );
I see, interesting. I changed it to:
Code: Select all
sd.mesh = MeshManager::getSingleton().createManual(s2, "General", 0);
sd.mesh->importV1(sd.mesh1.get(), false,false,false);but it has the same issue ![]()
Well this makes this logic unusable for any project that loads levels, destroys parts, edits etc (using v1 still, or needs v1 tangents from code) am I right?
Even this doesn't unload those v1 forever copies?
Code: Select all
MeshManager::getSingleton().destroyAllResourcePools();
v1::MeshManager::getSingleton().destroyAllResourcePools();I know it'd be best not to use this V1tangents and specify proper tangents myself in code, but IDK yet which values for them will be right.
Mmm... maybe my assumption that the problem was in createByImportingV1 is wrong.
I'll check it out tomorrow.
Problem 2: after continuous road editing Fps drops, seems it is leaking something.
I can reproduce a very small gradual FPS loss over time; but it doesn't seem like a huge RAM leak, but rather a small one. This is going to take time.
So in the meantime I'll report the other issues I found:
I noticed in System Monitor that Release was using +20% CPU while Debug was only using 4% (I have 24 threads; 4% CPU is 100% of a full thread). That's when I thought "AHA!".
I get a massive FPS speed up by making Debug use multithreading. Simply find this:
Code: Select all
void GraphicsSystem::chooseSceneManager()
{
#if OGRE_DEBUG_MODE >= OGRE_DEBUG_HIGH
//Debugging multithreaded code is a PITA, disable it.
const size_t numThreads = 1;
#else
//getNumLogicalCores() may return 0 if couldn't detect
const size_t numThreads = std::max<size_t>( 1, PlatformInformation::getNumLogicalCores() );
#endif
And change numThreads to have the same value as Release.
Even though Debug still doesn't reach 60 fps, it now went from unbearable 7fps to a much more decent 40-45fps.
This is likely this (I didn't double check):
Problem 3: I noticed that GPU RAM use value gets big after longer time when window is inactive.
In debug mode minimizing or fully occluding the window triggers the following exception:
An exception has occured: OGRE EXCEPTION(1:InvalidStateException): Mapping the buffer twice within the same frame detected! This is not allowed. in BufferPacked::map at ../../OgreMain/src/Vao/OgreBufferPacked.cpp (line 180)
Just walking up the stack when the exception happens makes the problem obvious:
OgreNext is complaining that buffers are being mapped twice in the same frame (because somewhere inside BaseSystem::update you are issuing rendering commands) but when the window is hidden, renderOneFrame doesn't get called; which means no new frames are generated (thus everything you do is in the "same frame"):
Code: Select all
if( mRenderWindow->isVisible() )
mQuit |= !mRoot->renderOneFrame();Simply getting rid of if( mRenderWindow->isVisible() ) fixes the problem. Of course this means a hidden window will be wasting GPU cycles; but this bug happens because you're doing GPU stuff while the window is visible.
However this shouldn't be much of a problem because MainEntryPoints::mainAppSingleThreaded already does the following:
Code: Select all
if( !renderWindow->isVisible() )
{
// Don't burn CPU cycles unnecessary when we're minimized.
Ogre::Threads::Sleep( 500 );
}
Thus commenting out if( mRenderWindow->isVisible() ) is a quick fix to many of your problems while minimized.
Ideally you should avoid doing GPU stuff while minimized.
I didn't try seeing what happens if I changed it to:
Code: Select all
if( mRenderWindow->isVisible() )
{
BaseSystem::update( timeSinceLast );
mQuit |= !mRoot->renderOneFrame();
}Or any similar variation.
Relevant Documentation section.
You have various of these, which were so annoying I had to comment them out from OgreNext to shut them up.
I noticed one of them happens here:
Code: Select all
texCamera->setOrientation( light->getParentNode()->_getDerivedOrientation() );This means that you likely modified a light's position or direction after SceneManager::updateAllTransforms (e.g. in a listener)
If you can hunt down the code that changes the light and move it so that it happens before SceneManager::updateAllTransforms, then awesome. Note: Data Breakpoints are awesome for hunting changes to variables.
If you can't move it before updateAllTransforms, then issue a light->getParentNode()->_getFullTransformUpdated right after you've modified the light. Note that if you keep moving lights around inside listeners, you might cause incorrect rendering (because OgreNext thought the light was somewhere, but you moved somewhere else, then again, then again). These bugs are rare though.
This code is wrong:
Code: Select all
try
{
indexBuffer = vaoManager->createIndexBuffer(
IndexBufferPacked::IT_16BIT, idxCnt, BT_IMMUTABLE, indices, true );
}
catch (Exception &e)
{
// When keepAsShadow = true, the memory will be freed when the index buffer is destroyed.
// However if for some weird reason there is an exception raised, the memory will
// not be freed, so it is up to us to do so.
OGRE_FREE_SIMD( indexBuffer, MEMCATEGORY_GEOMETRY );
indexBuffer = 0;
throw e;
}
It should be:
Code: Select all
try
{
indexBuffer = vaoManager->createIndexBuffer(
IndexBufferPacked::IT_16BIT, idxCnt, BT_IMMUTABLE, indices, true );
}
catch (Exception &e)
{
// When keepAsShadow = true, the memory will be freed when the index buffer is destroyed.
// However if for some weird reason there is an exception raised, the memory will
// not be freed, so it is up to us to do so.
OGRE_FREE_SIMD( indices, MEMCATEGORY_GEOMETRY );
indices = 0;
throw e;
}
Alternatively you could use FreeOnDestructor which is more user friendly.
The same happens with vertex buffer creation:
Code: Select all
VertexBufferPacked *vertexBuffer = 0;
try
{
vertexBuffer = vaoManager->createVertexBuffer(
vertexElements, vertCnt, partialMesh ? BT_DEFAULT : BT_IMMUTABLE, &v[0], true );
}
catch (Exception &e)
{
// we passed keepAsShadow = true to createVertexBuffer, thus Ogre will free the pointer
// if keepAsShadow = false, YOU need to free the pointer
// OGRE_FREE_SIMD( vertexBuffer, MEMCATEGORY_GEOMETRY );
// vertexBuffer = 0;
OGRE_FREE_SIMD( v, MEMCATEGORY_GEOMETRY );
v = 0;
throw e;
}
Ok that's what I have for now.
Okay thank you for help with there. I'll address them later.
I made a video with a faster way to leak v1 meshes GPU RAM.
So, as on video:
By the way, you can use:
Code: Select all
size_t bytesPerVertex = VaoManager::calculateVertexSize( vertexElements );instead of manually tracking the size in vertSize (which is error-prone):
Code: Select all
vertexElements.push_back( VertexElement2( VET_FLOAT3, VES_POSITION ) ); vertSize += 3;
vertSize *= sizeof( float );