From d95272281168405d817b25d4da06b2298286d343 Mon Sep 17 00:00:00 2001 From: Areloch Date: Sat, 3 Feb 2024 16:10:28 -0600 Subject: [PATCH 1/7] Updates the field types used in the editor to utilize the GuiPopUpMenuCtrlEx to make them support categories and be able to search filter them Updates the dataBlock field type to properly present categorized listings Expands the datablock Field to have an edit and add buttons on the field to make the workflow simpler Adds utility functions to GuiPopUpMenuCtrlEx to control indentation, categories and searchability Expands datablock editor functionality to be able to create a datablock of a type to pre-set the inheritFrom param of the process early(used for the add new button on DB fields to carry-through the current DB to the creation process of a derivative) --- Engine/source/gui/controls/guiPopUpCtrlEx.cpp | 14 +- Engine/source/gui/controls/guiPopUpCtrlEx.h | 12 +- .../source/gui/editor/guiInspectorTypes.cpp | 20 +-- Engine/source/gui/editor/guiInspectorTypes.h | 19 +-- .../gui/editor/inspector/datablockField.cpp | 140 +++++++++++++++++- .../gui/editor/inspector/datablockField.h | 8 +- .../datablockEditor/datablockEditor.tscript | 27 +++- 7 files changed, 205 insertions(+), 35 deletions(-) diff --git a/Engine/source/gui/controls/guiPopUpCtrlEx.cpp b/Engine/source/gui/controls/guiPopUpCtrlEx.cpp index ddfcb4d5b..45183285d 100644 --- a/Engine/source/gui/controls/guiPopUpCtrlEx.cpp +++ b/Engine/source/gui/controls/guiPopUpCtrlEx.cpp @@ -855,7 +855,7 @@ void GuiPopUpMenuCtrlEx::sortID() } //------------------------------------------------------------------------------ -void GuiPopUpMenuCtrlEx::addEntry(const char *buf, S32 id, U32 scheme) +void GuiPopUpMenuCtrlEx::addEntry(const char *buf, S32 id, U32 scheme, const bool& intented) { if( !buf ) { @@ -882,6 +882,7 @@ void GuiPopUpMenuCtrlEx::addEntry(const char *buf, S32 id, U32 scheme) dStrcpy( e.buf, buf, 256 ); e.id = id; e.scheme = scheme; + e.indented = intented; // see if there is a shortcut key char * cp = dStrchr( e.buf, '~' ); @@ -1326,8 +1327,8 @@ void GuiPopUpMenuCtrlEx::closePopUp() mSelIndex = ( mRevNum >= mSelIndex && mSelIndex != -1 ) ? mRevNum - mSelIndex : mSelIndex; if ( mSelIndex != -1 ) { - if ( mReplaceText ) - setText(mTl->mList[mSelIndex].text); + if (mReplaceText) + setText(mEntries[mSelIndex].buf); for(U32 i=0; i < mEntries.size(); i++) { @@ -1465,7 +1466,12 @@ void GuiPopUpMenuCtrlEx::onAction() } else { - mTl->addEntry(mEntries[j].id, mEntries[j].buf); + String entryText = mEntries[j].buf; + + if(mEntries[j].indented) + entryText = String(" ") + entryText; + + mTl->addEntry(mEntries[j].id, entryText.c_str()); } } diff --git a/Engine/source/gui/controls/guiPopUpCtrlEx.h b/Engine/source/gui/controls/guiPopUpCtrlEx.h index 503d91690..e1136626c 100644 --- a/Engine/source/gui/controls/guiPopUpCtrlEx.h +++ b/Engine/source/gui/controls/guiPopUpCtrlEx.h @@ -86,8 +86,9 @@ class GuiPopUpMenuCtrlEx : public GuiTextCtrl S32 id; U16 ascii; U16 scheme; - bool usesColorBox; // Added - ColorI colorbox; // Added + bool usesColorBox; // Added + ColorI colorbox; // Added + bool indented; // Added }; struct Scheme @@ -156,7 +157,11 @@ class GuiPopUpMenuCtrlEx : public GuiTextCtrl void setBitmap(const char *name); // Added void sort(); void sortID(); // Added - void addEntry(const char *buf, S32 id = -1, U32 scheme = 0); + void addEntry(const char *buf, S32 id = -1, U32 scheme = 0, const bool& indented = false); + void addCategory(const char *buf) + { + addEntry(buf, -2, 0); + } void addScheme(U32 id, ColorI fontColor, ColorI fontColorHL, ColorI fontColorSEL); void onRender(Point2I offset, const RectI &updateRect); void onAction(); @@ -184,6 +189,7 @@ class GuiPopUpMenuCtrlEx : public GuiTextCtrl S32 getNumEntries() { return( mEntries.size() ); } void replaceText(S32); + void setCanSearch(const bool& canSearch) { mTextSearchItems = canSearch; } void setSearchText(String searchTxt) { mSearchText = String::ToLower(searchTxt); onAction(); } DECLARE_CONOBJECT(GuiPopUpMenuCtrlEx); diff --git a/Engine/source/gui/editor/guiInspectorTypes.cpp b/Engine/source/gui/editor/guiInspectorTypes.cpp index 49d693523..61e884a0e 100644 --- a/Engine/source/gui/editor/guiInspectorTypes.cpp +++ b/Engine/source/gui/editor/guiInspectorTypes.cpp @@ -56,9 +56,9 @@ ConsoleDocClass( GuiInspectorTypeMenuBase, GuiControl* GuiInspectorTypeMenuBase::constructEditControl() { - GuiControl* retCtrl = new GuiPopUpMenuCtrl(); + GuiControl* retCtrl = new GuiPopUpMenuCtrlEx(); - GuiPopUpMenuCtrl *menu = dynamic_cast(retCtrl); + GuiPopUpMenuCtrlEx *menu = dynamic_cast(retCtrl); // Let's make it look pretty. retCtrl->setDataField( StringTable->insert("profile"), NULL, "ToolsGuiPopupMenuProfile" ); @@ -89,7 +89,7 @@ void GuiInspectorTypeMenuBase::setValue( StringTableEntry newValue ) ctrl->setText( newValue ); } -void GuiInspectorTypeMenuBase::_populateMenu( GuiPopUpMenuCtrl *menu ) +void GuiInspectorTypeMenuBase::_populateMenu( GuiPopUpMenuCtrlEx *menu ) { // do nothing, child classes override this. } @@ -105,7 +105,7 @@ ConsoleDocClass( GuiInspectorTypeEnum, "@internal" ); -void GuiInspectorTypeEnum::_populateMenu( GuiPopUpMenuCtrl *menu ) +void GuiInspectorTypeEnum::_populateMenu( GuiPopUpMenuCtrlEx *menu ) { const EngineEnumTable* table = mField->table; if( !table ) @@ -148,7 +148,7 @@ ConsoleDocClass( GuiInspectorTypeCubemapName, "@internal" ); -void GuiInspectorTypeCubemapName::_populateMenu( GuiPopUpMenuCtrl *menu ) +void GuiInspectorTypeCubemapName::_populateMenu(GuiPopUpMenuCtrlEx *menu ) { PROFILE_SCOPE( GuiInspectorTypeCubemapName_populateMenu ); @@ -356,7 +356,7 @@ ConsoleDocClass( GuiInspectorTypeGuiProfile, "@internal" ); -void GuiInspectorTypeGuiProfile::_populateMenu( GuiPopUpMenuCtrl *menu ) +void GuiInspectorTypeGuiProfile::_populateMenu(GuiPopUpMenuCtrlEx *menu ) { // Check whether we should show profiles from the editor category. @@ -399,7 +399,7 @@ ConsoleDocClass(GuiInspectorTypeActionMap, "@internal" ); -void GuiInspectorTypeActionMap::_populateMenu(GuiPopUpMenuCtrl* menu) +void GuiInspectorTypeActionMap::_populateMenu(GuiPopUpMenuCtrlEx* menu) { // Add the action maps to the menu. //First add a blank entry so you can clear the action map @@ -1671,7 +1671,7 @@ ConsoleDocClass( GuiInspectorTypeSFXParameterName, "@internal" ); -void GuiInspectorTypeSFXParameterName::_populateMenu( GuiPopUpMenuCtrl *menu ) +void GuiInspectorTypeSFXParameterName::_populateMenu(GuiPopUpMenuCtrlEx *menu ) { SimSet* set = Sim::getSFXParameterGroup(); for( SimSet::iterator iter = set->begin(); iter != set->end(); ++ iter ) @@ -1703,7 +1703,7 @@ ConsoleDocClass( GuiInspectorTypeSFXStateName, "@internal" ); -void GuiInspectorTypeSFXStateName::_populateMenu( GuiPopUpMenuCtrl *menu ) +void GuiInspectorTypeSFXStateName::_populateMenu(GuiPopUpMenuCtrlEx *menu ) { menu->addEntry( "", 0 ); @@ -1737,7 +1737,7 @@ ConsoleDocClass( GuiInspectorTypeSFXSourceName, "@internal" ); -void GuiInspectorTypeSFXSourceName::_populateMenu( GuiPopUpMenuCtrl *menu ) +void GuiInspectorTypeSFXSourceName::_populateMenu(GuiPopUpMenuCtrlEx *menu ) { menu->addEntry( "", 0 ); diff --git a/Engine/source/gui/editor/guiInspectorTypes.h b/Engine/source/gui/editor/guiInspectorTypes.h index d5cb90f93..2f9e26b31 100644 --- a/Engine/source/gui/editor/guiInspectorTypes.h +++ b/Engine/source/gui/editor/guiInspectorTypes.h @@ -45,6 +45,7 @@ #ifndef _GUITEXTEDITSLIDERCTRL_H_ #include "gui/controls/guiTextEditSliderCtrl.h" #endif +#include "gui/controls/guiPopUpCtrlEx.h" class GuiPopUpMenuCtrl; @@ -63,7 +64,7 @@ public: //----------------------------------------------------------------------------- virtual GuiControl* constructEditControl(); virtual void setValue( StringTableEntry newValue ); - virtual void _populateMenu( GuiPopUpMenuCtrl *menu ); + virtual void _populateMenu( GuiPopUpMenuCtrlEx *menu ); }; //----------------------------------------------------------------------------- @@ -77,7 +78,7 @@ public: DECLARE_CONOBJECT(GuiInspectorTypeEnum); static void consoleInit(); - virtual void _populateMenu( GuiPopUpMenuCtrl *menu ); + virtual void _populateMenu(GuiPopUpMenuCtrlEx *menu ); }; //----------------------------------------------------------------------------- @@ -91,7 +92,7 @@ public: DECLARE_CONOBJECT(GuiInspectorTypeCubemapName); static void consoleInit(); - virtual void _populateMenu( GuiPopUpMenuCtrl *menu ); + virtual void _populateMenu(GuiPopUpMenuCtrlEx *menu ); }; //-------------------------------------------------------------------------------- @@ -129,7 +130,7 @@ public: GuiInspectorTypeRegularMaterialName() {} DECLARE_CONOBJECT(GuiInspectorTypeRegularMaterialName); static void consoleInit(); - virtual void _populateMenu( GuiPopUpMenuCtrl *menu ); + virtual void _populateMenu(GuiPopUpMenuCtrlEx *menu ); }; //-------------------------------------------------------------------------------- @@ -183,7 +184,7 @@ public: DECLARE_CONOBJECT(GuiInspectorTypeGuiProfile); static void consoleInit(); - virtual void _populateMenu( GuiPopUpMenuCtrl *menu ); + virtual void _populateMenu(GuiPopUpMenuCtrlEx *menu ); }; //----------------------------------------------------------------------------- @@ -197,7 +198,7 @@ public: DECLARE_CONOBJECT(GuiInspectorTypeActionMap); static void consoleInit(); - virtual void _populateMenu(GuiPopUpMenuCtrl* menu); + virtual void _populateMenu(GuiPopUpMenuCtrlEx * menu); }; //----------------------------------------------------------------------------- @@ -561,7 +562,7 @@ public: DECLARE_CONOBJECT(GuiInspectorTypeSFXParameterName); static void consoleInit(); - virtual void _populateMenu( GuiPopUpMenuCtrl *menu ); + virtual void _populateMenu(GuiPopUpMenuCtrlEx *menu ); }; @@ -576,7 +577,7 @@ public: DECLARE_CONOBJECT(GuiInspectorTypeSFXStateName); static void consoleInit(); - virtual void _populateMenu( GuiPopUpMenuCtrl *menu ); + virtual void _populateMenu(GuiPopUpMenuCtrlEx *menu ); }; @@ -591,7 +592,7 @@ public: DECLARE_CONOBJECT(GuiInspectorTypeSFXSourceName); static void consoleInit(); - virtual void _populateMenu( GuiPopUpMenuCtrl *menu ); + virtual void _populateMenu(GuiPopUpMenuCtrlEx *menu ); }; //----------------------------------------------------------------------------- diff --git a/Engine/source/gui/editor/inspector/datablockField.cpp b/Engine/source/gui/editor/inspector/datablockField.cpp index cce8cb443..85bf418bb 100644 --- a/Engine/source/gui/editor/inspector/datablockField.cpp +++ b/Engine/source/gui/editor/inspector/datablockField.cpp @@ -32,6 +32,7 @@ #include "sfx/sfxEnvironment.h" #include "sfx/sfxAmbience.h" #include "sfx/sfxTrack.h" +#include "T3D/gameBase/gameBase.h" //----------------------------------------------------------------------------- @@ -64,30 +65,157 @@ void GuiInspectorDatablockField::setClassName( StringTableEntry className ) } } -void GuiInspectorDatablockField::_populateMenu( GuiPopUpMenuCtrl* menu ) +void GuiInspectorDatablockField::_populateMenu( GuiPopUpMenuCtrlEx* menu ) { + menu->setCanSearch(true); menu->addScheme( 1, ColorI( 80, 0, 0, 255 ), ColorI( 80, 0, 0, 255 ), ColorI( 80, 0, 0, 255 ) ); // For client-only coloring. menu->addEntry( "", 0 ); // For unsetting. SimSet* set = _getDatablockSet(); U32 id = 1; - for( SimSet::iterator iter = set->begin(); iter != set->end(); ++ iter ) + //We can do some special filtering here if it's derived from GameBase a la categories + if(mDesiredClass->isSubclassOf(AbstractClassRep::findClassRep("GameBaseData"))) { - SimDataBlock* datablock = dynamic_cast< SimDataBlock* >( *iter ); + //First, do categories + Vector categories; + for (SimSet::iterator iter = set->begin(); iter != set->end(); ++iter) + { + SimDataBlock* datablock = dynamic_cast(*iter); + + // Skip non-datablocks if we somehow encounter them. + if (!datablock) + continue; + + if (datablock && (!mDesiredClass || datablock->getClassRep()->isClass(mDesiredClass))) + { + GameBaseData *data = dynamic_cast(datablock); + if(data) + { + String category = data->mCategory; + if(category.isNotEmpty() && (categories.empty() || categories.find_next(category) == -1)) + categories.push_back(category); + } + } + } + + if (categories.size() > 0) + { + categories.push_back("No Category"); + + //Now that we have our categories, lets populate our list + for (Vector::iterator catIter = categories.begin(); catIter != categories.end(); ++catIter) + { + String categoryName = String::ToLower(catIter->c_str()); + if (categoryName != String::EmptyString) + { + menu->addCategory(catIter->c_str()); + id++; + } + + for (SimSet::iterator iter = set->begin(); iter != set->end(); ++iter) + { + GameBaseData* datablock = dynamic_cast(*iter); + + // Skip non-datablocks if we somehow encounter them. + if (!datablock) + continue; + + if (datablock && (!mDesiredClass || datablock->getClassRep()->isClass(mDesiredClass)) && + (String::ToLower(datablock->mCategory) == categoryName || (datablock->mCategory == String::EmptyString && categoryName == String("No Category")))) + { + menu->addEntry(datablock->getName(), id++, datablock->isClientOnly() ? 1 : 0, true); + } + } + } + + return; + } + } + + for (SimSet::iterator iter = set->begin(); iter != set->end(); ++iter) + { + SimDataBlock* datablock = dynamic_cast(*iter); // Skip non-datablocks if we somehow encounter them. - if( !datablock ) + if (!datablock) continue; // Ok, now we have to figure inheritance info. - if( datablock && ( !mDesiredClass || datablock->getClassRep()->isClass( mDesiredClass ) ) ) - menu->addEntry( datablock->getName(), id ++, datablock->isClientOnly() ? 1 : 0 ); + if (datablock && (!mDesiredClass || datablock->getClassRep()->isClass(mDesiredClass))) + menu->addEntry(datablock->getName(), id++, datablock->isClientOnly() ? 1 : 0); } menu->sort(); } +GuiControl* GuiInspectorDatablockField::constructEditControl() +{ + // Create base filename edit controls + GuiControl* retCtrl = Parent::constructEditControl(); + if (retCtrl == NULL) + return retCtrl; + + char szBuffer[512]; + + // Create "Open in Editor" button + mEditButton = new GuiButtonCtrl(); + dSprintf(szBuffer, sizeof(szBuffer), "DatablockEditorPlugin.openDatablock(%d.getText());", retCtrl->getId()); + mEditButton->setField("Command", szBuffer); + mEditButton->setText("..."); + + mEditButton->setDataField(StringTable->insert("Profile"), NULL, "GuiInspectorButtonProfile"); + mEditButton->setDataField(StringTable->insert("tooltipprofile"), NULL, "GuiToolTipProfile"); + mEditButton->setDataField(StringTable->insert("hovertime"), NULL, "1000"); + mEditButton->setDataField(StringTable->insert("tooltip"), NULL, "Edit this datablock"); + + mEditButton->registerObject(); + addObject(mEditButton); + + //Add add button + mAddButton = new GuiBitmapButtonCtrl(); + dSprintf(szBuffer, sizeof(szBuffer), "DatablockEditorPlugin.createNewDatablockOfType(%s, %d.getText());", mDesiredClass->getClassName(), retCtrl->getId()); + mAddButton->setField("Command", szBuffer); + + char addBtnBitmapName[512] = "ToolsModule:iconAdd_Image"; + mAddButton->setBitmap(StringTable->insert(addBtnBitmapName)); + + mAddButton->setDataField(StringTable->insert("Profile"), NULL, "GuiButtonProfile"); + mAddButton->setDataField(StringTable->insert("tooltipprofile"), NULL, "GuiToolTipProfile"); + mAddButton->setDataField(StringTable->insert("hovertime"), NULL, "1000"); + mAddButton->setDataField(StringTable->insert("tooltip"), NULL, "Create new datablock"); + + mAddButton->registerObject(); + addObject(mAddButton); + + return retCtrl; +} + +bool GuiInspectorDatablockField::updateRects() +{ + S32 dividerPos, dividerMargin; + mInspector->getDivider(dividerPos, dividerMargin); + Point2I fieldExtent = getExtent(); + Point2I fieldPos = getPosition(); + + mCaptionRect.set(0, 0, fieldExtent.x - dividerPos - dividerMargin, fieldExtent.y); + mEditCtrlRect.set(fieldExtent.x - dividerPos + dividerMargin, 1, dividerPos - dividerMargin - 34, fieldExtent.y); + + bool resized = mEdit->resize(mEditCtrlRect.point, mEditCtrlRect.extent); + if (mEditButton != NULL) + { + mBrowseRect.set(fieldExtent.x - 32, 2, 14, fieldExtent.y - 4); + resized |= mEditButton->resize(mBrowseRect.point, mBrowseRect.extent); + } + + if (mAddButton != NULL) + { + RectI shapeEdRect(fieldExtent.x - 16, 2, 14, fieldExtent.y - 4); + resized |= mAddButton->resize(shapeEdRect.point, shapeEdRect.extent); + } + + return resized; +} //----------------------------------------------------------------------------- // GuiInspectorTypeSFXDescriptionName diff --git a/Engine/source/gui/editor/inspector/datablockField.h b/Engine/source/gui/editor/inspector/datablockField.h index f08a51072..b12224766 100644 --- a/Engine/source/gui/editor/inspector/datablockField.h +++ b/Engine/source/gui/editor/inspector/datablockField.h @@ -23,6 +23,7 @@ #ifndef _GUI_INSPECTOR_DATABLOCKFIELD_H_ #define _GUI_INSPECTOR_DATABLOCKFIELD_H_ +#include "gui/controls/guiPopUpCtrlEx.h" #include "gui/editor/guiInspectorTypes.h" @@ -38,9 +39,14 @@ class GuiInspectorDatablockField : public GuiInspectorTypeMenuBase protected: AbstractClassRep *mDesiredClass; + SimObjectPtr mAddButton; + SimObjectPtr mEditButton; + RectI mBrowseRect; virtual SimSet* _getDatablockSet() const { return Sim::getDataBlockSet(); } - virtual void _populateMenu( GuiPopUpMenuCtrl* menu ); + virtual void _populateMenu( GuiPopUpMenuCtrlEx* menu ); + virtual GuiControl* constructEditControl(); + virtual bool updateRects(); public: diff --git a/Templates/BaseGame/game/tools/datablockEditor/datablockEditor.tscript b/Templates/BaseGame/game/tools/datablockEditor/datablockEditor.tscript index 2fa9cc30a..5f9d23d78 100644 --- a/Templates/BaseGame/game/tools/datablockEditor/datablockEditor.tscript +++ b/Templates/BaseGame/game/tools/datablockEditor/datablockEditor.tscript @@ -629,10 +629,21 @@ function DatablockEditorPlugin::deleteDatablock( %this ) } //--------------------------------------------------------------------------------------------- +function DatablockEditorPlugin::createNewDatablockOfType(%this, %class, %inheritFrom) +{ + $DATABLOCK_EDITOR_NEWDB_CLASS = %class; + $DATABLOCK_EDITOR_NEWDB_INHERITFROM = %inheritFrom; + + %this.pickDatablockPath(); +} function DatablockEditorPlugin::createDatablock(%this) { - %class = DatablockEditorTypeTree.getItemText(DatablockEditorTypeTree.getSelectedItem()); + if($DATABLOCK_EDITOR_NEWDB_CLASS $= "") + %class = DatablockEditorTypeTree.getItemText(DatablockEditorTypeTree.getSelectedItem()); + else + %class = $DATABLOCK_EDITOR_NEWDB_CLASS; + if( %class !$= "" ) { // Need to prompt for a name. @@ -659,6 +670,12 @@ function DatablockEditorPlugin::createDatablock(%this) %list.add( %datablock.getName(), %i + 1 ); } + if($DATABLOCK_EDITOR_NEWDB_INHERITFROM !$= "") + { + %inheritFromListId = %list.findText($DATABLOCK_EDITOR_NEWDB_INHERITFROM); + %list.setSelected(%inheritFromListId); + } + // Set up state of client-side checkbox. %clientSideCheckBox = DatablockEditorCreatePrompt-->ClientSideCheckBox; @@ -717,7 +734,11 @@ function DatablockEditorPlugin::createPromptNameCheck(%this, %path) function DatablockEditorPlugin::createDatablockFinish( %this, %name, %copySource ) { - %class = DatablockEditorTypeTree.getItemText(DatablockEditorTypeTree.getSelectedItem()); + if($DATABLOCK_EDITOR_NEWDB_CLASS $= "") + %class = DatablockEditorTypeTree.getItemText(DatablockEditorTypeTree.getSelectedItem()); + else + %class = $DATABLOCK_EDITOR_NEWDB_CLASS; + if( %class !$= "" ) { %action = %this.createUndo( ActionCreateDatablock, "Create New Datablock" ); @@ -742,6 +763,8 @@ function DatablockEditorPlugin::createDatablockFinish( %this, %name, %copySource %action.redo(); } + + $DATABLOCK_EDITOR_NEWDB_CLASS = ""; } //--------------------------------------------------------------------------------------------- From 7ef4552196a90f29627111092a6add6f697fc6c1 Mon Sep 17 00:00:00 2001 From: Areloch Date: Sun, 4 Feb 2024 15:25:35 -0600 Subject: [PATCH 2/7] Fixed category filtering logic for datablockField populateMenu --- Engine/source/gui/editor/inspector/datablockField.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Engine/source/gui/editor/inspector/datablockField.cpp b/Engine/source/gui/editor/inspector/datablockField.cpp index 85bf418bb..bd6962add 100644 --- a/Engine/source/gui/editor/inspector/datablockField.cpp +++ b/Engine/source/gui/editor/inspector/datablockField.cpp @@ -121,8 +121,9 @@ void GuiInspectorDatablockField::_populateMenu( GuiPopUpMenuCtrlEx* menu ) if (!datablock) continue; - if (datablock && (!mDesiredClass || datablock->getClassRep()->isClass(mDesiredClass)) && - (String::ToLower(datablock->mCategory) == categoryName || (datablock->mCategory == String::EmptyString && categoryName == String("No Category")))) + String dbCategory = String(datablock->mCategory).isEmpty() ? String("no category") : String::ToLower(datablock->mCategory); + + if (datablock && (!mDesiredClass || datablock->getClassRep()->isClass(mDesiredClass)) && (dbCategory == categoryName)) { menu->addEntry(datablock->getName(), id++, datablock->isClientOnly() ? 1 : 0, true); } From 915fac31b372fad597510bd2768c1b6e70e42f84 Mon Sep 17 00:00:00 2001 From: James Urquhart Date: Sun, 29 Oct 2023 00:38:37 +0100 Subject: [PATCH 3/7] Basic refactoring WIP --- Engine/source/core/dataChunker.h | 429 ++++++++++++-------------- Engine/source/core/frameAllocator.cpp | 26 +- Engine/source/core/frameAllocator.h | 341 ++++++++++++-------- Engine/source/core/resource.h | 1 + 4 files changed, 430 insertions(+), 367 deletions(-) diff --git a/Engine/source/core/dataChunker.h b/Engine/source/core/dataChunker.h index 3394dd27a..247793650 100644 --- a/Engine/source/core/dataChunker.h +++ b/Engine/source/core/dataChunker.h @@ -1,323 +1,300 @@ //----------------------------------------------------------------------------- -// Copyright (c) 2012 GarageGames, LLC +// Copyright (c) 2023 tgemit contributors. +// See AUTHORS file and git repository for contributor information. // -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to -// deal in the Software without restriction, including without limitation the -// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or -// sell copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING -// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS -// IN THE SOFTWARE. +// SPDX-License-Identifier: MIT //----------------------------------------------------------------------------- -#ifndef _DATACHUNKER_H_ +#pragma once #define _DATACHUNKER_H_ #ifndef _PLATFORM_H_ # include "platform/platform.h" #endif +#ifndef _PLATFORMASSERT_H_ +# include "platform/platformAssert.h" +#endif + +#include +#include +#include "core/frameAllocator.h" +//#include "math/mMathFn.h" // tgemit - needed here for the moment -//---------------------------------------------------------------------------- /// Implements a chunked data allocator. /// -/// Calling new/malloc all the time is a time consuming operation. Therefore, -/// we provide the DataChunker, which allocates memory in blocks of -/// chunkSize (by default 16k, see ChunkSize, though it can be set in -/// the constructor), then doles it out as requested, in chunks of up to -/// chunkSize in size. +/// This memory allocator allocates data in chunks of bytes, +/// the default size being ChunkSize. +/// Bytes are sourced from the current head chunk until expended, +/// in which case a new chunk of bytes will be allocated from +/// the system memory allocator. /// -/// It will assert if you try to get more than ChunkSize bytes at a time, -/// and it deals with the logic of allocating new blocks and giving out -/// word-aligned chunks. -/// -/// Note that new/free/realloc WILL NOT WORK on memory gotten from the -/// DataChunker. This also only grows (you can call freeBlocks to deallocate -/// and reset things). -class DataChunker +template class BaseDataChunker { public: - /// Block of allocated memory. - /// - /// This has nothing to do with datablocks as used in the rest of Torque. - struct DataBlock + enum { - DataBlock* next; ///< linked list pointer to the next DataBlock for this chunker - S32 curIndex; ///< current allocation point within this DataBlock - DataBlock(); - ~DataBlock(); - inline U8 *getData(); + ChunkSize = 16384 }; - enum { - PaddDBSize = (sizeof(DataBlock) + 3) & ~3, ///< Padded size of DataBlock - ChunkSize = 16384 - PaddDBSize ///< Default size of each DataBlock page in the DataChunker + struct alignas(uintptr_t) DataBlock : public AlignedBufferAllocator + { + DataBlock* mNext; + + inline DataBlock* getEnd() + { + return this + 1; + } }; - /// Return a pointer to a chunk of memory from a pre-allocated block. - /// - /// This memory goes away when you call freeBlocks. - /// - /// This memory is word-aligned. - /// @param size Size of chunk to return. This must be less than chunkSize or else - /// an assertion will occur. - void *alloc(S32 size); +protected: + dsize_t mChunkSize; + DataBlock* mChunkHead; - /// Free all allocated memory blocks. - /// - /// This invalidates all pointers returned from alloc(). - void freeBlocks(bool keepOne = false); - - /// Initialize using blocks of a given size. - /// - /// One new block is allocated at constructor-time. - /// - /// @param size Size in bytes of the space to allocate for each block. - DataChunker(S32 size=ChunkSize); - ~DataChunker(); - - /// Swaps the memory allocated in one data chunker for another. This can be used to implement - /// packing of memory stored in a DataChunker. - void swap(DataChunker &d) - { - DataBlock *temp = d.mCurBlock; - d.mCurBlock = mCurBlock; - mCurBlock = temp; - } - public: + + BaseDataChunker(U32 chunkSize = BaseDataChunker::ChunkSize) : mChunkSize(chunkSize), mChunkHead(NULL) + { + } + + virtual ~BaseDataChunker() + { + freeBlocks(false); + } + + DataBlock* allocChunk(dsize_t chunkSize) + { + DataBlock* newChunk = (DataBlock*)dMalloc(sizeof(DataBlock) + chunkSize); + constructInPlace(newChunk); + newChunk->initWithBytes((T*)newChunk->getEnd(), chunkSize); + newChunk->mNext = mChunkHead; + mChunkHead = newChunk; + return newChunk; + } + + void* alloc(dsize_t numBytes) + { + void* theAlloc = mChunkHead ? mChunkHead->allocBytes(numBytes) : NULL; + if (theAlloc == NULL) + { + dsize_t actualSize = std::max(mChunkSize, numBytes); + allocChunk(actualSize); + theAlloc = mChunkHead->allocBytes(numBytes); + AssertFatal(theAlloc != NULL, "Something really odd going on here"); + } + return theAlloc; + } + + void freeBlocks(bool keepOne = false) + { + DataBlock* itr = mChunkHead; + while (itr) + { + DataBlock* nextItr = itr->mNext; + if (nextItr == NULL && keepOne) + { + itr->setPosition(0); + break; + } + dFree(itr); + itr = nextItr; + } + mChunkHead = itr; + } + U32 countUsedBlocks() { U32 count = 0; - if (!mCurBlock) - return 0; - for (DataBlock *ptr = mCurBlock; ptr != NULL; ptr = ptr->next) + for (DataBlock* itr = mChunkHead; itr; itr = itr->mNext) { count++; } return count; } - - void setChunkSize(U32 size) + + dsize_t countUsedBytes() { - AssertFatal(mCurBlock == NULL, "Cant resize now"); + dsize_t count = 0; + for (DataBlock* itr = mChunkHead; itr; itr = itr->mNext) + { + count += itr->getPositionBytes(); + } + return count; + } + + void setChunkSize(dsize_t size) + { + AssertFatal(mChunkHead == NULL, "Tried setting AFTER init"); mChunkSize = size; } +}; - +class DataChunker : public BaseDataChunker +{ public: - - DataBlock* mCurBlock; ///< current page we're allocating data from. If the - ///< data size request is greater than the memory space currently - ///< available in the current page, a new page will be allocated. - S32 mChunkSize; ///< The size allocated for each page in the DataChunker + DataChunker() : BaseDataChunker(BaseDataChunker::ChunkSize) { ; } + explicit DataChunker(dsize_t size) : BaseDataChunker(size) { ; } }; -inline U8 *DataChunker::DataBlock::getData() -{ - return (U8*)this + DataChunker::PaddDBSize; -} -//---------------------------------------------------------------------------- - -template -class Chunker: private DataChunker +/// Implements a derivative of BaseDataChunker designed for +/// allocating structs of type T without initialization. +template class Chunker : private BaseDataChunker { public: - Chunker(S32 size = DataChunker::ChunkSize) : DataChunker(size) {}; - T* alloc() { return reinterpret_cast(DataChunker::alloc(S32(sizeof(T)))); } - void clear() { freeBlocks(); } + Chunker(dsize_t size = BaseDataChunker::ChunkSize) : BaseDataChunker(std::max(sizeof(T), size)) + { + } + + T* alloc() + { + return (T*)BaseDataChunker::alloc(sizeof(T)); + } + + void clear() + { + BaseDataChunker::freeBlocks(); + } }; -//---------------------------------------------------------------------------- -/// This class is similar to the Chunker<> class above. But it allows for multiple -/// types of structs to be stored. -/// CodeReview: This could potentially go into DataChunker directly, but I wasn't sure if -/// CodeReview: That would be polluting it. BTR -class MultiTypedChunker : private DataChunker +/// Implements a derivative of BaseDataChunker designed for +/// allocating structs of various types Y without initialization. +/// @note: this is horribly suboptimal for types not multiples of uintptr_t in size. +class MultiTypedChunker : private BaseDataChunker { public: - MultiTypedChunker(S32 size = DataChunker::ChunkSize) : DataChunker(size) {}; + MultiTypedChunker(dsize_t size = BaseDataChunker::ChunkSize) : BaseDataChunker(std::max(sizeof(uintptr_t), size)) + { + } - /// Use like so: MyType* t = chunker.alloc(); - template - T* alloc() { return reinterpret_cast(DataChunker::alloc(S32(sizeof(T)))); } - void clear() { freeBlocks(true); } + template Y* alloc() + { + return (Y*)BaseDataChunker::alloc(sizeof(Y)); + } + + void clear() + { + BaseDataChunker::freeBlocks(true); + } }; -//---------------------------------------------------------------------------- - -/// Templatized data chunker class with proper construction and destruction of its elements. -/// -/// DataChunker just allocates space. This subclass actually constructs/destructs the -/// elements. This class is appropriate for more complex classes. -template -class ClassChunker: private DataChunker +/// Implements a simple linked list for ClassChunker and FreeListChunker. +template struct ChunkerFreeClassList { -public: - ClassChunker(S32 size = DataChunker::ChunkSize) : DataChunker(size) + ChunkerFreeClassList* mNextList; + + ChunkerFreeClassList() : mNextList(NULL) { - mElementSize = getMax(U32(sizeof(T)), U32(sizeof(T *))); - mFreeListHead = NULL; } - /// Allocates and properly constructs in place a new element. - T *alloc() + void reset() { - if(mFreeListHead == NULL) - return constructInPlace(reinterpret_cast(DataChunker::alloc(mElementSize))); - T* ret = mFreeListHead; - mFreeListHead = *(reinterpret_cast(mFreeListHead)); - return constructInPlace(ret); + mNextList = NULL; } - /// Properly destructs and frees an element allocated with the alloc method. - void free(T* elem) + bool isEmpty() { - destructInPlace(elem); - *(reinterpret_cast(elem)) = mFreeListHead; - mFreeListHead = elem; + return mNextList == NULL; } - void freeBlocks( bool keepOne = false ) - { - DataChunker::freeBlocks( keepOne ); - mFreeListHead = NULL; + T* pop() + { + ChunkerFreeClassList* oldNext = mNextList; + mNextList = mNextList ? mNextList->mNextList : NULL; + return (T*)oldNext; } -private: - S32 mElementSize; ///< the size of each element, or the size of a pointer, whichever is greater - T *mFreeListHead; ///< a pointer to a linked list of freed elements for reuse + void push(ChunkerFreeClassList* other) + { + other->mNextList = mNextList; + mNextList = other; + } }; -//---------------------------------------------------------------------------- - -template -class FreeListChunker +/// Implements a derivative of BaseDataChunker designed for +/// allocating structs or classes of type T with initialization. +template class ClassChunker : private BaseDataChunker { +protected: + ChunkerFreeClassList mFreeListHead; + public: - FreeListChunker(DataChunker *inChunker) - : mChunker( inChunker ), - mOwnChunker( false ), - mFreeListHead( NULL ) + ClassChunker(dsize_t size = BaseDataChunker::ChunkSize) { - mElementSize = getMax(U32(sizeof(T)), U32(sizeof(T *))); + } - FreeListChunker(S32 size = DataChunker::ChunkSize) - : mFreeListHead( NULL ) + T* alloc() { - mChunker = new DataChunker( size ); - mOwnChunker = true; - - mElementSize = getMax(U32(sizeof(T)), U32(sizeof(T *))); + if (mFreeListHead.isEmpty()) + { + return constructInPlace((T*)BaseDataChunker::alloc(sizeof(T))); + } + else + { + return constructInPlace(mFreeListHead.pop()); + } } - ~FreeListChunker() + void free(T* item) { - if ( mOwnChunker ) - delete mChunker; + destructInPlace(item); + mFreeListHead.push(reinterpret_cast*>(item)); } - T *alloc() + void freeBlocks(bool keepOne=false) { - if(mFreeListHead == NULL) - return reinterpret_cast(mChunker->alloc(mElementSize)); - T* ret = mFreeListHead; - mFreeListHead = *(reinterpret_cast(mFreeListHead)); - return ret; + BaseDataChunker::freeBlocks(keepOne); } - - void free(T* elem) - { - *(reinterpret_cast(elem)) = mFreeListHead; - mFreeListHead = elem; - } - - /// Allow people to free all their memory if they want. - void freeBlocks( bool keepOne = false ) - { - mChunker->freeBlocks( keepOne ); - mFreeListHead = NULL; - } - -private: - DataChunker *mChunker; - bool mOwnChunker; - - S32 mElementSize; - T *mFreeListHead; }; - -class FreeListChunkerUntyped +/// Implements a chunker which uses the data of another BaseDataChunker +/// as underlying storage. +template class FreeListChunker { +protected: + BaseDataChunker* mChunker; + bool mOwnsChunker; + ChunkerFreeClassList mFreeListHead; + public: - FreeListChunkerUntyped(U32 inElementSize, DataChunker *inChunker) - : mChunker( inChunker ), - mOwnChunker( false ), - mElementSize( inElementSize ), - mFreeListHead( NULL ) + FreeListChunker(BaseDataChunker* otherChunker) : + mChunker(otherChunker), + mOwnsChunker(false) { } - FreeListChunkerUntyped(U32 inElementSize, S32 size = DataChunker::ChunkSize) - : mElementSize( inElementSize ), - mFreeListHead( NULL ) + FreeListChunker(dsize_t size = BaseDataChunker::ChunkSize) { - mChunker = new DataChunker( size ); - mOwnChunker = true; + mChunker = new BaseDataChunker(size); + mOwnsChunker = true; } - ~FreeListChunkerUntyped() + BaseDataChunker* getChunker() { - if ( mOwnChunker ) - delete mChunker; + return mChunker; } - void *alloc() + T* alloc() { - if(mFreeListHead == NULL) - return mChunker->alloc(mElementSize); - - void *ret = mFreeListHead; - mFreeListHead = *(reinterpret_cast(mFreeListHead)); - return ret; + if (mFreeListHead.isEmpty()) + { + return constructInPlace((T*)mChunker->alloc(sizeof(T))); + } + else + { + return constructInPlace(mFreeListHead.pop()); + } } - void free(void* elem) + void free(T* item) { - *(reinterpret_cast(elem)) = mFreeListHead; - mFreeListHead = elem; + destructInPlace(item); + mFreeListHead.push(reinterpret_cast*>(item)); } - // Allow people to free all their memory if they want. - void freeBlocks() + void freeBlocks(bool keepOne) { - mChunker->freeBlocks(); - - // We have to terminate the freelist as well or else we'll run - // into crazy unused memory. - mFreeListHead = NULL; + BaseDataChunker::freeBlocks(keepOne); } - - U32 getElementSize() const { return mElementSize; } - -private: - DataChunker *mChunker; - bool mOwnChunker; - - const U32 mElementSize; - void *mFreeListHead; }; -#endif diff --git a/Engine/source/core/frameAllocator.cpp b/Engine/source/core/frameAllocator.cpp index 5c9530271..7f0435194 100644 --- a/Engine/source/core/frameAllocator.cpp +++ b/Engine/source/core/frameAllocator.cpp @@ -23,15 +23,21 @@ #include "core/frameAllocator.h" #include "console/engineAPI.h" -U8* FrameAllocator::smBuffer = NULL; -U32 FrameAllocator::smWaterMark = 0; -U32 FrameAllocator::smHighWaterMark = 0; +thread_local FrameAllocator::FrameAllocatorType FrameAllocator::smMainInstance; + +#ifdef TORQUE_MEM_DEBUG +thread_local dsize_t FrameAllocator::smAllocatedBytes; +#endif + +#if defined(TORQUE_DEBUG) + +dsize_t FrameAllocator::smMaxFrameAllocation; + + +DefineEngineFunction(getMaxFrameAllocation, S32, (), , "") +{ + return (S32)FrameAllocator::smMaxFrameAllocation; +} + -#ifdef TORQUE_DEBUG -U32 FrameAllocator::smMaxFrameAllocation = 0; - -DefineEngineFunction(getMaxFrameAllocation, S32, (),,"") -{ - return FrameAllocator::getMaxFrameAllocation(); -} #endif diff --git a/Engine/source/core/frameAllocator.h b/Engine/source/core/frameAllocator.h index 98bf8b110..807359946 100644 --- a/Engine/source/core/frameAllocator.h +++ b/Engine/source/core/frameAllocator.h @@ -1,5 +1,5 @@ //----------------------------------------------------------------------------- -// Copyright (c) 2012 GarageGames, LLC +// Copyright (c) 2013 GarageGames, LLC // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to @@ -27,14 +27,112 @@ #include "platform/platform.h" #endif -/// This #define is used by the FrameAllocator to align starting addresses to -/// be byte aligned to this value. This is important on the 360 and possibly -/// on other platforms as well. Use this #define anywhere alignment is needed. -/// -/// NOTE: Do not change this value per-platform unless you have a very good -/// reason for doing so. It has the potential to cause inconsistencies in -/// memory which is allocated and expected to be contiguous. -#define FRAMEALLOCATOR_BYTE_ALIGNMENT 4 +template class AlignedBufferAllocator +{ +protected: + T* mBuffer; + U32 mHighWaterMark; + U32 mWaterMark; + +public: + + typedef T ValueType; + + AlignedBufferAllocator() : mBuffer(NULL), mHighWaterMark(0), mWaterMark(0) + { + } + + inline void initWithElements(T* ptr, U32 numElements) + { + mBuffer = ptr; + mHighWaterMark = numElements; + mWaterMark = 0; + } + + inline void initWithBytes(T* ptr, dsize_t bytes) + { + mBuffer = ptr; + mHighWaterMark = (U32)(calcMaxElementSize(bytes)); + mWaterMark = 0; + } + + inline T* allocBytes(const size_t numBytes) + { + T* ptr = &mBuffer[mWaterMark]; + size_t numElements = calcRequiredElementSize(numBytes); + if (((size_t)mWaterMark + (size_t)numElements) > (size_t)mHighWaterMark) // safety check + { +#ifdef TORQUE_MEM_DEBUG + AssertFatal(false, "Overflow"); +#endif + return NULL; + } + mWaterMark += (U32)numElements; + return ptr; + } + + inline T* allocElements(const U32 numElements) + { + T* ptr = &mBuffer[mWaterMark]; + if (((size_t)mWaterMark + (size_t)numElements) > (size_t)mHighWaterMark) // safety check + { +#ifdef TORQUE_MEM_DEBUG + AssertFatal(false, "Overflow"); +#endif + return NULL; + } + mWaterMark += numElements; + return ptr; + } + + inline void setPosition(const U32 waterMark) + { + AssertFatal(waterMark <= mHighWaterMark, "Error, invalid waterMark"); + mWaterMark = waterMark; + } + + /// Calculates maximum elements required to store numBytes bytes (may overshoot) + static inline U32 calcRequiredElementSize(const dsize_t numBytes) + { + return (U32)((numBytes + (sizeof(T) - 1)) / sizeof(T)); + } + + /// Calculates maximum elements required to store numBytes bytes + static inline U32 calcMaxElementSize(const dsize_t numBytes) + { + return (U32)(numBytes / sizeof(T)); + } + + inline T* getAlignedBuffer() const + { + return mBuffer; + } + + inline U32 getPosition() const + { + return mWaterMark; + } + + inline U32 getSize() const + { + return mHighWaterMark; + } + + inline U32 getElementsLeft() const + { + return mHighWaterMark - mWaterMark; + } + + inline dsize_t getPositionBytes() const + { + return mWaterMark * sizeof(T); + } + + inline dsize_t getSizeBytes() const + { + return mHighWaterMark * sizeof(T); + } +}; /// Temporary memory pool for per-frame allocations. /// @@ -54,91 +152,77 @@ /// @endcode class FrameAllocator { - static U8* smBuffer; - static U32 smHighWaterMark; - static U32 smWaterMark; +public: + static dsize_t smMaxFrameAllocation; +#ifdef TORQUE_MEM_DEBUG + static thread_local dsize_t smAllocatedBytes; +#endif + typedef AlignedBufferAllocator FrameAllocatorType; -#ifdef TORQUE_DEBUG - static U32 smMaxFrameAllocation; + inline static void init(const U32 frameSize) + { + FrameAllocatorType::ValueType* curPtr = smMainInstance.getAlignedBuffer(); + AssertFatal(curPtr == NULL, "Error, already initialized"); + if (curPtr) + return; + +#ifdef TORQUE_MEM_DEBUG + smAllocatedBytes = 0; #endif - public: - inline static void init(const U32 frameSize); - inline static void destroy(); + U32 elementSize = FrameAllocatorType::calcRequiredElementSize(frameSize); + FrameAllocatorType::ValueType* newAlignedBuffer = new FrameAllocatorType::ValueType[elementSize]; + smMainInstance.initWithElements(newAlignedBuffer, elementSize); + } - inline static void* alloc(const U32 allocSize); + inline static void destroy() + { + FrameAllocatorType::ValueType* curPtr = smMainInstance.getAlignedBuffer(); + AssertFatal(smMainInstance.getAlignedBuffer() != NULL, "Error, not initialized"); + if (curPtr == NULL) + return; - inline static void setWaterMark(const U32); - inline static U32 getWaterMark(); - inline static U32 getHighWaterMark(); + delete[] curPtr; + smMainInstance.initWithElements(NULL, 0); + } -#ifdef TORQUE_DEBUG - static U32 getMaxFrameAllocation() { return smMaxFrameAllocation; } + inline static void* alloc(const U32 allocSize) + { + void* outPtr = smMainInstance.allocBytes(allocSize); + +#ifdef TORQUE_MEM_DEBUG + smAllocatedBytes += allocSize; + if (smAllocatedBytes > smMaxFrameAllocation) + { + smMaxFrameAllocation = smAllocatedBytes; + } #endif + + return outPtr; + } + + inline static void setWaterMark(const U32 waterMark) + { +#ifdef TORQUE_MEM_DEBUG + AssertFatal(waterMark % sizeof(FrameAllocatorType::ValueType) == 0, "Misaligned watermark"); + smAllocatedBytes = waterMark; +#endif + smMainInstance.setPosition(waterMark / sizeof(FrameAllocatorType::ValueType)); + } + + inline static U32 getWaterMark() + { + return smMainInstance.getPositionBytes(); + } + + inline static U32 getHighWaterMark() + { + return smMainInstance.getSizeBytes(); + } + + static thread_local FrameAllocatorType smMainInstance; }; -void FrameAllocator::init(const U32 frameSize) -{ -#ifdef FRAMEALLOCATOR_DEBUG_GUARD - AssertISV( false, "FRAMEALLOCATOR_DEBUG_GUARD has been removed because it allows non-contiguous memory allocation by the FrameAllocator, and this is *not* ok." ); -#endif - - AssertFatal(smBuffer == NULL, "Error, already initialized"); - smBuffer = new U8[frameSize]; - smWaterMark = 0; - smHighWaterMark = frameSize; -} - -void FrameAllocator::destroy() -{ - AssertFatal(smBuffer != NULL, "Error, not initialized"); - - delete [] smBuffer; - smBuffer = NULL; - smWaterMark = 0; - smHighWaterMark = 0; -} - - -void* FrameAllocator::alloc(const U32 allocSize) -{ - U32 _allocSize = allocSize; - - AssertFatal(smBuffer != NULL, "Error, no buffer!"); - AssertFatal(smWaterMark + _allocSize <= smHighWaterMark, "Error alloc too large, increase frame size!"); - smWaterMark = ( smWaterMark + ( FRAMEALLOCATOR_BYTE_ALIGNMENT - 1 ) ) & (~( FRAMEALLOCATOR_BYTE_ALIGNMENT - 1 )); - - // Sanity check. - AssertFatal( !( smWaterMark & ( FRAMEALLOCATOR_BYTE_ALIGNMENT - 1 ) ), "Frame allocation is not on a specified byte boundry." ); - - U8* p = &smBuffer[smWaterMark]; - smWaterMark += _allocSize; - -#ifdef TORQUE_DEBUG - if (smWaterMark > smMaxFrameAllocation) - smMaxFrameAllocation = smWaterMark; -#endif - - return p; -} - - -void FrameAllocator::setWaterMark(const U32 waterMark) -{ - AssertFatal(waterMark < smHighWaterMark, "Error, invalid waterMark"); - smWaterMark = waterMark; -} - -U32 FrameAllocator::getWaterMark() -{ - return smWaterMark; -} - -U32 FrameAllocator::getHighWaterMark() -{ - return smHighWaterMark; -} - /// Helper class to deal with FrameAllocator usage. /// /// The purpose of this class is to make it simpler and more reliable to use the @@ -173,19 +257,13 @@ public: { return FrameAllocator::alloc(allocSize); } - - template - T* alloc(const U32 numElements) const - { - return reinterpret_cast(FrameAllocator::alloc(numElements * sizeof(T))); - } }; /// Class for temporary variables that you want to allocate easily using /// the FrameAllocator. For example: /// @code /// FrameTemp tempStr(32); // NOTE! This parameter is NOT THE SIZE IN BYTES. See constructor docs. -/// dStrcat( tempStr, SomeOtherString, 32 * sizeof(char) ); +/// dStrcat( tempStr, SomeOtherString ); /// tempStr[2] = 'l'; /// Con::printf( tempStr ); /// Con::printf( "Foo: %s", ~tempStr ); @@ -193,7 +271,7 @@ public: /// /// This will automatically handle getting and restoring the watermark of the /// FrameAllocator when it goes out of scope. You should notice the strange -/// operator in front of tempStr on the printf call. This is normally a unary +/// operator infront of tempStr on the printf call. This is normally a unary /// operator for ones-complement, but in this class it will simply return the /// memory of the allocation. It's the same as doing (const char *)tempStr /// in the above case. The reason why it is necessary for the second printf @@ -203,13 +281,16 @@ public: /// /// @note It is important to note that this object is designed to just be a /// temporary array of a dynamic size. Some wierdness may occur if you try -/// to perform crazy pointer stuff with it using regular operators on it. +/// do perform crazy pointer stuff with it using regular operators on it. +/// I implemented what I thought were the most common operators that it +/// would be used for. If strange things happen, you will need to debug +/// them yourself. template class FrameTemp { protected: U32 mWaterMark; - T *mMemory; + T* mMemory; U32 mNumObjectsInMemory; public: @@ -228,26 +309,28 @@ public: /// @endcode /// /// @param count The number of objects to allocate - FrameTemp( const U32 count = 1 ) : mNumObjectsInMemory( count ) + FrameTemp(const U32 count = 1) : mNumObjectsInMemory(count) { - AssertFatal( count > 0, "Allocating a FrameTemp with less than one instance" ); + AssertFatal(count > 0, "Allocating a FrameTemp with less than one instance"); mWaterMark = FrameAllocator::getWaterMark(); - mMemory = reinterpret_cast( FrameAllocator::alloc( sizeof( T ) * count ) ); + mMemory = reinterpret_cast(FrameAllocator::alloc(sizeof(T) * count)); - for( S32 i = 0; i < mNumObjectsInMemory; i++ ) - constructInPlace( &mMemory[i] ); + for (U32 i = 0; i < mNumObjectsInMemory; i++) + constructInPlace(&mMemory[i]); } /// Destructor restores the watermark ~FrameTemp() { // Call destructor - for( S32 i = 0; i < mNumObjectsInMemory; i++ ) - destructInPlace( &mMemory[i] ); + for (U32 i = 0; i < mNumObjectsInMemory; i++) + destructInPlace(&mMemory[i]); - FrameAllocator::setWaterMark( mWaterMark ); + FrameAllocator::setWaterMark(mWaterMark); } + U32 getObjectCount(void) const { return mNumObjectsInMemory; } + /// NOTE: This will return the memory, NOT perform a ones-complement T* operator ~() { return mMemory; }; /// NOTE: This will return the memory, NOT perform a ones-complement @@ -264,44 +347,40 @@ public: T** operator &() { return &mMemory; }; const T** operator &() const { return &mMemory; }; - operator T*() { return mMemory; } - operator const T*() const { return mMemory; } + operator T* () { return mMemory; } + operator const T* () const { return mMemory; } - operator T&() { return *mMemory; } - operator const T&() const { return *mMemory; } + operator T& () { return *mMemory; } + operator const T& () const { return *mMemory; } operator T() { return *mMemory; } - operator const T() const { return *mMemory; } - - T& operator []( U32 i ) { return mMemory[ i ]; } - const T& operator []( U32 i ) const { return mMemory[ i ]; } + operator const T() const { return *mMemory; - T& operator []( S32 i ) { return mMemory[ i ]; } - const T& operator []( S32 i ) const { return mMemory[ i ]; } + inline T* address() const { return mMemory; } - /// @name Vector-like Interface - /// @{ - T *address() const { return mMemory; } - dsize_t size() const { return mNumObjectsInMemory; } - /// @} + // This ifdef is to satisfy the ever so pedantic GCC compiler + // Which seems to upset visual studio. + T& operator[](const U32 idx) { return mMemory[idx]; } + const T& operator[](const U32 idx) const { return mMemory[idx]; } + T& operator[](const S32 idx) { return mMemory[idx]; } + const T& operator[](const S32 idx) const { return mMemory[idx]; } }; //----------------------------------------------------------------------------- // FrameTemp specializations for types with no constructor/destructor #define FRAME_TEMP_NC_SPEC(type) \ - template<> \ - inline FrameTemp::FrameTemp( const U32 count ) \ - { \ - AssertFatal( count > 0, "Allocating a FrameTemp with less than one instance" ); \ - mWaterMark = FrameAllocator::getWaterMark(); \ - mMemory = reinterpret_cast( FrameAllocator::alloc( sizeof( type ) * count ) ); \ - mNumObjectsInMemory = 0; \ - } \ - template<>\ - inline FrameTemp::~FrameTemp() \ - { \ - FrameAllocator::setWaterMark( mWaterMark ); \ - } \ +template<> \ +inline FrameTemp::FrameTemp( const U32 count ) \ +{ \ +AssertFatal( count > 0, "Allocating a FrameTemp with less than one instance" ); \ +mWaterMark = FrameAllocator::getWaterMark(); \ +mMemory = reinterpret_cast( FrameAllocator::alloc( sizeof( type ) * count ) ); \ +} \ +template<>\ +inline FrameTemp::~FrameTemp() \ +{ \ +FrameAllocator::setWaterMark( mWaterMark ); \ +} \ FRAME_TEMP_NC_SPEC(char); FRAME_TEMP_NC_SPEC(float); diff --git a/Engine/source/core/resource.h b/Engine/source/core/resource.h index 393e3f372..ad5c81e59 100644 --- a/Engine/source/core/resource.h +++ b/Engine/source/core/resource.h @@ -62,6 +62,7 @@ class ResourceHolderBase public: static FreeListChunker smHolderFactory; + ResourceHolderBase() : mRes(NULL) { ; } // @note this is needed for the chunked allocator virtual ~ResourceHolderBase() {} // Return void pointer to resource data. From 3781c7fae5b3d0dda485102afbc1b259206bd0ef Mon Sep 17 00:00:00 2001 From: James Urquhart Date: Sun, 10 Dec 2023 11:57:08 +0000 Subject: [PATCH 4/7] Add an alternate allocator for DecalManager; Also fix SFX weirdness. --- Engine/source/T3D/decal/decalInstance.h | 7 ++ Engine/source/T3D/decal/decalManager.cpp | 52 +---------- Engine/source/T3D/decal/decalManager.h | 6 +- Engine/source/T3D/sfx/sfx3DWorld.cpp | 7 ++ Engine/source/T3D/sfx/sfx3DWorld.h | 2 + Engine/source/T3D/sfx/sfxSpace.h | 4 + Engine/source/core/dataChunker.cpp | 82 ----------------- Engine/source/core/dataChunker.h | 109 ++++++++++++++++++++++- Engine/source/core/frameAllocator.h | 48 ++-------- Engine/source/core/util/swizzle.h | 4 +- Engine/source/sfx/sfxSoundscape.cpp | 7 ++ Engine/source/sfx/sfxSoundscape.h | 3 + 12 files changed, 146 insertions(+), 185 deletions(-) diff --git a/Engine/source/T3D/decal/decalInstance.h b/Engine/source/T3D/decal/decalInstance.h index 9365f4a3a..c13fb6ad9 100644 --- a/Engine/source/T3D/decal/decalInstance.h +++ b/Engine/source/T3D/decal/decalInstance.h @@ -43,6 +43,13 @@ class DecalInstance { public: + typedef DWordDataBlob<256> SizeClass1; + typedef DWordDataBlob<512> SizeClass2; + typedef DWordDataBlob<1024> SizeClass3; + typedef ThreeTieredChunker DecalDataChunker; + + DecalDataChunker::Handle mAllocHandle; + DecalData *mDataBlock; Point3F mPosition; diff --git a/Engine/source/T3D/decal/decalManager.cpp b/Engine/source/T3D/decal/decalManager.cpp index bf1e7ad2a..39e90f159 100644 --- a/Engine/source/T3D/decal/decalManager.cpp +++ b/Engine/source/T3D/decal/decalManager.cpp @@ -200,17 +200,6 @@ S32 QSORT_CALLBACK cmpDecalRenderOrder( const void *p1, const void *p2 ) } // namespace {} -// These numbers should be tweaked to get as many dynamically placed decals -// as possible to allocate buffer arrays with the FreeListChunker. -enum -{ - SIZE_CLASS_0 = 256, - SIZE_CLASS_1 = 512, - SIZE_CLASS_2 = 1024, - - NUM_SIZE_CLASSES = 3 -}; - //------------------------------------------------------------------------- // DecalManager //------------------------------------------------------------------------- @@ -228,10 +217,6 @@ DecalManager::DecalManager() mDirty = false; - mChunkers[0] = new FreeListChunkerUntyped( SIZE_CLASS_0 * sizeof( U8 ) ); - mChunkers[1] = new FreeListChunkerUntyped( SIZE_CLASS_1 * sizeof( U8 ) ); - mChunkers[2] = new FreeListChunkerUntyped( SIZE_CLASS_2 * sizeof( U8 ) ); - GFXDevice::getDeviceEventSignal().notify(this, &DecalManager::_handleGFXEvent); } @@ -240,9 +225,6 @@ DecalManager::~DecalManager() GFXDevice::getDeviceEventSignal().remove(this, &DecalManager::_handleGFXEvent); clearData(); - - for( U32 i = 0; i < NUM_SIZE_CLASSES; ++ i ) - delete mChunkers[ i ]; } void DecalManager::consoleInit() @@ -913,14 +895,9 @@ void DecalManager::_generateWindingOrder( const Point3F &cornerPoint, VectormVertCount + sizeof( U16 ) * inst->mIndxCount ); - else - data = mChunkers[sizeClass]->alloc(); + inst->mAllocHandle = mChunkers.alloc(sizeof(DecalVertex) * inst->mVertCount + sizeof(U16) * inst->mIndxCount); + U8* data = (U8*)inst->mAllocHandle.ptr; inst->mVerts = reinterpret_cast< DecalVertex* >( data ); data = (U8*)data + sizeof( DecalVertex ) * inst->mVertCount; inst->mIndices = reinterpret_cast< U16* >( data ); @@ -930,15 +907,7 @@ void DecalManager::_freeBuffers( DecalInstance *inst ) { if ( inst->mVerts != NULL ) { - const S32 sizeClass = _getSizeClass( inst ); - - if ( sizeClass == -1 ) - dFree( inst->mVerts ); - else - { - // Use FreeListChunker - mChunkers[sizeClass]->free( inst->mVerts ); - } + mChunkers.free(inst->mAllocHandle); inst->mVerts = NULL; inst->mVertCount = 0; @@ -974,21 +943,6 @@ void DecalManager::_freePools() } } -S32 DecalManager::_getSizeClass( DecalInstance *inst ) const -{ - U32 bytes = inst->mVertCount * sizeof( DecalVertex ) + inst->mIndxCount * sizeof ( U16 ); - - if ( bytes <= SIZE_CLASS_0 ) - return 0; - if ( bytes <= SIZE_CLASS_1 ) - return 1; - if ( bytes <= SIZE_CLASS_2 ) - return 2; - - // Size is outside of the largest chunker. - return -1; -} - void DecalManager::prepRenderImage( SceneRenderState* state ) { PROFILE_SCOPE( DecalManager_RenderDecals ); diff --git a/Engine/source/T3D/decal/decalManager.h b/Engine/source/T3D/decal/decalManager.h index 4ab636f06..9dee7bd8a 100644 --- a/Engine/source/T3D/decal/decalManager.h +++ b/Engine/source/T3D/decal/decalManager.h @@ -110,7 +110,7 @@ class DecalManager : public SceneObject Vector< GFXVertexBufferHandle* > mVBPool; Vector< GFXPrimitiveBufferHandle* > mPBPool; - FreeListChunkerUntyped *mChunkers[3]; + DecalInstance::DecalDataChunker mChunkers; #ifdef DECALMANAGER_DEBUG Vector mDebugPlanes; @@ -167,10 +167,6 @@ class DecalManager : public SceneObject void _freeBuffers( DecalInstance *inst ); void _freePools(); - /// Returns index used to index into the correct sized FreeListChunker for - /// allocating vertex and index arrays. - S32 _getSizeClass( DecalInstance *inst ) const; - // Hide this from Doxygen /// @cond bool _handleGFXEvent(GFXDevice::GFXDeviceEventType event); diff --git a/Engine/source/T3D/sfx/sfx3DWorld.cpp b/Engine/source/T3D/sfx/sfx3DWorld.cpp index 1246895d0..4fe3e64cc 100644 --- a/Engine/source/T3D/sfx/sfx3DWorld.cpp +++ b/Engine/source/T3D/sfx/sfx3DWorld.cpp @@ -62,6 +62,13 @@ SFX3DWorld* gSFX3DWorld; //----------------------------------------------------------------------------- +SFX3DObject::SFX3DObject() + : Parent(NULL, NULL) +{ +} + +//----------------------------------------------------------------------------- + SFX3DObject::SFX3DObject( SFX3DWorld* world, SceneObject* object ) : Parent( world, object ) { diff --git a/Engine/source/T3D/sfx/sfx3DWorld.h b/Engine/source/T3D/sfx/sfx3DWorld.h index 86f233b33..66b3e1dd1 100644 --- a/Engine/source/T3D/sfx/sfx3DWorld.h +++ b/Engine/source/T3D/sfx/sfx3DWorld.h @@ -46,6 +46,8 @@ class SFX3DObject : public SceneObjectLink, public SFXObject< 3 > public: typedef SceneObjectLink Parent; + + SFX3DObject(); /// SFX3DObject( SFX3DWorld* world, SceneObject* object ); diff --git a/Engine/source/T3D/sfx/sfxSpace.h b/Engine/source/T3D/sfx/sfxSpace.h index d43339c30..73f3a3bc2 100644 --- a/Engine/source/T3D/sfx/sfxSpace.h +++ b/Engine/source/T3D/sfx/sfxSpace.h @@ -27,6 +27,10 @@ #include "scene/sceneSpace.h" #endif +#ifndef _SFXSOURCE_H_ +#include "sfx/sfxSource.h" +#endif + #ifndef _SCENEAMBIENTSOUNDOBJECT_H_ #include "scene/mixin/sceneAmbientSoundObject.h" #endif diff --git a/Engine/source/core/dataChunker.cpp b/Engine/source/core/dataChunker.cpp index bd46c3a87..756133b98 100644 --- a/Engine/source/core/dataChunker.cpp +++ b/Engine/source/core/dataChunker.cpp @@ -22,85 +22,3 @@ #include "platform/platform.h" #include "core/dataChunker.h" - - -//---------------------------------------------------------------------------- - -DataChunker::DataChunker(S32 size) -{ - mChunkSize = size; - mCurBlock = NULL; -} - -DataChunker::~DataChunker() -{ - freeBlocks(); -} - -void *DataChunker::alloc(S32 size) -{ - if (size > mChunkSize) - { - DataBlock * temp = (DataBlock*)dMalloc(DataChunker::PaddDBSize + size); - AssertFatal(temp, "Malloc failed"); - constructInPlace(temp); - if (mCurBlock) - { - temp->next = mCurBlock->next; - mCurBlock->next = temp; - } - else - { - mCurBlock = temp; - temp->curIndex = mChunkSize; - } - return temp->getData(); - } - - if(!mCurBlock || size + mCurBlock->curIndex > mChunkSize) - { - const U32 paddDBSize = (sizeof(DataBlock) + 3) & ~3; - DataBlock *temp = (DataBlock*)dMalloc(paddDBSize+ mChunkSize); - AssertFatal(temp, "Malloc failed"); - constructInPlace(temp); - temp->next = mCurBlock; - mCurBlock = temp; - } - - void *ret = mCurBlock->getData() + mCurBlock->curIndex; - mCurBlock->curIndex += (size + 3) & ~3; // dword align - return ret; -} - -DataChunker::DataBlock::DataBlock() -{ - curIndex = 0; - next = NULL; -} - -DataChunker::DataBlock::~DataBlock() -{ -} - -void DataChunker::freeBlocks(bool keepOne) -{ - while (mCurBlock && mCurBlock->next) - { - DataBlock* temp = mCurBlock->next; - dFree(mCurBlock); - mCurBlock = temp; - } - - if (!keepOne) - { - if (mCurBlock) - dFree(mCurBlock); - - mCurBlock = NULL; - } - else if (mCurBlock) - { - mCurBlock->curIndex = 0; - mCurBlock->next = NULL; - } -} diff --git a/Engine/source/core/dataChunker.h b/Engine/source/core/dataChunker.h index 247793650..6e4dafefb 100644 --- a/Engine/source/core/dataChunker.h +++ b/Engine/source/core/dataChunker.h @@ -14,10 +14,13 @@ #ifndef _PLATFORMASSERT_H_ # include "platform/platformAssert.h" #endif +#ifndef _FRAMEALLOCATOR_H_ +#include "core/frameAllocator.h" +#endif #include #include -#include "core/frameAllocator.h" + //#include "math/mMathFn.h" // tgemit - needed here for the moment /// Implements a chunked data allocator. @@ -242,7 +245,7 @@ public: mFreeListHead.push(reinterpret_cast*>(item)); } - void freeBlocks(bool keepOne=false) + void freeBlocks(bool keepOne = false) { BaseDataChunker::freeBlocks(keepOne); } @@ -293,8 +296,106 @@ public: mFreeListHead.push(reinterpret_cast*>(item)); } - void freeBlocks(bool keepOne) + void freeBlocks(bool keepOne = false) { - BaseDataChunker::freeBlocks(keepOne); + mChunker->freeBlocks(keepOne); + } +}; + +template struct DWordDataBlob +{ + U32 data[(byteSize + 3)/ 4]; +}; + +/// Implements a three-tiered chunker +/// K1..3 should be ordered from low to high +template class ThreeTieredChunker +{ +public: + struct Handle + { + U32 tier; + void* ptr; + + Handle() : tier(0), ptr(NULL) { ; } + Handle(const Handle& other) : tier(other.tier), ptr(other.ptr) { ; } + Handle(U32 in_tier, void* in_ptr) : tier(in_tier), ptr(in_ptr) { ; } + + Handle& operator=(const Handle& other) { + tier = other.tier; + ptr = other.ptr; + return *this; + } + }; + +protected: + + ClassChunker mT1; + ClassChunker mT2; + ClassChunker mT3; + +public: + + Handle alloc(U32 byteSize) + { + Handle outH; + + if (byteSize > sizeof(K3)) + { + const U32 wordSize = (byteSize + 3) / 4; + outH = Handle(0, (void*)(new U32[wordSize])); + } + else + { + if (byteSize <= sizeof(K1)) + { + outH = Handle(1, (void*)mT1.alloc()); + } + else if (byteSize <= sizeof(K2)) + { + outH = Handle(2, (void*)mT2.alloc()); + } + else if (byteSize <= sizeof(K3)) + { + outH = Handle(3, (void*)mT3.alloc()); + } + else + { + outH = Handle(0, NULL); + } + } + + return outH; + } + + void free(Handle& item) + { + if (item.ptr == NULL) + return; + + switch (item.tier) + { + case 0: + delete[] ((U32*)item.ptr); + break; + case 1: + mT1.free((K1*)item.ptr); + break; + case 2: + mT2.free((K2*)item.ptr); + break; + case 3: + mT3.free((K3*)item.ptr); + break; + default: + break; + } + } + + void freeBlocks(bool keepOne = false) + { + mT1.freeBlocks(keepOne); + mT2.freeBlocks(keepOne); + mT3.freeBlocks(keepOne); } }; diff --git a/Engine/source/core/frameAllocator.h b/Engine/source/core/frameAllocator.h index 807359946..0b70528f6 100644 --- a/Engine/source/core/frameAllocator.h +++ b/Engine/source/core/frameAllocator.h @@ -330,22 +330,13 @@ public: } U32 getObjectCount(void) const { return mNumObjectsInMemory; } - - /// NOTE: This will return the memory, NOT perform a ones-complement - T* operator ~() { return mMemory; }; - /// NOTE: This will return the memory, NOT perform a ones-complement - const T* operator ~() const { return mMemory; }; - - /// NOTE: This will dereference the memory, NOT do standard unary plus behavior - T& operator +() { return *mMemory; }; - /// NOTE: This will dereference the memory, NOT do standard unary plus behavior - const T& operator +() const { return *mMemory; }; + U32 size(void) const { return mNumObjectsInMemory; } T& operator *() { return *mMemory; }; - const T& operator *() const { return *mMemory; }; + const T& operator *() const { return *mMemory; } - T** operator &() { return &mMemory; }; - const T** operator &() const { return &mMemory; }; + T** operator &() { return &mMemory; } + const T** operator &() const { return &mMemory; } operator T* () { return mMemory; } operator const T* () const { return mMemory; } @@ -354,7 +345,7 @@ public: operator const T& () const { return *mMemory; } operator T() { return *mMemory; } - operator const T() const { return *mMemory; + operator const T() const { return *mMemory; } inline T* address() const { return mMemory; } @@ -366,35 +357,6 @@ public: const T& operator[](const S32 idx) const { return mMemory[idx]; } }; -//----------------------------------------------------------------------------- -// FrameTemp specializations for types with no constructor/destructor -#define FRAME_TEMP_NC_SPEC(type) \ -template<> \ -inline FrameTemp::FrameTemp( const U32 count ) \ -{ \ -AssertFatal( count > 0, "Allocating a FrameTemp with less than one instance" ); \ -mWaterMark = FrameAllocator::getWaterMark(); \ -mMemory = reinterpret_cast( FrameAllocator::alloc( sizeof( type ) * count ) ); \ -} \ -template<>\ -inline FrameTemp::~FrameTemp() \ -{ \ -FrameAllocator::setWaterMark( mWaterMark ); \ -} \ - -FRAME_TEMP_NC_SPEC(char); -FRAME_TEMP_NC_SPEC(float); -FRAME_TEMP_NC_SPEC(double); -FRAME_TEMP_NC_SPEC(bool); -FRAME_TEMP_NC_SPEC(int); -FRAME_TEMP_NC_SPEC(short); - -FRAME_TEMP_NC_SPEC(unsigned char); -FRAME_TEMP_NC_SPEC(unsigned int); -FRAME_TEMP_NC_SPEC(unsigned short); - -#undef FRAME_TEMP_NC_SPEC - //----------------------------------------------------------------------------- #endif // _H_FRAMEALLOCATOR_ diff --git a/Engine/source/core/util/swizzle.h b/Engine/source/core/util/swizzle.h index 9185f7b53..f8038f624 100644 --- a/Engine/source/core/util/swizzle.h +++ b/Engine/source/core/util/swizzle.h @@ -144,8 +144,8 @@ inline void Swizzle::InPlace( void *memory, const dsize_t size ) c // FrameTemp should work because the PNG loading code uses the FrameAllocator, so // it should only get used on an image w/ that size as max -patw FrameTemp buffer( size ); - dMemcpy( ~buffer, memory, size ); - ToBuffer( memory, ~buffer, size ); + dMemcpy( buffer.address(), memory, size); + ToBuffer( memory, buffer.address(), size); } } diff --git a/Engine/source/sfx/sfxSoundscape.cpp b/Engine/source/sfx/sfxSoundscape.cpp index c90015ffc..b65c573d7 100644 --- a/Engine/source/sfx/sfxSoundscape.cpp +++ b/Engine/source/sfx/sfxSoundscape.cpp @@ -39,6 +39,13 @@ //----------------------------------------------------------------------------- +SFXSoundscape::SFXSoundscape() + : mAmbience( NULL ) +{ +} + +//----------------------------------------------------------------------------- + SFXSoundscape::SFXSoundscape( SFXAmbience* ambience ) : mAmbience( ambience ) { diff --git a/Engine/source/sfx/sfxSoundscape.h b/Engine/source/sfx/sfxSoundscape.h index 603a84917..586a4a2b3 100644 --- a/Engine/source/sfx/sfxSoundscape.h +++ b/Engine/source/sfx/sfxSoundscape.h @@ -106,6 +106,9 @@ class SFXSoundscape bool _isUnique() const { return mFlags.test( FlagUnique ); } public: + + /// Defaault constructor for allocator + SFXSoundscape(); /// Create a soundscape associated with the given ambient space. SFXSoundscape( SFXAmbience* ambience ); From 7332dd66436a7b013834cd3774b727a880f9ea67 Mon Sep 17 00:00:00 2001 From: James Urquhart Date: Mon, 5 Feb 2024 22:53:09 +0000 Subject: [PATCH 5/7] Add tests for FrameAllocator and DataChunker --- Engine/source/core/dataChunker.h | 33 +- Engine/source/core/frameAllocator.cpp | 13 +- Engine/source/core/frameAllocator.h | 18 +- Engine/source/testing/dataChunkerTest.cpp | 347 +++++++++++++++++++ Engine/source/testing/frameAllocatorTest.cpp | 195 +++++++++++ 5 files changed, 597 insertions(+), 9 deletions(-) create mode 100644 Engine/source/testing/dataChunkerTest.cpp create mode 100644 Engine/source/testing/frameAllocatorTest.cpp diff --git a/Engine/source/core/dataChunker.h b/Engine/source/core/dataChunker.h index 6e4dafefb..8112ef1d1 100644 --- a/Engine/source/core/dataChunker.h +++ b/Engine/source/core/dataChunker.h @@ -39,6 +39,8 @@ public: ChunkSize = 16384 }; + typedef T AlignmentType; + struct alignas(uintptr_t) DataBlock : public AlignedBufferAllocator { DataBlock* mNext; @@ -129,6 +131,19 @@ public: AssertFatal(mChunkHead == NULL, "Tried setting AFTER init"); mChunkSize = size; } + + bool isManagedByChunker(void* ptr) const + { + U8* chkPtr = (U8*)ptr; + for (DataBlock* itr = mChunkHead; itr; itr = itr->mNext) + { + const U8* blockStart = (U8*)itr->getAlignedBuffer(); + const U8* blockEnd = (U8*)itr->getAlignedBufferEnd(); + if (chkPtr >= blockStart && chkPtr < blockEnd) + return true; + } + return false; + } }; class DataChunker : public BaseDataChunker @@ -166,6 +181,8 @@ public: class MultiTypedChunker : private BaseDataChunker { public: + typedef uintptr_t AlignmentType; + MultiTypedChunker(dsize_t size = BaseDataChunker::ChunkSize) : BaseDataChunker(std::max(sizeof(uintptr_t), size)) { } @@ -195,7 +212,7 @@ template struct ChunkerFreeClassList mNextList = NULL; } - bool isEmpty() + bool isEmpty() const { return mNextList == NULL; } @@ -248,7 +265,15 @@ public: void freeBlocks(bool keepOne = false) { BaseDataChunker::freeBlocks(keepOne); + mFreeListHead.reset(); } + + inline bool isManagedByChunker(void* ptr) const + { + return BaseDataChunker::isManagedByChunker(ptr); + } + + inline ChunkerFreeClassList& getFreeListHead() { return mFreeListHead; } }; /// Implements a chunker which uses the data of another BaseDataChunker @@ -390,6 +415,8 @@ public: default: break; } + + item.ptr = NULL; } void freeBlocks(bool keepOne = false) @@ -398,4 +425,8 @@ public: mT2.freeBlocks(keepOne); mT3.freeBlocks(keepOne); } + + inline ClassChunker& getT1Chunker() { return mT1; } + inline ClassChunker& getT2Chunker() { return mT2; } + inline ClassChunker& getT3Chunker() { return mT3; } }; diff --git a/Engine/source/core/frameAllocator.cpp b/Engine/source/core/frameAllocator.cpp index 7f0435194..b1e276e8d 100644 --- a/Engine/source/core/frameAllocator.cpp +++ b/Engine/source/core/frameAllocator.cpp @@ -29,15 +29,18 @@ thread_local FrameAllocator::FrameAllocatorType FrameAllocator::smMainInstance thread_local dsize_t FrameAllocator::smAllocatedBytes; #endif -#if defined(TORQUE_DEBUG) +U32 FrameAllocator::smMaxFrameAllocation; -dsize_t FrameAllocator::smMaxFrameAllocation; - - -DefineEngineFunction(getMaxFrameAllocation, S32, (), , "") +U32 FrameAllocator::getMaxFrameAllocation() { return (S32)FrameAllocator::smMaxFrameAllocation; } +#if defined(TORQUE_DEBUG) + +DefineEngineFunction(getMaxFrameAllocation, S32, (), , "") +{ + return (S32)FrameAllocator::getMaxFrameAllocation(); +} #endif diff --git a/Engine/source/core/frameAllocator.h b/Engine/source/core/frameAllocator.h index 0b70528f6..9613cb7ad 100644 --- a/Engine/source/core/frameAllocator.h +++ b/Engine/source/core/frameAllocator.h @@ -103,11 +103,21 @@ public: return (U32)(numBytes / sizeof(T)); } + static inline U32 calcRequiredPaddedByteSize(const dsize_t numBytes) + { + return calcRequiredElementSize(numBytes) * sizeof(T); + } + inline T* getAlignedBuffer() const { return mBuffer; } + inline T* getAlignedBufferEnd() const + { + return mBuffer + mHighWaterMark; + } + inline U32 getPosition() const { return mWaterMark; @@ -153,7 +163,7 @@ public: class FrameAllocator { public: - static dsize_t smMaxFrameAllocation; + static U32 smMaxFrameAllocation; #ifdef TORQUE_MEM_DEBUG static thread_local dsize_t smAllocatedBytes; #endif @@ -220,6 +230,8 @@ public: return smMainInstance.getSizeBytes(); } + static U32 getMaxFrameAllocation(); + static thread_local FrameAllocatorType smMainInstance; }; @@ -253,7 +265,7 @@ public: FrameAllocator::setWaterMark(mMarker); } - void* alloc(const U32 allocSize) const + void* alloc(const U32 allocSize) { return FrameAllocator::alloc(allocSize); } @@ -336,7 +348,7 @@ public: const T& operator *() const { return *mMemory; } T** operator &() { return &mMemory; } - const T** operator &() const { return &mMemory; } + T* const * operator &() const { return &mMemory; } operator T* () { return mMemory; } operator const T* () const { return mMemory; } diff --git a/Engine/source/testing/dataChunkerTest.cpp b/Engine/source/testing/dataChunkerTest.cpp new file mode 100644 index 000000000..a66bd634f --- /dev/null +++ b/Engine/source/testing/dataChunkerTest.cpp @@ -0,0 +1,347 @@ +//----------------------------------------------------------------------------- +// Copyright (c) 2023-2024 tgemit contributors. +// See AUTHORS file and git repository for contributor information. +// +// SPDX-License-Identifier: MIT +//----------------------------------------------------------------------------- + +#ifdef TORQUE_TESTS_ENABLED +#include "testing/unitTesting.h" +#include "core/dataChunker.h" + +struct TestClassChunkerStruct +{ + U32 value; + U32 value2; + + TestClassChunkerStruct() + { + value = 0xC001B33F; + value2 = 0x10101010; + } + + ~TestClassChunkerStruct() + { + value = 0; + value2 = 0; + } +}; + + +TEST(BaseDataChunkerTest, BaseDataChunker_Should_Function_Correctly) +{ + BaseDataChunker testChunks(1024); + BaseDataChunker testChunk4(1024); + BaseDataChunker testChunk8(1024); + + EXPECT_TRUE(testChunks.countUsedBlocks() == 0); + EXPECT_TRUE(testChunk4.countUsedBlocks() == 0); + EXPECT_TRUE(testChunk8.countUsedBlocks() == 0); + + testChunks.alloc(1); + testChunk4.alloc(1); + testChunk8.alloc(1); + + EXPECT_TRUE(testChunks.countUsedBlocks() == 1); + EXPECT_TRUE(testChunk4.countUsedBlocks() == 1); + EXPECT_TRUE(testChunk8.countUsedBlocks() == 1); + + testChunks.alloc(1); + testChunk4.alloc(1); + testChunk8.alloc(1); + + EXPECT_TRUE(testChunks.countUsedBlocks() == 1); + EXPECT_TRUE(testChunk4.countUsedBlocks() == 1); + EXPECT_TRUE(testChunk8.countUsedBlocks() == 1); + + EXPECT_TRUE(testChunks.countUsedBytes() == (sizeof(TestClassChunkerStruct) * 2)); + EXPECT_TRUE(testChunk4.countUsedBytes() == (sizeof(U32) * 2)); + EXPECT_TRUE(testChunk8.countUsedBytes() == (sizeof(U64) * 2)); + + testChunks.freeBlocks(true); + testChunk4.freeBlocks(true); + testChunk8.freeBlocks(true); + + EXPECT_TRUE(testChunks.countUsedBlocks() == 1); + EXPECT_TRUE(testChunk4.countUsedBlocks() == 1); + EXPECT_TRUE(testChunk8.countUsedBlocks() == 1); + + testChunks.freeBlocks(false); + testChunk4.freeBlocks(false); + testChunk8.freeBlocks(false); + + EXPECT_TRUE(testChunks.countUsedBlocks() == 0); + EXPECT_TRUE(testChunk4.countUsedBlocks() == 0); + EXPECT_TRUE(testChunk8.countUsedBlocks() == 0); + + testChunks.setChunkSize(sizeof(TestClassChunkerStruct)); + testChunks.alloc(1); + EXPECT_TRUE(testChunks.countUsedBlocks() == 1); + testChunks.alloc(1); + EXPECT_TRUE(testChunks.countUsedBlocks() == 2); +} + +TEST(DataChunkerTest, DataChunker_Should_Function_Correctly) +{ + DataChunker testChunk(1024); + + testChunk.alloc(1024); + + EXPECT_TRUE(testChunk.countUsedBlocks() == 1); + + testChunk.alloc(1024); + + EXPECT_TRUE(testChunk.countUsedBlocks() == 2); + + testChunk.alloc(4096); + + EXPECT_TRUE(testChunk.countUsedBytes() == (1024+1024+4096)); + + EXPECT_TRUE(testChunk.countUsedBlocks() == 3); + + testChunk.alloc(12); + + EXPECT_TRUE(testChunk.countUsedBlocks() == 4); + + testChunk.alloc(12); + + EXPECT_TRUE(testChunk.countUsedBlocks() == 4); + + U32 reqEls = AlignedBufferAllocator::calcRequiredElementSize(12) * sizeof(uintptr_t); + + EXPECT_TRUE(testChunk.countUsedBytes() == (1024+1024+4096+reqEls+reqEls)); + + testChunk.freeBlocks(true); + EXPECT_TRUE(testChunk.countUsedBlocks() == 1); + testChunk.freeBlocks(false); + EXPECT_TRUE(testChunk.countUsedBlocks() == 0); + + // Large block cases + + testChunk.alloc(8192); + EXPECT_TRUE(testChunk.countUsedBlocks() == 1); + testChunk.freeBlocks(true); + EXPECT_TRUE(testChunk.countUsedBlocks() == 1); + + testChunk.alloc(8192); + testChunk.alloc(1024); + EXPECT_TRUE(testChunk.countUsedBlocks() == 2); + testChunk.freeBlocks(true); + EXPECT_TRUE(testChunk.countUsedBlocks() == 1); + testChunk.freeBlocks(false); + EXPECT_TRUE(testChunk.countUsedBlocks() == 0); + + // Instead using the chunk size + + for (U32 i=0; i<8; i++) + { + testChunk.alloc(1024); + } + EXPECT_TRUE(testChunk.countUsedBlocks() == 8); + testChunk.freeBlocks(false); + EXPECT_TRUE(testChunk.countUsedBlocks() == 0); +} + +TEST(ChunkerTest,Chunker_Should_Function_Correctly) +{ + Chunker foo; + TestClassChunkerStruct* value = foo.alloc(); + EXPECT_TRUE(value->value != 0xC001B33F); + EXPECT_TRUE(value->value2 != 0x10101010); + // Should otherwise just act like DataChunker +} + +TEST(MultiTypedChunkerTest,MultiTypedChunker_Should_Function_Correctly) +{ + struct TVS1 + { + int a; + int b; + }; + struct TVS2 + { + int a; + int b; + int c; + }; + MultiTypedChunker chunker; + TVS1* v1 = chunker.alloc(); + TVS2* v2 = chunker.alloc(); + TVS2* v3 = chunker.alloc(); + + EXPECT_TRUE(((U8*)v2) - ((U8*)v1) == sizeof(TVS1)); + EXPECT_TRUE(((U8*)v3) - ((U8*)v2) == AlignedBufferAllocator::calcRequiredPaddedByteSize(sizeof(TVS2))); +} + +TEST(ChunkerFreeClassListTest,ChunkerFreeClassList_Should_Function_Correctly) +{ + TestClassChunkerStruct list[5]; + ChunkerFreeClassList freeListTest; + + // Push & pop works as expected + EXPECT_TRUE(freeListTest.isEmpty() == true); + freeListTest.push((ChunkerFreeClassList*)&list[0]); + EXPECT_TRUE(freeListTest.isEmpty() == false); + freeListTest.push((ChunkerFreeClassList*)&list[4]); + EXPECT_TRUE(freeListTest.pop() == &list[4]); + EXPECT_TRUE(freeListTest.pop() == &list[0]); + EXPECT_TRUE(freeListTest.pop() == NULL); + + // Reset clears list head + freeListTest.push((ChunkerFreeClassList*)&list[4]); + freeListTest.reset(); + EXPECT_TRUE(freeListTest.pop() == NULL); +} + + +TEST(FreeListChunkerTest, FreeListChunkerTest_Should_Function_Correctly) +{ + FreeListChunker testFreeList; + + TestClassChunkerStruct* s1 = testFreeList.alloc(); + TestClassChunkerStruct* s2 = testFreeList.alloc(); + + // Allocation is sequential + EXPECT_TRUE(s2 > s1); + EXPECT_TRUE(((s2 - s1) == 1)); + + testFreeList.free(s1); + + // But previous reallocations are reused + TestClassChunkerStruct* s3 = testFreeList.alloc(); + TestClassChunkerStruct* s4 = testFreeList.alloc(); + + EXPECT_TRUE(s1 == s3); + EXPECT_TRUE(((s4 - s2) == 1)); // continues from previous free alloc + + // Check sharing + + FreeListChunker sharedChunker(testFreeList.getChunker()); + + s2 = testFreeList.alloc(); + EXPECT_TRUE(((s2 - s4) == 1)); +} + +TEST(ClassChunkerTest, ClassChunker_Should_Function_Correctly) +{ + ClassChunker testClassList; + + TestClassChunkerStruct* s1 = testClassList.alloc(); + TestClassChunkerStruct* s2 = testClassList.alloc(); + + // Allocation is sequential + EXPECT_TRUE(s2 > s1); + EXPECT_TRUE(((s2 - s1) == 1)); + + testClassList.free(s1); + EXPECT_TRUE(s1->value == 0); + EXPECT_TRUE(s1->value2 == 0); + + // But previous reallocations are reused + TestClassChunkerStruct* s3 = testClassList.alloc(); + TestClassChunkerStruct* s4 = testClassList.alloc(); + + EXPECT_TRUE(s1 == s3); + EXPECT_TRUE(((s4 - s2) == 1)); // continues from previous free alloc + + // Values should be initialized correctly for all allocs at this point + EXPECT_TRUE(s1->value == 0xC001B33F); + EXPECT_TRUE(s1->value2 == 0x10101010); + EXPECT_TRUE(s2->value == 0xC001B33F); + EXPECT_TRUE(s2->value2 == 0x10101010); + EXPECT_TRUE(s3->value == 0xC001B33F); + EXPECT_TRUE(s3->value2 == 0x10101010); + EXPECT_TRUE(s4->value == 0xC001B33F); + EXPECT_TRUE(s4->value2 == 0x10101010); + + // Should still be valid if using freeBlocks + testClassList.freeBlocks(true); + EXPECT_TRUE(s1->value == 0xC001B33F); + EXPECT_TRUE(s1->value2 == 0x10101010); + EXPECT_TRUE(s2->value == 0xC001B33F); + EXPECT_TRUE(s2->value2 == 0x10101010); + EXPECT_TRUE(s3->value == 0xC001B33F); + EXPECT_TRUE(s3->value2 == 0x10101010); + EXPECT_TRUE(s4->value == 0xC001B33F); + EXPECT_TRUE(s4->value2 == 0x10101010); +} + + +TEST(ThreeTieredChunkerTest,ThreeTieredChunker_Should_Function_Correctly) +{ + struct TThreeSA + { + uintptr_t a; + }; + struct TThreeSB + { + uintptr_t a; + uintptr_t b; + }; + struct TThreeSC + { + uintptr_t a; + uintptr_t b; + uintptr_t c; + }; + struct TThreeSD + { + uintptr_t a; + uintptr_t b; + uintptr_t c; + uintptr_t d; + }; + ThreeTieredChunker threeChunker; + + // Alloc should alloc in the correct lists + + auto h1 = threeChunker.alloc(sizeof(TThreeSA)); + auto h2 = threeChunker.alloc(sizeof(TThreeSB)); + auto h3 = threeChunker.alloc(sizeof(TThreeSC)); + auto h4 = threeChunker.alloc(sizeof(TThreeSD)); + + EXPECT_TRUE(threeChunker.getT1Chunker().isManagedByChunker(h3.ptr) == false); + EXPECT_TRUE(threeChunker.getT2Chunker().isManagedByChunker(h3.ptr) == false); + EXPECT_TRUE(threeChunker.getT3Chunker().isManagedByChunker(h3.ptr) == true); + EXPECT_TRUE(h3.tier == 3); + + EXPECT_TRUE(threeChunker.getT1Chunker().isManagedByChunker(h2.ptr) == false); + EXPECT_TRUE(threeChunker.getT2Chunker().isManagedByChunker(h2.ptr) == true); + EXPECT_TRUE(threeChunker.getT3Chunker().isManagedByChunker(h2.ptr) == false); + EXPECT_TRUE(h2.tier == 2); + + EXPECT_TRUE(threeChunker.getT1Chunker().isManagedByChunker(h1.ptr) == true); + EXPECT_TRUE(threeChunker.getT2Chunker().isManagedByChunker(h1.ptr) == false); + EXPECT_TRUE(threeChunker.getT3Chunker().isManagedByChunker(h1.ptr) == false); + EXPECT_TRUE(h1.tier == 1); + + EXPECT_TRUE(threeChunker.getT1Chunker().isManagedByChunker(h4.ptr) == false); + EXPECT_TRUE(threeChunker.getT2Chunker().isManagedByChunker(h4.ptr) == false); + EXPECT_TRUE(threeChunker.getT3Chunker().isManagedByChunker(h4.ptr) == false); + EXPECT_TRUE(h4.tier == 0); + + threeChunker.free(h1); + threeChunker.free(h2); + threeChunker.free(h3); + threeChunker.free(h4); + + EXPECT_TRUE(h1.ptr == NULL); + EXPECT_TRUE(h2.ptr == NULL); + EXPECT_TRUE(h3.ptr == NULL); + EXPECT_TRUE(h4.ptr == NULL); + + // freeBlocks should also clear ALL the list heads + + EXPECT_FALSE(threeChunker.getT1Chunker().getFreeListHead().isEmpty()); + EXPECT_FALSE(threeChunker.getT2Chunker().getFreeListHead().isEmpty()); + EXPECT_FALSE(threeChunker.getT3Chunker().getFreeListHead().isEmpty()); + + threeChunker.freeBlocks(false); + + EXPECT_TRUE(threeChunker.getT1Chunker().getFreeListHead().isEmpty()); + EXPECT_TRUE(threeChunker.getT2Chunker().getFreeListHead().isEmpty()); + EXPECT_TRUE(threeChunker.getT3Chunker().getFreeListHead().isEmpty()); +} + + +#endif diff --git a/Engine/source/testing/frameAllocatorTest.cpp b/Engine/source/testing/frameAllocatorTest.cpp new file mode 100644 index 000000000..2790f5d49 --- /dev/null +++ b/Engine/source/testing/frameAllocatorTest.cpp @@ -0,0 +1,195 @@ +//----------------------------------------------------------------------------- +// Copyright (c) 2023-2024 tgemit contributors. +// See AUTHORS file and git repository for contributor information. +// +// SPDX-License-Identifier: MIT +//----------------------------------------------------------------------------- + +#ifdef TORQUE_TESTS_ENABLED +#include "testing/unitTesting.h" +#include "core/frameAllocator.h" + +struct TestAlignmentStruct +{ + U64 values[4]; +}; + +TEST(AlignedBufferAllocatorTest, AlignedBufferAllocator_Should_Function_Correctly) +{ + AlignedBufferAllocator ba4; + AlignedBufferAllocator ba8; + AlignedBufferAllocator bas; + + const U32 bufSize32 = (sizeof(TestAlignmentStruct) / 4) * 20; + U32 testAlignmentBuffer[bufSize32]; + for (U32 i=0; i fooTemp(20); + EXPECT_TRUE(FrameAllocator::getWaterMark() == sizeof(TestAlignmentStruct)*20); + EXPECT_TRUE(&fooTemp[0] == fooTemp.address()); + EXPECT_TRUE((&fooTemp[1] - &fooTemp[0]) == 1); + EXPECT_TRUE(fooTemp.getObjectCount() == 20); + EXPECT_TRUE(fooTemp.size() == 20); + + const FrameTemp& fooC = fooTemp; + EXPECT_TRUE(&fooC[0] == fooC.address()); + EXPECT_TRUE((&fooC[1] - &fooC[0]) == 1); + EXPECT_TRUE(fooC.getObjectCount() == 20); + EXPECT_TRUE(fooC.size() == 20); + + // Accessors should work + + // Call the overloaded operators by name + TestAlignmentStruct& value = fooTemp.operator*(); + TestAlignmentStruct** ptr = fooTemp.operator&(); + const TestAlignmentStruct* constPtr = fooTemp.operator const TestAlignmentStruct * (); + TestAlignmentStruct& ref = fooTemp.operator TestAlignmentStruct & (); + const TestAlignmentStruct& constRef = fooTemp.operator const TestAlignmentStruct & (); + TestAlignmentStruct copy = fooTemp.operator TestAlignmentStruct(); + + EXPECT_TRUE(*ptr == fooTemp.address()); + EXPECT_TRUE(&value == fooTemp.address()); + EXPECT_TRUE(constPtr == fooTemp.address()); + EXPECT_TRUE(&ref == fooTemp.address()); + EXPECT_TRUE(&constRef == fooTemp.address()); + EXPECT_TRUE(© != fooTemp.address()); + + // Same for fooC + const TestAlignmentStruct& Cvalue = fooC.operator*(); + TestAlignmentStruct* const* Cptr = fooC.operator&(); + const TestAlignmentStruct* CconstPtr = fooC.operator const TestAlignmentStruct * (); + const TestAlignmentStruct& CconstRef = fooC.operator const TestAlignmentStruct & (); + + EXPECT_TRUE(*Cptr == fooC.address()); + EXPECT_TRUE(&Cvalue == fooC.address()); + EXPECT_TRUE(CconstPtr == fooC.address()); + EXPECT_TRUE(&CconstRef == fooC.address()); + EXPECT_TRUE(&ref == fooC.address()); + EXPECT_TRUE(&constRef == fooC.address()); + } + + // Exiting scope sets watermark + EXPECT_TRUE(FrameAllocator::getWaterMark() == 0); + + // Test the destructor actually gets called + + struct FTDestructTest + { + ~FTDestructTest() + { + gFTDestructTest++; + } + }; + + { + gFTDestructTest = 0; + FrameTemp foo2Temp(10); + } + + EXPECT_TRUE(gFTDestructTest == 10); +} + + +#endif From 45898694e49bd49a5dc9365fbdc6fff8110f2c49 Mon Sep 17 00:00:00 2001 From: James Urquhart Date: Tue, 6 Feb 2024 02:30:47 +0000 Subject: [PATCH 6/7] Reimplement FrameAllocator and FrameTemp; Tidy up DataChunker header. - Also additional work on tests to reflect watermark behavior change --- Engine/source/core/dataChunker.h | 6 +- Engine/source/core/frameAllocator.cpp | 41 +-- Engine/source/core/frameAllocator.h | 324 ++++++++----------- Engine/source/testing/frameAllocatorTest.cpp | 10 +- 4 files changed, 154 insertions(+), 227 deletions(-) diff --git a/Engine/source/core/dataChunker.h b/Engine/source/core/dataChunker.h index 8112ef1d1..cefefa2fe 100644 --- a/Engine/source/core/dataChunker.h +++ b/Engine/source/core/dataChunker.h @@ -5,7 +5,7 @@ // SPDX-License-Identifier: MIT //----------------------------------------------------------------------------- -#pragma once +#ifndef _DATACHUNKER_H_ #define _DATACHUNKER_H_ #ifndef _PLATFORM_H_ @@ -21,8 +21,6 @@ #include #include -//#include "math/mMathFn.h" // tgemit - needed here for the moment - /// Implements a chunked data allocator. /// /// This memory allocator allocates data in chunks of bytes, @@ -430,3 +428,5 @@ public: inline ClassChunker& getT2Chunker() { return mT2; } inline ClassChunker& getT3Chunker() { return mT3; } }; + +#endif diff --git a/Engine/source/core/frameAllocator.cpp b/Engine/source/core/frameAllocator.cpp index b1e276e8d..4aee9bcdf 100644 --- a/Engine/source/core/frameAllocator.cpp +++ b/Engine/source/core/frameAllocator.cpp @@ -1,46 +1,15 @@ //----------------------------------------------------------------------------- -// Copyright (c) 2012 GarageGames, LLC +// Copyright (C) 2024 tgemit contributors. +// See AUTHORS file and git repository for contributor information. // -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to -// deal in the Software without restriction, including without limitation the -// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or -// sell copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING -// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS -// IN THE SOFTWARE. +// SPDX-License-Identifier: MIT //----------------------------------------------------------------------------- #include "core/frameAllocator.h" -#include "console/engineAPI.h" -thread_local FrameAllocator::FrameAllocatorType FrameAllocator::smMainInstance; +thread_local ManagedAlignedBufferAllocator FrameAllocator::smFrameAllocator; #ifdef TORQUE_MEM_DEBUG -thread_local dsize_t FrameAllocator::smAllocatedBytes; +thread_local dsize_t FrameAllocator::smMaxAllocationBytes = 0; #endif -U32 FrameAllocator::smMaxFrameAllocation; - -U32 FrameAllocator::getMaxFrameAllocation() -{ - return (S32)FrameAllocator::smMaxFrameAllocation; -} - -#if defined(TORQUE_DEBUG) - -DefineEngineFunction(getMaxFrameAllocation, S32, (), , "") -{ - return (S32)FrameAllocator::getMaxFrameAllocation(); -} - -#endif diff --git a/Engine/source/core/frameAllocator.h b/Engine/source/core/frameAllocator.h index 9613cb7ad..f9546dae9 100644 --- a/Engine/source/core/frameAllocator.h +++ b/Engine/source/core/frameAllocator.h @@ -1,23 +1,8 @@ //----------------------------------------------------------------------------- -// Copyright (c) 2013 GarageGames, LLC +// Copyright (C) 2023-2024 tgemit contributors. +// See AUTHORS file and git repository for contributor information. // -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to -// deal in the Software without restriction, including without limitation the -// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or -// sell copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING -// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS -// IN THE SOFTWARE. +// SPDX-License-Identifier: MIT //----------------------------------------------------------------------------- #ifndef _FRAMEALLOCATOR_H_ @@ -27,6 +12,19 @@ #include "platform/platform.h" #endif +/// Implements an buffer which allocates data based on the alignment of type T. +/// +/// Example usage: +/// +/// @code +/// AlignedBufferAllocator alloc32; +/// alloc32.initWithElements(new U32[10], 10); +/// +/// void* ptr = alloc32.allocBytes(2); +/// // Reset back to start +/// alloc32.setPosition(0); +/// @endcode +/// template class AlignedBufferAllocator { protected: @@ -42,6 +40,7 @@ public: { } + /// Inits allocator based on a ptr to a memory block of size numElements * sizeof(T) inline void initWithElements(T* ptr, U32 numElements) { mBuffer = ptr; @@ -49,6 +48,7 @@ public: mWaterMark = 0; } + /// Inits allocator based on a ptr to a memory block of size bytes inline void initWithBytes(T* ptr, dsize_t bytes) { mBuffer = ptr; @@ -56,6 +56,7 @@ public: mWaterMark = 0; } + /// Allocs numBytes worth of elements inline T* allocBytes(const size_t numBytes) { T* ptr = &mBuffer[mWaterMark]; @@ -71,6 +72,7 @@ public: return ptr; } + /// Allocates numElements elements inline T* allocElements(const U32 numElements) { T* ptr = &mBuffer[mWaterMark]; @@ -85,6 +87,7 @@ public: return ptr; } + /// Sets current aligned watermark position inline void setPosition(const U32 waterMark) { AssertFatal(waterMark <= mHighWaterMark, "Error, invalid waterMark"); @@ -144,231 +147,186 @@ public: } }; -/// Temporary memory pool for per-frame allocations. /// -/// In the course of rendering a frame, it is often necessary to allocate -/// many small chunks of memory, then free them all in a batch. For instance, -/// say we're allocating storage for some vertex calculations: +/// Implements an AlignedBufferAllocator which manages its own memory. /// -/// @code -/// // Get FrameAllocator memory... -/// U32 waterMark = FrameAllocator::getWaterMark(); -/// F32 * ptr = (F32*)FrameAllocator::alloc(sizeof(F32)*2*targetMesh->vertsPerFrame); -/// -/// ... calculations ... -/// -/// // Free frameAllocator memory -/// FrameAllocator::setWaterMark(waterMark); -/// @endcode -class FrameAllocator +template class ManagedAlignedBufferAllocator : public AlignedBufferAllocator { public: - static U32 smMaxFrameAllocation; -#ifdef TORQUE_MEM_DEBUG - static thread_local dsize_t smAllocatedBytes; -#endif - typedef AlignedBufferAllocator FrameAllocatorType; + T* mMemory; - inline static void init(const U32 frameSize) + ManagedAlignedBufferAllocator() : mMemory(NULL) { - FrameAllocatorType::ValueType* curPtr = smMainInstance.getAlignedBuffer(); - AssertFatal(curPtr == NULL, "Error, already initialized"); - if (curPtr) - return; - -#ifdef TORQUE_MEM_DEBUG - smAllocatedBytes = 0; -#endif - - U32 elementSize = FrameAllocatorType::calcRequiredElementSize(frameSize); - FrameAllocatorType::ValueType* newAlignedBuffer = new FrameAllocatorType::ValueType[elementSize]; - smMainInstance.initWithElements(newAlignedBuffer, elementSize); } - inline static void destroy() + ~ManagedAlignedBufferAllocator() { - FrameAllocatorType::ValueType* curPtr = smMainInstance.getAlignedBuffer(); - AssertFatal(smMainInstance.getAlignedBuffer() != NULL, "Error, not initialized"); - if (curPtr == NULL) - return; - - delete[] curPtr; - smMainInstance.initWithElements(NULL, 0); + destroy(); } - inline static void* alloc(const U32 allocSize) + void init(const dsize_t byteSize) { - void* outPtr = smMainInstance.allocBytes(allocSize); - -#ifdef TORQUE_MEM_DEBUG - smAllocatedBytes += allocSize; - if (smAllocatedBytes > smMaxFrameAllocation) - { - smMaxFrameAllocation = smAllocatedBytes; - } -#endif - - return outPtr; + AssertFatal(mMemory == NULL, "ManagedAlignedBufferAllocator already initialized"); + U32 frameSize = calcRequiredElementSize(byteSize); + mMemory = new U32[frameSize]; + initWithElements(mMemory, frameSize); } - inline static void setWaterMark(const U32 waterMark) + void destroy() { -#ifdef TORQUE_MEM_DEBUG - AssertFatal(waterMark % sizeof(FrameAllocatorType::ValueType) == 0, "Misaligned watermark"); - smAllocatedBytes = waterMark; -#endif - smMainInstance.setPosition(waterMark / sizeof(FrameAllocatorType::ValueType)); + //setPositition(0); + delete[] mMemory; + mMemory = NULL; } - - inline static U32 getWaterMark() - { - return smMainInstance.getPositionBytes(); - } - - inline static U32 getHighWaterMark() - { - return smMainInstance.getSizeBytes(); - } - - static U32 getMaxFrameAllocation(); - - static thread_local FrameAllocatorType smMainInstance; }; -/// Helper class to deal with FrameAllocator usage. -/// -/// The purpose of this class is to make it simpler and more reliable to use the -/// FrameAllocator. Simply use it like this: +/// Implements a thread-local global buffer for frame-based allocations. +/// All allocations are aligned to U32. +/// +/// Example usage: /// /// @code -/// FrameAllocatorMarker mem; -/// -/// char *buff = (char*)mem.alloc(100); +/// U32 waterMark = FrameAllocator::getWaterMark(); +/// void* ptr = FrameAllocator::alloc(10); +/// // Cleanup... +/// FrameAllocator::setWaterMark(waterMark); /// @endcode /// -/// When you leave the scope you defined the FrameAllocatorMarker in, it will -/// automatically restore the watermark on the FrameAllocator. In situations -/// with complex branches, this can be a significant headache remover, as you -/// don't have to remember to reset the FrameAllocator on every posssible branch. -class FrameAllocatorMarker +/// See also: FrameAllocatorMarker, FrameTemp. +/// +/// NOTE: worker threads which use FrameAllocator should call init and destroy. i.e. +/// +/// @code +/// FrameAllocator::init(1024 * 1024 * 12); +/// // Do work... +/// FrameAllocator::destroy(); +/// @endcode +/// +class FrameAllocator { - U32 mMarker; +protected: + + static thread_local ManagedAlignedBufferAllocator smFrameAllocator; +#ifdef TORQUE_MEM_DEBUG + static thread_local dsize_t smMaxAllocationBytes; +#endif public: + + inline TORQUE_FORCEINLINE static void init(const dsize_t byteSize) { return smFrameAllocator.init(byteSize); } + inline TORQUE_FORCEINLINE static void destroy() { smFrameAllocator.destroy(); } + + inline TORQUE_FORCEINLINE static void* alloc(const dsize_t numBytes) + { +#ifdef TORQUE_MEM_DEBUG + const dsize_t allocBytes = smFrameAllocator.getPositionBytes(); + smMaxAllocationBytes = allocBytes > smMaxAllocationBytes ? allocBytes : smMaxAllocationBytes; +#endif + return smFrameAllocator.allocBytes(numBytes); + } + + inline TORQUE_FORCEINLINE static U32 getWaterMark() { return smFrameAllocator.getPosition(); } + inline TORQUE_FORCEINLINE static dsize_t getWaterMarkBytes() { return smFrameAllocator.getPositionBytes(); } + inline TORQUE_FORCEINLINE static void setWaterMark(U32 pos) { return smFrameAllocator.setPosition(pos); } + + inline TORQUE_FORCEINLINE static U32 getHighWaterMark() { return smFrameAllocator.getSizeBytes(); } +}; + +/// Helper class which saves and restores the previous water mark level from FrameAllocator based on scope. +/// +/// Example usage: +/// +/// @code +/// FrameAllocatorMarker marker; +/// void* ptr = marker.alloc(1024); +/// @endcode +/// +class FrameAllocatorMarker +{ + U32 mPosition; + +public: + FrameAllocatorMarker() { - mMarker = FrameAllocator::getWaterMark(); + mPosition = FrameAllocator::getWaterMark(); } ~FrameAllocatorMarker() { - FrameAllocator::setWaterMark(mMarker); + FrameAllocator::setWaterMark(mPosition); } - void* alloc(const U32 allocSize) + /// Allocs numBytes of memory from FrameAllocator + inline TORQUE_FORCEINLINE static void* alloc(const dsize_t numBytes) { - return FrameAllocator::alloc(allocSize); + return FrameAllocator::alloc(numBytes); } }; -/// Class for temporary variables that you want to allocate easily using -/// the FrameAllocator. For example: +/// Helper class which temporarily allocates a set of elements of type T from FrameAllocator, +/// restoring the water mark when the scope has ended as with FrameAllocatorMarker. +/// +/// Example usage: +/// /// @code -/// FrameTemp tempStr(32); // NOTE! This parameter is NOT THE SIZE IN BYTES. See constructor docs. -/// dStrcat( tempStr, SomeOtherString ); -/// tempStr[2] = 'l'; -/// Con::printf( tempStr ); -/// Con::printf( "Foo: %s", ~tempStr ); +/// FrameTemp textMarker(64); +/// for (U32 i=0; i class FrameTemp { -protected: - U32 mWaterMark; - T* mMemory; - U32 mNumObjectsInMemory; + T* mData; + U32 mSize; + U32 mPosition; public: - /// Constructor will store the FrameAllocator watermark and allocate the memory off - /// of the FrameAllocator. - /// - /// @note It is important to note that, unlike the FrameAllocatorMarker and the - /// FrameAllocator itself, the argument to allocate is NOT the size in bytes, - /// doing: - /// @code - /// FrameTemp f64s(5); - /// @endcode - /// Is the same as - /// @code - /// F64 *f64s = new F64[5]; - /// @endcode - /// - /// @param count The number of objects to allocate - FrameTemp(const U32 count = 1) : mNumObjectsInMemory(count) - { - AssertFatal(count > 0, "Allocating a FrameTemp with less than one instance"); - mWaterMark = FrameAllocator::getWaterMark(); - mMemory = reinterpret_cast(FrameAllocator::alloc(sizeof(T) * count)); - for (U32 i = 0; i < mNumObjectsInMemory; i++) - constructInPlace(&mMemory[i]); + FrameTemp(const U32 numElements = 0) + { + mPosition = FrameAllocator::getWaterMark(); + mData = (T*)FrameAllocator::alloc(sizeof(T) * numElements); + mSize = numElements; } - /// Destructor restores the watermark ~FrameTemp() { - // Call destructor - for (U32 i = 0; i < mNumObjectsInMemory; i++) - destructInPlace(&mMemory[i]); - - FrameAllocator::setWaterMark(mWaterMark); + for (U32 i = 0; i < mSize; i++) + destructInPlace(&mData[i]); + FrameAllocator::setWaterMark(mPosition); } - U32 getObjectCount(void) const { return mNumObjectsInMemory; } - U32 size(void) const { return mNumObjectsInMemory; } + // Operators - T& operator *() { return *mMemory; }; - const T& operator *() const { return *mMemory; } + inline TORQUE_FORCEINLINE T& operator*() { return *mData; } + inline TORQUE_FORCEINLINE const T& operator*() const { return *mData; } - T** operator &() { return &mMemory; } - T* const * operator &() const { return &mMemory; } + inline TORQUE_FORCEINLINE T** operator&() { return &mData; } + inline TORQUE_FORCEINLINE T* const * operator&() const { return &mData; } - operator T* () { return mMemory; } - operator const T* () const { return mMemory; } + inline TORQUE_FORCEINLINE operator T&() { return *mData; } + inline TORQUE_FORCEINLINE operator const T&() const { return *mData; } - operator T& () { return *mMemory; } - operator const T& () const { return *mMemory; } + inline TORQUE_FORCEINLINE operator T* () { return mData; } + inline TORQUE_FORCEINLINE operator const T* () const { return mData; } - operator T() { return *mMemory; } - operator const T() const { return *mMemory; } + inline TORQUE_FORCEINLINE operator T () { return *mData; } + inline TORQUE_FORCEINLINE operator const T () const { return *mData; } - inline T* address() const { return mMemory; } + inline TORQUE_FORCEINLINE T& operator[](const dsize_t idx) { return mData[idx]; } + inline TORQUE_FORCEINLINE const T& operator[](const dsize_t idx) const { return mData[idx]; } - // This ifdef is to satisfy the ever so pedantic GCC compiler - // Which seems to upset visual studio. - T& operator[](const U32 idx) { return mMemory[idx]; } - const T& operator[](const U32 idx) const { return mMemory[idx]; } - T& operator[](const S32 idx) { return mMemory[idx]; } - const T& operator[](const S32 idx) const { return mMemory[idx]; } + // Utils + + inline TORQUE_FORCEINLINE T* address() const { return mData; } + inline TORQUE_FORCEINLINE const U32 size() const { return mSize; } + inline TORQUE_FORCEINLINE const U32 getObjectCount() const { return mSize; } }; -//----------------------------------------------------------------------------- #endif // _H_FRAMEALLOCATOR_ diff --git a/Engine/source/testing/frameAllocatorTest.cpp b/Engine/source/testing/frameAllocatorTest.cpp index 2790f5d49..0dab0c1a3 100644 --- a/Engine/source/testing/frameAllocatorTest.cpp +++ b/Engine/source/testing/frameAllocatorTest.cpp @@ -89,13 +89,13 @@ TEST(FrameAllocatorTest, FrameAllocator_Should_Function_Correctly) EXPECT_TRUE(FrameAllocator::getWaterMark() == 104); FrameAllocator::alloc(1); - EXPECT_TRUE(FrameAllocator::getWaterMark() == 108); + EXPECT_TRUE(FrameAllocator::getWaterMark() == 105); FrameAllocator::alloc(5); - EXPECT_TRUE(FrameAllocator::getWaterMark() == 116); + EXPECT_TRUE(FrameAllocator::getWaterMark() == 107); // 5 bytes == 2 ints FrameAllocator::setWaterMark(0); FrameAllocator::alloc(1); - EXPECT_TRUE(FrameAllocator::getWaterMark() == 4); + EXPECT_TRUE(FrameAllocator::getWaterMarkBytes() == 4); FrameAllocator::setWaterMark(0); } @@ -127,7 +127,7 @@ TEST(FrameTempTest, FrameTempShould_Function_Correctly) FrameAllocator::setWaterMark(0); { FrameTemp fooTemp(20); - EXPECT_TRUE(FrameAllocator::getWaterMark() == sizeof(TestAlignmentStruct)*20); + EXPECT_TRUE(FrameAllocator::getWaterMarkBytes() == sizeof(TestAlignmentStruct)*20); EXPECT_TRUE(&fooTemp[0] == fooTemp.address()); EXPECT_TRUE((&fooTemp[1] - &fooTemp[0]) == 1); EXPECT_TRUE(fooTemp.getObjectCount() == 20); @@ -171,7 +171,7 @@ TEST(FrameTempTest, FrameTempShould_Function_Correctly) } // Exiting scope sets watermark - EXPECT_TRUE(FrameAllocator::getWaterMark() == 0); + EXPECT_TRUE(FrameAllocator::getWaterMarkBytes() == 0); // Test the destructor actually gets called From 28ba2f247321b40bf4979b84d80ddfb7bc2532d5 Mon Sep 17 00:00:00 2001 From: James Urquhart Date: Wed, 7 Feb 2024 00:05:14 +0000 Subject: [PATCH 7/7] Fix gcc & clang Compile for FrameAllocator changes --- Engine/source/core/frameAllocator.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Engine/source/core/frameAllocator.h b/Engine/source/core/frameAllocator.h index f9546dae9..7d09f2b86 100644 --- a/Engine/source/core/frameAllocator.h +++ b/Engine/source/core/frameAllocator.h @@ -152,6 +152,7 @@ public: /// template class ManagedAlignedBufferAllocator : public AlignedBufferAllocator { +typedef AlignedBufferAllocator Parent; public: T* mMemory; @@ -167,14 +168,14 @@ public: void init(const dsize_t byteSize) { AssertFatal(mMemory == NULL, "ManagedAlignedBufferAllocator already initialized"); - U32 frameSize = calcRequiredElementSize(byteSize); + U32 frameSize = Parent::calcRequiredElementSize(byteSize); mMemory = new U32[frameSize]; - initWithElements(mMemory, frameSize); + AlignedBufferAllocator::initWithElements(mMemory, frameSize); } void destroy() { - //setPositition(0); + Parent::setPosition(0); delete[] mMemory; mMemory = NULL; }