From 79ef6b12334bdd2478db2ba884fc5d2fcd51ea73 Mon Sep 17 00:00:00 2001 From: lainon Date: Fri, 7 Oct 2022 12:26:46 +0300 Subject: [PATCH 1/2] Replace insert and push to emplace, using std::move, check parent before access --- OgreMain/src/OgreScriptTranslator.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/OgreMain/src/OgreScriptTranslator.cpp b/OgreMain/src/OgreScriptTranslator.cpp index 9d7ed3b8044..44c87278c11 100644 --- a/OgreMain/src/OgreScriptTranslator.cpp +++ b/OgreMain/src/OgreScriptTranslator.cpp @@ -1227,7 +1227,7 @@ namespace Ogre{ ++itor; } } - paramVec.push_back( std::pair( prop->name, value ) ); + paramVec.emplace_back( prop->name, value ); } } } @@ -1361,7 +1361,7 @@ namespace Ogre{ AbstractNodeList::const_iterator i0 = getNodeAt(prop->values, 0), i1 = getNodeAt(prop->values, 1); String name, value; if(getString(*i0, &name) && getString(*i1, &value)) - mTextureAliases.insert(std::make_pair(name, value)); + mTextureAliases.emplace(name, value); else compiler->addError(ScriptCompiler::CE_INVALIDPARAMETERS, prop->file, prop->line, "set_texture_alias must have 2 string argument"); @@ -3351,7 +3351,7 @@ namespace Ogre{ } else { - names[n++] = name; + names[n++] = std::move(name); } } else @@ -4740,7 +4740,7 @@ namespace Ogre{ value += ((AtomAbstractNode*)(*it).get())->value; } } - customParameters.push_back(std::make_pair(name, value)); + customParameters.emplace_back(name, value); } } else if((*i)->type == ANT_OBJECT) @@ -4818,7 +4818,7 @@ namespace Ogre{ ProcessResourceNameScriptCompilerEvent evt(ProcessResourceNameScriptCompilerEvent::GPU_PROGRAM, value); compiler->_fireEvent(&evt, 0); - customParameters.push_back(std::make_pair("delegate", evt.mName)); + customParameters.emplace_back("delegate", evt.mName); } else { @@ -4835,7 +4835,7 @@ namespace Ogre{ value += ((AtomAbstractNode*)(*it).get())->value; } } - customParameters.push_back(std::make_pair(name, value)); + customParameters.emplace_back(name, value); } } else if((*i)->type == ANT_OBJECT) @@ -4949,7 +4949,7 @@ namespace Ogre{ } } } - customParameters.push_back(std::make_pair(name, value)); + customParameters.emplace_back(name, value); } } else if((*i)->type == ANT_OBJECT) @@ -6530,7 +6530,7 @@ namespace Ogre{ td->widthFactor = widthFactor; td->heightFactor = heightFactor; td->format = format; - td->fsaa = fsaa; + td->fsaa = std::move(fsaa); td->textureFlags = textureFlags; td->depthBufferId = depthBufferId; td->preferDepthTexture = preferDepthTexture; @@ -7936,7 +7936,7 @@ namespace Ogre{ if( getIdString( *it, &texName ) ) { if( colourIdx >= mRtv->colourAttachments.size() ) - mRtv->colourAttachments.push_back( RenderTargetViewEntry() ); + mRtv->colourAttachments.emplace_back(); mRtv->colourAttachments.back().textureName = texName; ++colourIdx; } @@ -9287,8 +9287,6 @@ namespace Ogre{ else { passQuad->mMaterialIsHlms = prop->id == ID_HLMS; - - String val; if( !getString(prop->values.front(), &passQuad->mMaterialName) ) { compiler->addError(ScriptCompiler::CE_INVALIDPARAMETERS, prop->file, prop->line); @@ -11537,7 +11535,8 @@ namespace Ogre{ (parent->id == ID_COMPOSITOR_NODE || parent->id == ID_SHADOW_NODE || parent->id == ID_SHADOW_MAP_REPEAT)) translator = &mCompositorTargetTranslator; - else if( obj->id == ID_RTV && (parent->id == ID_COMPOSITOR_NODE || + else if( obj->id == ID_RTV && parent && + ( parent->id == ID_COMPOSITOR_NODE || parent->id == ID_SHADOW_NODE) ) translator = &mCompositorRenderTargetViewTranslator; else if(obj->id == ID_SHADOW_MAP_TARGET_TYPE && parent && parent->id == ID_SHADOW_NODE) From 160d5f682c2561325c99a0984dee969fa8efd95a Mon Sep 17 00:00:00 2001 From: lainon Date: Fri, 7 Oct 2022 12:53:27 +0300 Subject: [PATCH 2/2] Code simplify, remove unused, replace list to vector, lower scope code --- OgreMain/src/OgreScriptTranslator.cpp | 78 +++++++++++++-------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/OgreMain/src/OgreScriptTranslator.cpp b/OgreMain/src/OgreScriptTranslator.cpp index 44c87278c11..da3826a4b8f 100644 --- a/OgreMain/src/OgreScriptTranslator.cpp +++ b/OgreMain/src/OgreScriptTranslator.cpp @@ -1361,7 +1361,7 @@ namespace Ogre{ AbstractNodeList::const_iterator i0 = getNodeAt(prop->values, 0), i1 = getNodeAt(prop->values, 1); String name, value; if(getString(*i0, &name) && getString(*i1, &value)) - mTextureAliases.emplace(name, value); + mTextureAliases.emplace( name, value ); else compiler->addError(ScriptCompiler::CE_INVALIDPARAMETERS, prop->file, prop->line, "set_texture_alias must have 2 string argument"); @@ -2399,10 +2399,11 @@ namespace Ogre{ } else { - AbstractNodeList::const_iterator i1 = getNodeAt(prop->values, 1), i2 = getNodeAt(prop->values, 2); + AbstractNodeList::const_iterator i1 = getNodeAt(prop->values, 1); bool val = false; if(getBoolean(prop->values.front(), &val)) { + AbstractNodeList::const_iterator i2 = getNodeAt( prop->values, 2 ); FogMode mode = FOG_NONE; ColourValue clr = ColourValue::White; Real dens = Real( 0.001 ), start = 0.0f, end = 1.0f; @@ -4685,7 +4686,7 @@ namespace Ogre{ //------------------------------------------------------------------------- void GpuProgramTranslator::translateGpuProgram(ScriptCompiler *compiler, ObjectAbstractNode *obj) { - list >::type customParameters; + vector >::type customParameters; String syntax, source; AbstractNodePtr params; for(AbstractNodeList::iterator i = obj->children.begin(); i != obj->children.end(); ++i) @@ -4740,7 +4741,7 @@ namespace Ogre{ value += ((AtomAbstractNode*)(*it).get())->value; } } - customParameters.emplace_back(name, value); + customParameters.emplace_back( name, value ); } } else if((*i)->type == ANT_OBJECT) @@ -4790,7 +4791,7 @@ namespace Ogre{ prog->_notifyOrigin(obj->file); // Set the custom parameters - for(list >::type::iterator i = customParameters.begin(); i != customParameters.end(); ++i) + for(vector >::type::iterator i = customParameters.begin(); i != customParameters.end(); ++i) prog->setParameter(i->first, i->second); // Set up default parameters @@ -4803,7 +4804,7 @@ namespace Ogre{ //------------------------------------------------------------------------- void GpuProgramTranslator::translateUnifiedGpuProgram(ScriptCompiler *compiler, ObjectAbstractNode *obj) { - list >::type customParameters; + vector >::type customParameters; AbstractNodePtr params; for(AbstractNodeList::iterator i = obj->children.begin(); i != obj->children.end(); ++i) { @@ -4818,7 +4819,7 @@ namespace Ogre{ ProcessResourceNameScriptCompilerEvent evt(ProcessResourceNameScriptCompilerEvent::GPU_PROGRAM, value); compiler->_fireEvent(&evt, 0); - customParameters.emplace_back("delegate", evt.mName); + customParameters.emplace_back( "delegate", evt.mName ); } else { @@ -4835,7 +4836,7 @@ namespace Ogre{ value += ((AtomAbstractNode*)(*it).get())->value; } } - customParameters.emplace_back(name, value); + customParameters.emplace_back( name, value ); } } else if((*i)->type == ANT_OBJECT) @@ -4874,7 +4875,7 @@ namespace Ogre{ prog->_notifyOrigin(obj->file); // Set the custom parameters - for(list >::type::iterator i = customParameters.begin(); i != customParameters.end(); ++i) + for(vector >::type::iterator i = customParameters.begin(); i != customParameters.end(); ++i) prog->setParameter(i->first, i->second); // Set up default parameters @@ -4900,7 +4901,7 @@ namespace Ogre{ return; } - list >::type customParameters; + vector >::type customParameters; String source; AbstractNodePtr params; for(AbstractNodeList::iterator i = obj->children.begin(); i != obj->children.end(); ++i) @@ -4949,7 +4950,7 @@ namespace Ogre{ } } } - customParameters.emplace_back(name, value); + customParameters.emplace_back( name, value ); } } else if((*i)->type == ANT_OBJECT) @@ -4991,7 +4992,7 @@ namespace Ogre{ prog->_notifyOrigin(obj->file); // Set the custom parameters - for(list >::type::iterator i = customParameters.begin(); i != customParameters.end(); ++i) + for(vector >::type::iterator i = customParameters.begin(); i != customParameters.end(); ++i) prog->setParameter(i->first, i->second); // Set up default parameters @@ -5003,7 +5004,7 @@ namespace Ogre{ } //------------------------------------------------------------------------- - uint32 parseProgramParameterDimensions(String& declarator, String type) + uint32 parseProgramParameterDimensions(const String& declarator, const String& type) { // Assume 1 unless otherwise specified uint32 dimensions = 1; @@ -5012,7 +5013,7 @@ namespace Ogre{ if (start != String::npos) { - size_t end = declarator.find_first_of("[", start); + size_t end = declarator.find_first_of('[', start); // int1, int2, etc. if (end != start) @@ -5025,10 +5026,10 @@ namespace Ogre{ // C-style array while (start != String::npos) { - end = declarator.find_first_of("]", start); + end = declarator.find_first_of(']', start); dimensions *= StringConverter::parseUnsignedInt( declarator.substr(start + 1, end - start - 1)); - start = declarator.find_first_of("[", end); + start = declarator.find_first_of('[', end); } } @@ -7702,7 +7703,7 @@ namespace Ogre{ else { AbstractNodeList::const_iterator it1 = prop->values.begin(); - AbstractNodeList::const_iterator it0 = it1++; + AbstractNodeList::const_iterator it0 = std::next( it1 ); uint32 outChannel; IdString bufferName; @@ -8627,7 +8628,7 @@ namespace Ogre{ uint32 colourIdx; LoadAction::LoadAction loadAction; AbstractNodeList::const_iterator it1 = prop->values.begin(); - AbstractNodeList::const_iterator it0 = it1++; + AbstractNodeList::const_iterator it0 = std::next( it1 ); if( getUInt( *it0, &colourIdx ) && getLoadAction( *it1, &loadAction ) ) { if( colourIdx < OGRE_MAX_MULTIPLE_RENDER_TARGETS ) @@ -8912,7 +8913,7 @@ namespace Ogre{ uint32 colourIdx; StoreAction::StoreAction storeAction; AbstractNodeList::const_iterator it1 = prop->values.begin(); - AbstractNodeList::const_iterator it0 = it1++; + AbstractNodeList::const_iterator it0 = std::next( it1 ); if( getUInt( *it0, &colourIdx ) && getStoreAction( *it1, &storeAction ) ) { if( colourIdx < OGRE_MAX_MULTIPLE_RENDER_TARGETS ) @@ -9307,8 +9308,8 @@ namespace Ogre{ else { AbstractNodeList::const_iterator it2 = prop->values.begin(); - AbstractNodeList::const_iterator it0 = it2++; - AbstractNodeList::const_iterator it1 = it2++; + AbstractNodeList::const_iterator it0 = std::next( it2 ); + AbstractNodeList::const_iterator it1 = std::next( it2, 2 ); uint32 id; String name; @@ -9717,9 +9718,9 @@ namespace Ogre{ gbuffer.resize( 2 ); AbstractNodeList::const_iterator it3 = prop->values.begin(); - AbstractNodeList::const_iterator it0 = it3++; - AbstractNodeList::const_iterator it1 = it3++; - AbstractNodeList::const_iterator it2 = it3++; + AbstractNodeList::const_iterator it0 = std::next( it3 ); + AbstractNodeList::const_iterator it1 = std::next( it3, 2 ); + AbstractNodeList::const_iterator it2 = std::next( it3, 3 ); if( !getIdString( *it0, &gbuffer[0] ) || !getIdString( *it1, &gbuffer[1] ) || @@ -9766,7 +9767,7 @@ namespace Ogre{ IdString depthTexture, refractions; AbstractNodeList::const_iterator it1 = prop->values.begin(); - AbstractNodeList::const_iterator it0 = it1++; + AbstractNodeList::const_iterator it0 = std::next( it1 ); if( !getIdString( *it0, &depthTexture ) || !getIdString( *it1, &refractions ) ) { @@ -9812,7 +9813,7 @@ namespace Ogre{ else { AbstractNodeList::const_iterator it1 = prop->values.begin(); - AbstractNodeList::const_iterator it0 = it1++; + AbstractNodeList::const_iterator it0 = std::next( it1 ); if( !getReal( *it0, &passScene->mUvBakingOffset.x ) || !getReal( *it1, &passScene->mUvBakingOffset.y ) ) @@ -9943,10 +9944,6 @@ namespace Ogre{ } AbstractNodeList::const_iterator it0 = prop->values.begin(); - AbstractNodeList::const_iterator it1 = it0; - if( prop->values.size() > 1 ) - ++it1; - String str; if( getString( *it0, &str ) ) { @@ -10719,8 +10716,8 @@ namespace Ogre{ else { AbstractNodeList::const_iterator it2 = prop->values.begin(); - AbstractNodeList::const_iterator it0 = it2++; - AbstractNodeList::const_iterator it1 = it2++; + AbstractNodeList::const_iterator it0 = std::next( it2 ); + AbstractNodeList::const_iterator it1 = std::next( it2, 2 ); uint32 id; String name; @@ -10772,6 +10769,7 @@ namespace Ogre{ else if((*i)->type == ANT_PROPERTY) { PropertyAbstractNode *prop = reinterpret_cast((*i).get()); + const AbstractNodePtr beginNode = prop->values.front(); switch(prop->id) { case ID_MIPMAP_METHOD: @@ -10786,9 +10784,9 @@ namespace Ogre{ } else { - if(prop->values.front()->type == ANT_ATOM) + if( beginNode->type == ANT_ATOM ) { - AtomAbstractNode *atom = (AtomAbstractNode*)prop->values.front().get(); + AtomAbstractNode *atom = (AtomAbstractNode *)beginNode.get(); switch(atom->id) { case ID_API_DEFAULT: @@ -10802,14 +10800,14 @@ namespace Ogre{ break; default: compiler->addError(ScriptCompiler::CE_INVALIDPARAMETERS, prop->file, prop->line, - prop->values.front()->getValue() + + beginNode->getValue() + " is not a valid miprmap_method (api_default, compute, or compute_hq)"); } } else { compiler->addError(ScriptCompiler::CE_INVALIDPARAMETERS, prop->file, prop->line, - prop->values.front()->getValue() + + beginNode->getValue() + " is not a valid miprmap_method (api_default, compute, or compute_hq)"); } } @@ -10827,10 +10825,10 @@ namespace Ogre{ else { int value = 8; - if( !getInt( prop->values.front(), &value ) ) + if( !getInt( beginNode, &value ) ) { compiler->addError(ScriptCompiler::CE_INVALIDPARAMETERS, prop->file, prop->line, - prop->values.front()->getValue() + " is not a valid integer argument"); + beginNode->getValue() + " is not a valid integer argument" ); } else { @@ -10850,10 +10848,10 @@ namespace Ogre{ } else { - if( !getFloat( prop->values.front(), &passMipmap->mGaussianDeviationFactor ) ) + if( !getFloat( beginNode, &passMipmap->mGaussianDeviationFactor ) ) { compiler->addError(ScriptCompiler::CE_INVALIDPARAMETERS, prop->file, prop->line, - prop->values.front()->getValue() + " is not a valid integer argument"); + beginNode->getValue() + " is not a valid integer argument" ); } } break;