From 1d6ed840fb9382c3234b719bbb97f72c836ac4d2 Mon Sep 17 00:00:00 2001 From: Florian Born Date: Wed, 20 Apr 2022 12:14:35 +0200 Subject: [PATCH 01/28] Replace single allocations in fbx loader by block allocation --- code/Common/StackAllocator.cpp | 84 ++++++++++++++++++++++++++++++++ code/Common/StackAllocator.h | 89 ++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 code/Common/StackAllocator.cpp create mode 100644 code/Common/StackAllocator.h diff --git a/code/Common/StackAllocator.cpp b/code/Common/StackAllocator.cpp new file mode 100644 index 000000000..c789099a2 --- /dev/null +++ b/code/Common/StackAllocator.cpp @@ -0,0 +1,84 @@ +/* +Open Asset Import Library (assimp) +---------------------------------------------------------------------- + +Copyright (c) 2006-2022, assimp team + + +All rights reserved. + +Redistribution and use of this software in source and binary forms, +with or without modification, are permitted provided that the +following conditions are met: + +* Redistributions of source code must retain the above + copyright notice, this list of conditions and the + following disclaimer. + +* Redistributions in binary form must reproduce the above + copyright notice, this list of conditions and the + following disclaimer in the documentation and/or other + materials provided with the distribution. + +* Neither the name of the assimp team, nor the names of its + contributors may be used to endorse or promote products + derived from this software without specific prior + written permission of the assimp team. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +---------------------------------------------------------------------- +*/ + +#include "StackAllocator.h" +#include + +using namespace Assimp; + + +StackAllocator::StackAllocator() { +} + +StackAllocator::~StackAllocator() { + FreeAll(); +} + +void *StackAllocator::Allocate(size_t byteSize) { + if (m_subIndex + byteSize > m_blockAllocationSize) // start a new block + { + // double block size every time, up to maximum of g_maxBytesPerBlock. + // Block size must be at least as large as byteSize, but we want to use this for small allocations anyway. + m_blockAllocationSize = std::max(std::min(m_blockAllocationSize * 2, g_maxBytesPerBlock), byteSize); + uint8_t *data = (uint8_t *)malloc(m_blockAllocationSize); + m_storageBlocks.push_back(data); + m_subIndex = byteSize; + return data; + } + + uint8_t *data = m_storageBlocks.back(); + data += m_subIndex; + m_subIndex += byteSize; + + return data; +} + +void StackAllocator::FreeAll() { + for (size_t i = 0; i < m_storageBlocks.size(); i++) { + free(m_storageBlocks[i]); + } + std::deque empty; + m_storageBlocks.swap(empty); + // start over: + m_blockAllocationSize = g_startBytesPerBlock; + m_subIndex = g_maxBytesPerBlock; +} diff --git a/code/Common/StackAllocator.h b/code/Common/StackAllocator.h new file mode 100644 index 000000000..b0f1ecfe1 --- /dev/null +++ b/code/Common/StackAllocator.h @@ -0,0 +1,89 @@ +/* +Open Asset Import Library (assimp) +---------------------------------------------------------------------- + +Copyright (c) 2006-2022, assimp team + + +All rights reserved. + +Redistribution and use of this software in source and binary forms, +with or without modification, are permitted provided that the +following conditions are met: + +* Redistributions of source code must retain the above + copyright notice, this list of conditions and the + following disclaimer. + +* Redistributions in binary form must reproduce the above + copyright notice, this list of conditions and the + following disclaimer in the documentation and/or other + materials provided with the distribution. + +* Neither the name of the assimp team, nor the names of its + contributors may be used to endorse or promote products + derived from this software without specific prior + written permission of the assimp team. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +---------------------------------------------------------------------- +*/ + +/** @file StackAllocator.h + * @brief A very bare-bone allocator class that is suitable when + * allocating many small objects, e.g. during parsing. + * Individual objects are not freed, instead only the whole memory + * can be deallocated. + */ +#ifndef AI_STACK_ALLOCATOR_H_INC +#define AI_STACK_ALLOCATOR_H_INC + + +#include + +namespace Assimp +{ + +/** @brief A very bare-bone allocator class that is suitable when + * allocating many small objects, e.g. during parsing. + * Individual objects are not freed, instead only the whole memory + * can be deallocated. +*/ +class StackAllocator { +public: + // Constructs the allocator + StackAllocator(); + // Destructs the allocator and frees all memory + ~StackAllocator(); + + // Returns a pointer to byteSize bytes of heap memory that persists + // for the lifetime of the allocator (or until FreeAll is called). + void *Allocate(size_t byteSize); + + // Releases all the memory owned by this allocator. + // Memory provided through function Allocate is not valid anymore after this function has been called. + void FreeAll(); + +private: + constexpr const static size_t g_maxBytesPerBlock = 64 * 1024 * 1024; // The maximum size (in bytes) of a block + constexpr const static size_t g_startBytesPerBlock = 16 * 1024; // Size of the first block. Next blocks will double in size until maximum size of g_maxBytesPerBlock + size_t m_blockAllocationSize = g_startBytesPerBlock; // Block size of the current block + size_t m_subIndex = g_maxBytesPerBlock; // The current byte offset in the current block + std::deque m_storageBlocks; // A list of blocks +}; + + +} // namespace Assimp + +#endif // include guard From 2b3c49cb93d404a023fe8b852ebdb3e331fa6a15 Mon Sep 17 00:00:00 2001 From: Florian Born Date: Wed, 20 Apr 2022 12:33:39 +0200 Subject: [PATCH 02/28] All allocation changes --- code/AssetLib/FBX/FBXBinaryTokenizer.cpp | 10 ++++------ code/AssetLib/FBX/FBXDocument.cpp | 23 ++++++++++------------- code/AssetLib/FBX/FBXDocument.h | 2 ++ code/AssetLib/FBX/FBXImporter.cpp | 10 ++++------ code/AssetLib/FBX/FBXParser.cpp | 20 ++++++++------------ code/AssetLib/FBX/FBXParser.h | 17 +++++++++++------ code/AssetLib/FBX/FBXTokenizer.cpp | 20 ++++++++++---------- code/AssetLib/FBX/FBXTokenizer.h | 7 ++++--- code/CMakeLists.txt | 2 ++ 9 files changed, 55 insertions(+), 56 deletions(-) diff --git a/code/AssetLib/FBX/FBXBinaryTokenizer.cpp b/code/AssetLib/FBX/FBXBinaryTokenizer.cpp index 1a4d11856..43154e2ab 100644 --- a/code/AssetLib/FBX/FBXBinaryTokenizer.cpp +++ b/code/AssetLib/FBX/FBXBinaryTokenizer.cpp @@ -341,8 +341,7 @@ void ReadData(const char*& sbegin_out, const char*& send_out, const char* input, // ------------------------------------------------------------------------------------------------ -bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, const char* end, bool const is64bits) -{ +bool ReadScope(TokenList &output_tokens, StackAllocator &token_allocator, const char *input, const char *&cursor, const char *end, bool const is64bits) { // the first word contains the offset at which this block ends const uint64_t end_offset = is64bits ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); @@ -408,7 +407,7 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, // XXX this is vulnerable to stack overflowing .. while(Offset(input, cursor) < end_offset - sentinel_block_length) { - ReadScope(output_tokens, input, cursor, input + end_offset - sentinel_block_length, is64bits); + ReadScope(output_tokens, token_allocator, input, cursor, input + end_offset - sentinel_block_length, is64bits); } output_tokens.push_back(new_Token(cursor, cursor + 1, TokenType_CLOSE_BRACKET, Offset(input, cursor) )); @@ -431,8 +430,7 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, // ------------------------------------------------------------------------------------------------ // TODO: Test FBX Binary files newer than the 7500 version to check if the 64 bits address behaviour is consistent -void TokenizeBinary(TokenList& output_tokens, const char* input, size_t length) -{ +void TokenizeBinary(TokenList &output_tokens, const char *input, size_t length, StackAllocator &token_allocator) { ai_assert(input); ASSIMP_LOG_DEBUG("Tokenizing binary FBX file"); @@ -465,7 +463,7 @@ void TokenizeBinary(TokenList& output_tokens, const char* input, size_t length) try { while (cursor < end ) { - if (!ReadScope(output_tokens, input, cursor, input + length, is64bits)) { + if (!ReadScope(output_tokens, token_allocator, input, cursor, input + length, is64bits)) { break; } } diff --git a/code/AssetLib/FBX/FBXDocument.cpp b/code/AssetLib/FBX/FBXDocument.cpp index b49ee625a..17b347f86 100644 --- a/code/AssetLib/FBX/FBXDocument.cpp +++ b/code/AssetLib/FBX/FBXDocument.cpp @@ -235,7 +235,7 @@ FileGlobalSettings::FileGlobalSettings(const Document &doc, std::shared_ptrCompound(); for(const ElementMap::value_type& el : sobjects.Elements()) { @@ -377,7 +372,7 @@ void Document::ReadObjects() { DOMWarning("encountered duplicate object id, ignoring first occurrence",el.second); } - objects[id] = new LazyObject(id, *el.second, *this); + objects[id] = new_LazyObject(id, *el.second, *this); // grab all animation stacks upfront since there is no listing of them if(!strcmp(el.first.c_str(),"AnimationStack")) { @@ -444,8 +439,10 @@ void Document::ReadPropertyTemplates() { } // ------------------------------------------------------------------------------------------------ -void Document::ReadConnections() { - const Scope& sc = parser.GetRootScope(); +void Document::ReadConnections() +{ + StackAllocator &allocator = parser.GetAllocator(); + const Scope &sc = parser.GetRootScope(); // read property templates from "Definitions" section const Element* const econns = sc["Connections"]; if(!econns || !econns->Compound()) { @@ -484,7 +481,7 @@ void Document::ReadConnections() { } // add new connection - const Connection* const c = new Connection(insertionOrder++,src,dest,prop,*this); + const Connection* const c = new_Connection(insertionOrder++,src,dest,prop,*this); src_connections.insert(ConnectionMap::value_type(src,c)); dest_connections.insert(ConnectionMap::value_type(dest,c)); } diff --git a/code/AssetLib/FBX/FBXDocument.h b/code/AssetLib/FBX/FBXDocument.h index c61a47410..142e870ff 100644 --- a/code/AssetLib/FBX/FBXDocument.h +++ b/code/AssetLib/FBX/FBXDocument.h @@ -80,6 +80,8 @@ class BlendShape; class Skin; class Cluster; +#define new_LazyObject new (allocator.Allocate(sizeof(LazyObject))) LazyObject +#define new_Connection new (allocator.Allocate(sizeof(Connection))) Connection /** Represents a delay-parsed FBX objects. Many objects in the scene * are not needed by assimp, so it makes no sense to parse them diff --git a/code/AssetLib/FBX/FBXImporter.cpp b/code/AssetLib/FBX/FBXImporter.cpp index 0f63acc8f..96c5a9f14 100644 --- a/code/AssetLib/FBX/FBXImporter.cpp +++ b/code/AssetLib/FBX/FBXImporter.cpp @@ -160,17 +160,18 @@ void FBXImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy TokenList tokens; try { + Assimp::StackAllocator tempAllocator; bool is_binary = false; if (!strncmp(begin, "Kaydara FBX Binary", 18)) { is_binary = true; - TokenizeBinary(tokens, begin, contents.size()); + TokenizeBinary(tokens, begin, contents.size(), tempAllocator); } else { - Tokenize(tokens, begin); + Tokenize(tokens, begin, tempAllocator); } // use this information to construct a very rudimentary // parse-tree representing the FBX scope structure - Parser parser(tokens, is_binary); + Parser parser(tokens, tempAllocator, is_binary); // take the raw parse-tree and convert it to a FBX DOM Document doc(parser, settings); @@ -189,10 +190,7 @@ void FBXImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy // Set FBX file scale is relative to CM must be converted to M for // assimp universal format (M) SetFileScale(size_relative_to_cm * 0.01f); - - std::for_each(tokens.begin(), tokens.end(), Util::delete_fun()); } catch (std::exception &) { - std::for_each(tokens.begin(), tokens.end(), Util::delete_fun()); throw; } } diff --git a/code/AssetLib/FBX/FBXParser.cpp b/code/AssetLib/FBX/FBXParser.cpp index e20377a3c..e6cc39f46 100644 --- a/code/AssetLib/FBX/FBXParser.cpp +++ b/code/AssetLib/FBX/FBXParser.cpp @@ -117,6 +117,7 @@ namespace FBX { // ------------------------------------------------------------------------------------------------ Element::Element(const Token& key_token, Parser& parser) : key_token(key_token) { TokenPtr n = nullptr; + StackAllocator &allocator = parser.GetAllocator(); do { n = parser.AdvanceToNextToken(); if(!n) { @@ -145,7 +146,7 @@ Element::Element(const Token& key_token, Parser& parser) : key_token(key_token) } if (n->Type() == TokenType_OPEN_BRACKET) { - compound.reset(new Scope(parser)); + compound.reset(new_Scope(parser)); // current token should be a TOK_CLOSE_BRACKET n = parser.CurrentToken(); @@ -178,6 +179,7 @@ Scope::Scope(Parser& parser,bool topLevel) } } + StackAllocator &allocator = parser.GetAllocator(); TokenPtr n = parser.AdvanceToNextToken(); if (n == nullptr) { ParseError("unexpected end of file"); @@ -208,22 +210,16 @@ Scope::Scope(Parser& parser,bool topLevel) } // ------------------------------------------------------------------------------------------------ -Scope::~Scope() { - for(ElementMap::value_type& v : elements) { - delete v.second; - } +Scope::~Scope() +{ } // ------------------------------------------------------------------------------------------------ -Parser::Parser (const TokenList& tokens, bool is_binary) -: tokens(tokens) -, last() -, current() -, cursor(tokens.begin()) -, is_binary(is_binary) +Parser::Parser(const TokenList &tokens, StackAllocator &allocator, bool is_binary) : + tokens(tokens), allocator(allocator), last(), current(), cursor(tokens.begin()), is_binary(is_binary) { ASSIMP_LOG_DEBUG("Parsing FBX tokens"); - root.reset(new Scope(*this,true)); + root = new_Scope(*this, true); } // ------------------------------------------------------------------------------------------------ diff --git a/code/AssetLib/FBX/FBXParser.h b/code/AssetLib/FBX/FBXParser.h index 314481e42..5eed61fef 100644 --- a/code/AssetLib/FBX/FBXParser.h +++ b/code/AssetLib/FBX/FBXParser.h @@ -52,6 +52,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include "Common/StackAllocator.h" #include "FBXCompileConfig.h" #include "FBXTokenizer.h" @@ -68,8 +69,8 @@ typedef std::fbx_unordered_multimap< std::string, Element* > ElementMap; typedef std::pair ElementCollection; -# define new_Scope new Scope -# define new_Element new Element +#define new_Scope new (allocator.Allocate(sizeof(Scope))) Scope +#define new_Element new (allocator.Allocate(sizeof(Element))) Element /** FBX data entity that consists of a key:value tuple. @@ -159,17 +160,21 @@ class Parser public: /** Parse given a token list. Does not take ownership of the tokens - * the objects must persist during the entire parser lifetime */ - Parser (const TokenList& tokens,bool is_binary); + Parser(const TokenList &tokens, StackAllocator &allocator, bool is_binary); ~Parser(); const Scope& GetRootScope() const { - return *root.get(); + return *root; } bool IsBinary() const { return is_binary; } + StackAllocator &GetAllocator() { + return allocator; + } + private: friend class Scope; friend class Element; @@ -180,10 +185,10 @@ private: private: const TokenList& tokens; - + StackAllocator &allocator; TokenPtr last, current; TokenList::const_iterator cursor; - std::unique_ptr root; + Scope *root; const bool is_binary; }; diff --git a/code/AssetLib/FBX/FBXTokenizer.cpp b/code/AssetLib/FBX/FBXTokenizer.cpp index 8698abac6..7f03b71da 100644 --- a/code/AssetLib/FBX/FBXTokenizer.cpp +++ b/code/AssetLib/FBX/FBXTokenizer.cpp @@ -96,7 +96,8 @@ AI_WONT_RETURN void TokenizeError(const std::string& message, unsigned int line, // process a potential data token up to 'cur', adding it to 'output_tokens'. // ------------------------------------------------------------------------------------------------ -void ProcessDataToken( TokenList& output_tokens, const char*& start, const char*& end, +void ProcessDataToken(TokenList &output_tokens, StackAllocator &token_allocator, + const char*& start, const char*& end, unsigned int line, unsigned int column, TokenType type = TokenType_DATA, @@ -133,8 +134,7 @@ void ProcessDataToken( TokenList& output_tokens, const char*& start, const char* } // ------------------------------------------------------------------------------------------------ -void Tokenize(TokenList& output_tokens, const char* input) -{ +void Tokenize(TokenList &output_tokens, const char *input, StackAllocator &token_allocator) { ai_assert(input); ASSIMP_LOG_DEBUG("Tokenizing ASCII FBX file"); @@ -166,7 +166,7 @@ void Tokenize(TokenList& output_tokens, const char* input) in_double_quotes = false; token_end = cur; - ProcessDataToken(output_tokens,token_begin,token_end,line,column); + ProcessDataToken(output_tokens, token_allocator, token_begin, token_end, line, column); pending_data_token = false; } continue; @@ -183,30 +183,30 @@ void Tokenize(TokenList& output_tokens, const char* input) continue; case ';': - ProcessDataToken(output_tokens,token_begin,token_end,line,column); + ProcessDataToken(output_tokens, token_allocator, token_begin, token_end, line, column); comment = true; continue; case '{': - ProcessDataToken(output_tokens,token_begin,token_end, line, column); + ProcessDataToken(output_tokens, token_allocator, token_begin, token_end, line, column); output_tokens.push_back(new_Token(cur,cur+1,TokenType_OPEN_BRACKET,line,column)); continue; case '}': - ProcessDataToken(output_tokens,token_begin,token_end,line,column); + ProcessDataToken(output_tokens, token_allocator, token_begin, token_end, line, column); output_tokens.push_back(new_Token(cur,cur+1,TokenType_CLOSE_BRACKET,line,column)); continue; case ',': if (pending_data_token) { - ProcessDataToken(output_tokens,token_begin,token_end,line,column,TokenType_DATA,true); + ProcessDataToken(output_tokens, token_allocator, token_begin, token_end, line, column, TokenType_DATA, true); } output_tokens.push_back(new_Token(cur,cur+1,TokenType_COMMA,line,column)); continue; case ':': if (pending_data_token) { - ProcessDataToken(output_tokens,token_begin,token_end,line,column,TokenType_KEY,true); + ProcessDataToken(output_tokens, token_allocator, token_begin, token_end, line, column, TokenType_KEY, true); } else { TokenizeError("unexpected colon", line, column); @@ -228,7 +228,7 @@ void Tokenize(TokenList& output_tokens, const char* input) } } - ProcessDataToken(output_tokens,token_begin,token_end,line,column,type); + ProcessDataToken(output_tokens, token_allocator, token_begin, token_end, line, column, type); } pending_data_token = false; diff --git a/code/AssetLib/FBX/FBXTokenizer.h b/code/AssetLib/FBX/FBXTokenizer.h index 877950945..d5f7999e4 100644 --- a/code/AssetLib/FBX/FBXTokenizer.h +++ b/code/AssetLib/FBX/FBXTokenizer.h @@ -47,6 +47,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define INCLUDED_AI_FBX_TOKENIZER_H #include "FBXCompileConfig.h" +#include "Common/StackAllocator.h" #include #include #include @@ -158,7 +159,7 @@ private: typedef const Token* TokenPtr; typedef std::vector< TokenPtr > TokenList; -#define new_Token new Token +#define new_Token new (token_allocator.Allocate(sizeof(Token))) Token /** Main FBX tokenizer function. Transform input buffer into a list of preprocessed tokens. @@ -168,7 +169,7 @@ typedef std::vector< TokenPtr > TokenList; * @param output_tokens Receives a list of all tokens in the input data. * @param input_buffer Textual input buffer to be processed, 0-terminated. * @throw DeadlyImportError if something goes wrong */ -void Tokenize(TokenList& output_tokens, const char* input); +void Tokenize(TokenList &output_tokens, const char *input, StackAllocator &tokenAllocator); /** Tokenizer function for binary FBX files. @@ -179,7 +180,7 @@ void Tokenize(TokenList& output_tokens, const char* input); * @param input_buffer Binary input buffer to be processed. * @param length Length of input buffer, in bytes. There is no 0-terminal. * @throw DeadlyImportError if something goes wrong */ -void TokenizeBinary(TokenList& output_tokens, const char* input, size_t length); +void TokenizeBinary(TokenList &output_tokens, const char *input, size_t length, StackAllocator &tokenAllocator); } // ! FBX diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index 5b08c0f37..e7eaf1d84 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -193,6 +193,8 @@ SET( Common_SRCS Common/ScenePreprocessor.cpp Common/ScenePreprocessor.h Common/SkeletonMeshBuilder.cpp + Common/StackAllocator.h + Common/StackAllocator.cpp Common/StandardShapes.cpp Common/TargetAnimation.cpp Common/TargetAnimation.h From d3646c3118b9b93fa7fe5dc9a979424450f8f72a Mon Sep 17 00:00:00 2001 From: Florian Born Date: Wed, 20 Apr 2022 16:11:09 +0200 Subject: [PATCH 03/28] Proper destruction of individual objects --- code/AssetLib/FBX/FBXDocument.cpp | 13 ++++++++++++- code/AssetLib/FBX/FBXDocument.h | 2 ++ code/AssetLib/FBX/FBXImporter.cpp | 14 +++++++++----- code/AssetLib/FBX/FBXParser.cpp | 17 ++++++++++++++--- code/AssetLib/FBX/FBXParser.h | 7 ++++--- code/AssetLib/FBX/FBXTokenizer.h | 1 + code/AssetLib/FBX/FBXUtil.h | 11 +++++++++++ code/Common/StackAllocator.h | 4 ++++ 8 files changed, 57 insertions(+), 12 deletions(-) diff --git a/code/AssetLib/FBX/FBXDocument.cpp b/code/AssetLib/FBX/FBXDocument.cpp index 17b347f86..437bf210b 100644 --- a/code/AssetLib/FBX/FBXDocument.cpp +++ b/code/AssetLib/FBX/FBXDocument.cpp @@ -257,7 +257,18 @@ Document::Document(Parser& parser, const ImportSettings& settings) : } // ------------------------------------------------------------------------------------------------ -Document::~Document() { +Document::~Document() +{ + // The document does not own the memory for the following objects, but we need to call their d'tor + // so they can properly free memory like string members: + + for (ObjectMap::value_type &v : objects) { + delete_LazyObject(v.second); + } + + for (ConnectionMap::value_type &v : src_connections) { + delete_Connection(v.second); + } // |dest_connections| contain the same Connection objects as the |src_connections| } diff --git a/code/AssetLib/FBX/FBXDocument.h b/code/AssetLib/FBX/FBXDocument.h index 142e870ff..11ee17f4c 100644 --- a/code/AssetLib/FBX/FBXDocument.h +++ b/code/AssetLib/FBX/FBXDocument.h @@ -82,6 +82,8 @@ class Cluster; #define new_LazyObject new (allocator.Allocate(sizeof(LazyObject))) LazyObject #define new_Connection new (allocator.Allocate(sizeof(Connection))) Connection +#define delete_LazyObject(_p) (_p)->~LazyObject() +#define delete_Connection(_p) (_p)->~Connection() /** Represents a delay-parsed FBX objects. Many objects in the scene * are not needed by assimp, so it makes no sense to parse them diff --git a/code/AssetLib/FBX/FBXImporter.cpp b/code/AssetLib/FBX/FBXImporter.cpp index 96c5a9f14..fa2122c4f 100644 --- a/code/AssetLib/FBX/FBXImporter.cpp +++ b/code/AssetLib/FBX/FBXImporter.cpp @@ -158,9 +158,8 @@ void FBXImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy // broadphase tokenizing pass in which we identify the core // syntax elements of FBX (brackets, commas, key:value mappings) TokenList tokens; - try { - - Assimp::StackAllocator tempAllocator; + Assimp::StackAllocator tempAllocator; + try { bool is_binary = false; if (!strncmp(begin, "Kaydara FBX Binary", 18)) { is_binary = true; @@ -190,8 +189,13 @@ void FBXImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy // Set FBX file scale is relative to CM must be converted to M for // assimp universal format (M) SetFileScale(size_relative_to_cm * 0.01f); - } catch (std::exception &) { - throw; + + // This collection does not own the memory for the tokens, but we need to call their d'tor + std::for_each(tokens.begin(), tokens.end(), Util::destructor_fun()); + + } catch (std::exception &) { + std::for_each(tokens.begin(), tokens.end(), Util::destructor_fun()); + throw; } } diff --git a/code/AssetLib/FBX/FBXParser.cpp b/code/AssetLib/FBX/FBXParser.cpp index e6cc39f46..976cdb90b 100644 --- a/code/AssetLib/FBX/FBXParser.cpp +++ b/code/AssetLib/FBX/FBXParser.cpp @@ -115,7 +115,9 @@ namespace Assimp { namespace FBX { // ------------------------------------------------------------------------------------------------ -Element::Element(const Token& key_token, Parser& parser) : key_token(key_token) { +Element::Element(const Token& key_token, Parser& parser) : + key_token(key_token), compound(nullptr) +{ TokenPtr n = nullptr; StackAllocator &allocator = parser.GetAllocator(); do { @@ -146,7 +148,7 @@ Element::Element(const Token& key_token, Parser& parser) : key_token(key_token) } if (n->Type() == TokenType_OPEN_BRACKET) { - compound.reset(new_Scope(parser)); + compound = new_Scope(parser); // current token should be a TOK_CLOSE_BRACKET n = parser.CurrentToken(); @@ -166,6 +168,10 @@ Element::Element(const Token& key_token, Parser& parser) : key_token(key_token) // ------------------------------------------------------------------------------------------------ Element::~Element() { + if (compound) { + delete_Scope(compound); + } + // no need to delete tokens, they are owned by the parser } @@ -212,6 +218,11 @@ Scope::Scope(Parser& parser,bool topLevel) // ------------------------------------------------------------------------------------------------ Scope::~Scope() { + // This collection does not own the memory for the elements, but we need to call their d'tor: + + for (ElementMap::value_type &v : elements) { + delete_Element(v.second); + } } // ------------------------------------------------------------------------------------------------ @@ -225,7 +236,7 @@ Parser::Parser(const TokenList &tokens, StackAllocator &allocator, bool is_binar // ------------------------------------------------------------------------------------------------ Parser::~Parser() { - // empty + delete_Scope(root); } // ------------------------------------------------------------------------------------------------ diff --git a/code/AssetLib/FBX/FBXParser.h b/code/AssetLib/FBX/FBXParser.h index 5eed61fef..9b654239b 100644 --- a/code/AssetLib/FBX/FBXParser.h +++ b/code/AssetLib/FBX/FBXParser.h @@ -71,7 +71,8 @@ typedef std::pair Element #define new_Scope new (allocator.Allocate(sizeof(Scope))) Scope #define new_Element new (allocator.Allocate(sizeof(Element))) Element - +#define delete_Scope(_p) (_p)->~Scope() +#define delete_Element(_p) (_p)->~Element() /** FBX data entity that consists of a key:value tuple. * @@ -91,7 +92,7 @@ public: ~Element(); const Scope* Compound() const { - return compound.get(); + return compound; } const Token& KeyToken() const { @@ -105,7 +106,7 @@ public: private: const Token& key_token; TokenList tokens; - std::unique_ptr compound; + Scope* compound; }; /** FBX data entity that consists of a 'scope', a collection diff --git a/code/AssetLib/FBX/FBXTokenizer.h b/code/AssetLib/FBX/FBXTokenizer.h index d5f7999e4..7e395d5e9 100644 --- a/code/AssetLib/FBX/FBXTokenizer.h +++ b/code/AssetLib/FBX/FBXTokenizer.h @@ -160,6 +160,7 @@ typedef const Token* TokenPtr; typedef std::vector< TokenPtr > TokenList; #define new_Token new (token_allocator.Allocate(sizeof(Token))) Token +#define delete_Token(_p) (_p)->~Token() /** Main FBX tokenizer function. Transform input buffer into a list of preprocessed tokens. diff --git a/code/AssetLib/FBX/FBXUtil.h b/code/AssetLib/FBX/FBXUtil.h index 0e0bb75be..4674a5054 100644 --- a/code/AssetLib/FBX/FBXUtil.h +++ b/code/AssetLib/FBX/FBXUtil.h @@ -66,6 +66,17 @@ struct delete_fun } }; +/** helper for std::for_each to call the destructor on all items in a container without freeing their heap*/ +template +struct destructor_fun { + void operator()(const volatile T* del) { + if (del) { + del->~T(); + } + } +}; + + /** Get a string representation for a #TokenType. */ const char* TokenTypeString(TokenType t); diff --git a/code/Common/StackAllocator.h b/code/Common/StackAllocator.h index b0f1ecfe1..b670b7434 100644 --- a/code/Common/StackAllocator.h +++ b/code/Common/StackAllocator.h @@ -67,6 +67,10 @@ public: // Destructs the allocator and frees all memory ~StackAllocator(); + // non copyable + StackAllocator(const StackAllocator &) = delete; + StackAllocator &operator=(const StackAllocator &) = delete; + // Returns a pointer to byteSize bytes of heap memory that persists // for the lifetime of the allocator (or until FreeAll is called). void *Allocate(size_t byteSize); From 320775b9392cd809b663440839b9124da844518c Mon Sep 17 00:00:00 2001 From: Florian Born Date: Wed, 20 Apr 2022 16:39:36 +0200 Subject: [PATCH 04/28] Compile fix --- code/Common/StackAllocator.h | 1 + 1 file changed, 1 insertion(+) diff --git a/code/Common/StackAllocator.h b/code/Common/StackAllocator.h index b670b7434..cc252f8df 100644 --- a/code/Common/StackAllocator.h +++ b/code/Common/StackAllocator.h @@ -51,6 +51,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include namespace Assimp { From 0355ae967feefd5811ddf4c37d91e43fff880a13 Mon Sep 17 00:00:00 2001 From: Florian Born Date: Wed, 20 Apr 2022 17:57:03 +0200 Subject: [PATCH 05/28] compile fix on other platforms --- code/Common/StackAllocator.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/Common/StackAllocator.cpp b/code/Common/StackAllocator.cpp index c789099a2..259a565be 100644 --- a/code/Common/StackAllocator.cpp +++ b/code/Common/StackAllocator.cpp @@ -59,7 +59,7 @@ void *StackAllocator::Allocate(size_t byteSize) { // double block size every time, up to maximum of g_maxBytesPerBlock. // Block size must be at least as large as byteSize, but we want to use this for small allocations anyway. m_blockAllocationSize = std::max(std::min(m_blockAllocationSize * 2, g_maxBytesPerBlock), byteSize); - uint8_t *data = (uint8_t *)malloc(m_blockAllocationSize); + uint8_t *data = new uint8_t[m_blockAllocationSize]; m_storageBlocks.push_back(data); m_subIndex = byteSize; return data; @@ -74,7 +74,7 @@ void *StackAllocator::Allocate(size_t byteSize) { void StackAllocator::FreeAll() { for (size_t i = 0; i < m_storageBlocks.size(); i++) { - free(m_storageBlocks[i]); + delete [] m_storageBlocks[i]; } std::deque empty; m_storageBlocks.swap(empty); From 7f0509ae87b852216ca853d7b7094fdb1cecf627 Mon Sep 17 00:00:00 2001 From: Florian Born Date: Thu, 21 Apr 2022 11:33:04 +0200 Subject: [PATCH 06/28] Stack allocator is now inline --- code/CMakeLists.txt | 2 +- code/Common/StackAllocator.h | 10 ++++++---- code/Common/{StackAllocator.cpp => StackAllocator.inl} | 8 ++++---- 3 files changed, 11 insertions(+), 9 deletions(-) rename code/Common/{StackAllocator.cpp => StackAllocator.inl} (93%) diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index e7eaf1d84..75a5cb377 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -194,7 +194,7 @@ SET( Common_SRCS Common/ScenePreprocessor.h Common/SkeletonMeshBuilder.cpp Common/StackAllocator.h - Common/StackAllocator.cpp + Common/StackAllocator.inl Common/StandardShapes.cpp Common/TargetAnimation.cpp Common/TargetAnimation.h diff --git a/code/Common/StackAllocator.h b/code/Common/StackAllocator.h index cc252f8df..aff36fa47 100644 --- a/code/Common/StackAllocator.h +++ b/code/Common/StackAllocator.h @@ -64,9 +64,9 @@ namespace Assimp class StackAllocator { public: // Constructs the allocator - StackAllocator(); + inline StackAllocator(); // Destructs the allocator and frees all memory - ~StackAllocator(); + inline ~StackAllocator(); // non copyable StackAllocator(const StackAllocator &) = delete; @@ -74,11 +74,11 @@ public: // Returns a pointer to byteSize bytes of heap memory that persists // for the lifetime of the allocator (or until FreeAll is called). - void *Allocate(size_t byteSize); + inline void *Allocate(size_t byteSize); // Releases all the memory owned by this allocator. // Memory provided through function Allocate is not valid anymore after this function has been called. - void FreeAll(); + inline void FreeAll(); private: constexpr const static size_t g_maxBytesPerBlock = 64 * 1024 * 1024; // The maximum size (in bytes) of a block @@ -91,4 +91,6 @@ private: } // namespace Assimp +#include "StackAllocator.inl" + #endif // include guard diff --git a/code/Common/StackAllocator.cpp b/code/Common/StackAllocator.inl similarity index 93% rename from code/Common/StackAllocator.cpp rename to code/Common/StackAllocator.inl index 259a565be..d973b7794 100644 --- a/code/Common/StackAllocator.cpp +++ b/code/Common/StackAllocator.inl @@ -46,14 +46,14 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. using namespace Assimp; -StackAllocator::StackAllocator() { +inline StackAllocator::StackAllocator() { } -StackAllocator::~StackAllocator() { +inline StackAllocator::~StackAllocator() { FreeAll(); } -void *StackAllocator::Allocate(size_t byteSize) { +inline void *StackAllocator::Allocate(size_t byteSize) { if (m_subIndex + byteSize > m_blockAllocationSize) // start a new block { // double block size every time, up to maximum of g_maxBytesPerBlock. @@ -72,7 +72,7 @@ void *StackAllocator::Allocate(size_t byteSize) { return data; } -void StackAllocator::FreeAll() { +inline void StackAllocator::FreeAll() { for (size_t i = 0; i < m_storageBlocks.size(); i++) { delete [] m_storageBlocks[i]; } From a415f33fb561d8e2531205aa61d1228e47158c19 Mon Sep 17 00:00:00 2001 From: Florian Born Date: Thu, 21 Apr 2022 16:09:28 +0200 Subject: [PATCH 07/28] merge failure, parts in this file were missing --- code/AssetLib/FBX/FBXDocument.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/FBX/FBXDocument.h b/code/AssetLib/FBX/FBXDocument.h index 11ee17f4c..c701056f6 100644 --- a/code/AssetLib/FBX/FBXDocument.h +++ b/code/AssetLib/FBX/FBXDocument.h @@ -1076,7 +1076,7 @@ private: /** DOM root for a FBX file */ class Document { public: - Document(const Parser& parser, const ImportSettings& settings); + Document(Parser& parser, const ImportSettings& settings); ~Document(); @@ -1160,7 +1160,7 @@ private: const ImportSettings& settings; ObjectMap objects; - const Parser& parser; + Parser& parser; PropertyTemplateMap templates; ConnectionMap src_connections; From b2ea018fd57326a6421375cc7999d08a7ee341a2 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 11 Sep 2022 17:04:58 +0200 Subject: [PATCH 08/28] Use user-define element destructor. --- code/AssetLib/FBX/FBXParser.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/AssetLib/FBX/FBXParser.h b/code/AssetLib/FBX/FBXParser.h index 597096548..67707342e 100644 --- a/code/AssetLib/FBX/FBXParser.h +++ b/code/AssetLib/FBX/FBXParser.h @@ -89,7 +89,7 @@ class Element { public: Element(const Token& key_token, Parser& parser); - ~Element() = default; + ~Element(): const Scope* Compound() const { return compound; From 822b2406943f4bc426b9260a5bba523d65aabe26 Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 8 Nov 2022 23:09:50 +0200 Subject: [PATCH 09/28] Support both pbrSpecGlos and materials_specular --- code/AssetLib/glTF2/glTF2Asset.h | 17 +++++++++++++ code/AssetLib/glTF2/glTF2Asset.inl | 22 +++++++++++++++- code/AssetLib/glTF2/glTF2AssetWriter.h | 1 + code/AssetLib/glTF2/glTF2AssetWriter.inl | 27 ++++++++++++++++++-- code/AssetLib/glTF2/glTF2Exporter.cpp | 32 +++++++++++++++++++++--- code/AssetLib/glTF2/glTF2Exporter.h | 2 ++ code/AssetLib/glTF2/glTF2Importer.cpp | 15 ++++++++++- include/assimp/material.h | 1 + 8 files changed, 110 insertions(+), 7 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Asset.h b/code/AssetLib/glTF2/glTF2Asset.h index 3becc4d9b..b241a4b59 100644 --- a/code/AssetLib/glTF2/glTF2Asset.h +++ b/code/AssetLib/glTF2/glTF2Asset.h @@ -44,6 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * * glTF Extensions Support: * KHR_materials_pbrSpecularGlossiness full + * KHR_materials_specular full * KHR_materials_unlit full * KHR_lights_punctual full * KHR_materials_sheen full @@ -718,6 +719,7 @@ const vec4 defaultBaseColor = { 1, 1, 1, 1 }; const vec3 defaultEmissiveFactor = { 0, 0, 0 }; const vec4 defaultDiffuseFactor = { 1, 1, 1, 1 }; const vec3 defaultSpecularFactor = { 1, 1, 1 }; +const vec3 defaultSpecularColorFactor = { 0, 0, 0 }; const vec3 defaultSheenFactor = { 0, 0, 0 }; const vec3 defaultAttenuationColor = { 1, 1, 1 }; @@ -761,6 +763,16 @@ struct PbrSpecularGlossiness { void SetDefaults(); }; +struct MaterialSpecular { + float specularFactor; + vec3 specularColorFactor; + TextureInfo specularTexture; + TextureInfo specularColorTexture; + + MaterialSpecular() { SetDefaults(); } + void SetDefaults(); +}; + struct MaterialSheen { vec3 sheenColorFactor; float sheenRoughnessFactor; @@ -818,6 +830,9 @@ struct Material : public Object { //extension: KHR_materials_pbrSpecularGlossiness Nullable pbrSpecularGlossiness; + //extension: KHR_materials_specular + Nullable materialSpecular; + //extension: KHR_materials_sheen Nullable materialSheen; @@ -1098,6 +1113,7 @@ public: //! Keeps info about the enabled extensions struct Extensions { bool KHR_materials_pbrSpecularGlossiness; + bool KHR_materials_specular; bool KHR_materials_unlit; bool KHR_lights_punctual; bool KHR_texture_transform; @@ -1112,6 +1128,7 @@ public: Extensions() : KHR_materials_pbrSpecularGlossiness(false), + KHR_materials_specular(false), KHR_materials_unlit(false), KHR_lights_punctual(false), KHR_texture_transform(false), diff --git a/code/AssetLib/glTF2/glTF2Asset.inl b/code/AssetLib/glTF2/glTF2Asset.inl index db47915d6..aaa652dcd 100644 --- a/code/AssetLib/glTF2/glTF2Asset.inl +++ b/code/AssetLib/glTF2/glTF2Asset.inl @@ -1236,7 +1236,7 @@ inline void Material::Read(Value &material, Asset &r) { ReadMember(material, "alphaCutoff", this->alphaCutoff); if (Value *extensions = FindObject(material, "extensions")) { - if (r.extensionsUsed.KHR_materials_pbrSpecularGlossiness) { + if (r.extensionsUsed.KHR_materials_pbrSpecularGlossiness) { //TODO: Maybe ignore this if KHR_materials_specular is also defined if (Value *curPbrSpecularGlossiness = FindObject(*extensions, "KHR_materials_pbrSpecularGlossiness")) { PbrSpecularGlossiness pbrSG; @@ -1249,6 +1249,19 @@ inline void Material::Read(Value &material, Asset &r) { this->pbrSpecularGlossiness = Nullable(pbrSG); } } + + if (r.extensionsUsed.KHR_materials_specular) { + if (Value *curMatSpecular = FindObject(*extensions, "KHR_materials_specular")) { + MaterialSpecular specular; + + ReadMember(*curMatSpecular, "specularFactor", specular.specularFactor); + ReadTextureProperty(r, *curMatSpecular, "specularTexture", specular.specularTexture); + ReadMember(*curMatSpecular, "specularColorFactor", specular.specularColorFactor); + ReadTextureProperty(r, *curMatSpecular, "specularColorTexture", specular.specularColorTexture); + + this->materialSpecular = Nullable(specular); + } + } // Extension KHR_texture_transform is handled in ReadTextureProperty @@ -1337,6 +1350,12 @@ inline void PbrSpecularGlossiness::SetDefaults() { glossinessFactor = 1.0f; } +inline void MaterialSpecular::SetDefaults() { + //KHR_materials_specular properties + SetVector(specularColorFactor, defaultSpecularColorFactor); + specularFactor = 0.f; +} + inline void MaterialSheen::SetDefaults() { //KHR_materials_sheen properties SetVector(sheenColorFactor, defaultSheenFactor); @@ -2018,6 +2037,7 @@ inline void Asset::ReadExtensionsUsed(Document &doc) { } CHECK_EXT(KHR_materials_pbrSpecularGlossiness); + CHECK_EXT(KHR_materials_specular); CHECK_EXT(KHR_materials_unlit); CHECK_EXT(KHR_lights_punctual); CHECK_EXT(KHR_texture_transform); diff --git a/code/AssetLib/glTF2/glTF2AssetWriter.h b/code/AssetLib/glTF2/glTF2AssetWriter.h index 089a15844..5489ec2d4 100644 --- a/code/AssetLib/glTF2/glTF2AssetWriter.h +++ b/code/AssetLib/glTF2/glTF2AssetWriter.h @@ -45,6 +45,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * * glTF Extensions Support: * KHR_materials_pbrSpecularGlossiness: full + * KHR_materials_specular: full * KHR_materials_unlit: full * KHR_materials_sheen: full * KHR_materials_clearcoat: full diff --git a/code/AssetLib/glTF2/glTF2AssetWriter.inl b/code/AssetLib/glTF2/glTF2AssetWriter.inl index 0be139595..c57603bb0 100644 --- a/code/AssetLib/glTF2/glTF2AssetWriter.inl +++ b/code/AssetLib/glTF2/glTF2AssetWriter.inl @@ -418,6 +418,25 @@ namespace glTF2 { exts.AddMember("KHR_materials_unlit", unlit, w.mAl); } + if (m.materialSpecular.isPresent) { + Value materialSpecular(rapidjson::Type::kObjectType); + materialSpecular.SetObject(); + + MaterialSpecular &specular = m.materialSpecular.value; + + if (specular.specularFactor != 0.f) { + WriteFloat(materialSpecular, specular.specularFactor, "specularFactor", w.mAl); + WriteTex(materialSpecular, specular.specularTexture, "specularTexture", w.mAl); + } + if (specular.specularColorFactor[0] != defaultSpecularColorFactor[0] && specular.specularColorFactor[1] != defaultSpecularColorFactor[1] && specular.specularColorFactor[2] != defaultSpecularColorFactor[2]) { + WriteVec(materialSpecular, specular.specularColorFactor, "specularColorFactor", w.mAl); + WriteTex(materialSpecular, specular.specularColorTexture, "specularColorTexture", w.mAl); + } + + if (!materialSpecular.ObjectEmpty()) { + exts.AddMember("KHR_materials_specular", materialSpecular, w.mAl); + } + if (m.materialSheen.isPresent) { Value materialSheen(rapidjson::Type::kObjectType); @@ -536,7 +555,7 @@ namespace glTF2 { inline void Write(Value& obj, Mesh& m, AssetWriter& w) { - /****************** Primitives *******************/ + /****************** Primitives *******************/ Value primitives; primitives.SetArray(); primitives.Reserve(unsigned(m.primitives.size()), w.mAl); @@ -915,6 +934,10 @@ namespace glTF2 { exts.PushBack(StringRef("KHR_materials_unlit"), mAl); } + if (this->mAsset.extensionsUsed.KHR_materials_specular) { + exts.PushBack(StringRef("KHR_materials_specular"), mAl); + } + if (this->mAsset.extensionsUsed.KHR_materials_sheen) { exts.PushBack(StringRef("KHR_materials_sheen"), mAl); } @@ -962,7 +985,7 @@ namespace glTF2 { if (d.mObjs.empty()) return; Value* container = &mDoc; - const char* context = "Document"; + const char* context = "Document"; if (d.mExtId) { Value* exts = FindObject(mDoc, "extensions"); diff --git a/code/AssetLib/glTF2/glTF2Exporter.cpp b/code/AssetLib/glTF2/glTF2Exporter.cpp index 3bfe49fba..15c193d36 100644 --- a/code/AssetLib/glTF2/glTF2Exporter.cpp +++ b/code/AssetLib/glTF2/glTF2Exporter.cpp @@ -644,7 +644,10 @@ bool glTF2Exporter::GetMatSpecGloss(const aiMaterial &mat, glTF2::PbrSpecularGlo bool result = false; // If has Glossiness, a Specular Color or Specular Texture, use the KHR_materials_pbrSpecularGlossiness extension // NOTE: This extension is being considered for deprecation (Dec 2020), may be replaced by KHR_material_specular - + // This extension has been deprecated, only export with the specific flag enabled, defaults to false. Uses KHR_material_specular default. + if (mat.Get(AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS) != AI_SUCCESS) { + return false; + } if (mat.Get(AI_MATKEY_GLOSSINESS_FACTOR, pbrSG.glossinessFactor) == AI_SUCCESS) { result = true; } else { @@ -674,6 +677,24 @@ bool glTF2Exporter::GetMatSpecGloss(const aiMaterial &mat, glTF2::PbrSpecularGlo return result; } +bool glTF2Exporter::GetMatSpecular(const aiMaterial &mat, glTF2::MaterialSpecular &specular) { + // Specular requires either/or + if (GetMatColor(mat, specular.specularColorFactor, AI_MATKEY_COLOR_SPECULAR) != AI_SUCCESS && mat.Get(AI_MATKEY_SPECULAR_FACTOR, specular.specularFactor) != AI_SUCCESS) { + return false; + } + + // default factors of zero disables specular, so do not export + if (specular.specularFactor == 0.0f && (specular.specularColorFactor[0] == defaultSpecularColorFactor[0] && specular.specularColorFactor[1] == defaultSpecularColorFactor[1] && specular.specularColorFactor[2] == defaultSpecularColorFactor[2])) { + return false; + } + + // Add any appropriate textures + GetMatTex(mat, specular.specularTexture, aiTextureType_SPECULAR); + GetMatTex(mat, specular.specularColorTexture, aiTextureType_SPECULAR); + + return true; +} + bool glTF2Exporter::GetMatSheen(const aiMaterial &mat, glTF2::MaterialSheen &sheen) { // Return true if got any valid Sheen properties or textures if (GetMatColor(mat, sheen.sheenColorFactor, AI_MATKEY_SHEEN_COLOR_FACTOR) != aiReturn_SUCCESS) { @@ -816,7 +837,7 @@ void glTF2Exporter::ExportMaterials() { { // KHR_materials_pbrSpecularGlossiness extension - // NOTE: This extension is being considered for deprecation (Dec 2020) + // This extension has been deprecated, only export with the specific flag enabled, defaults to false. Uses KHR_material_specular default. PbrSpecularGlossiness pbrSG; if (GetMatSpecGloss(mat, pbrSG)) { mAsset->extensionsUsed.KHR_materials_pbrSpecularGlossiness = true; @@ -833,7 +854,12 @@ void glTF2Exporter::ExportMaterials() { } else { // These extensions are not compatible with KHR_materials_unlit or KHR_materials_pbrSpecularGlossiness if (!m->pbrSpecularGlossiness.isPresent) { - // Sheen + MaterialSpecular specular; + if (GetMatSpecular(mat, specular)) { + mAsset->extensionsUsed.KHR_materials_specular = true; + m->materialSpecular = Nullable(specular); + } + MaterialSheen sheen; if (GetMatSheen(mat, sheen)) { mAsset->extensionsUsed.KHR_materials_sheen = true; diff --git a/code/AssetLib/glTF2/glTF2Exporter.h b/code/AssetLib/glTF2/glTF2Exporter.h index 99425228e..f211d2d53 100644 --- a/code/AssetLib/glTF2/glTF2Exporter.h +++ b/code/AssetLib/glTF2/glTF2Exporter.h @@ -76,6 +76,7 @@ struct OcclusionTextureInfo; struct Node; struct Texture; struct PbrSpecularGlossiness; +struct MaterialSpecular; struct MaterialSheen; struct MaterialClearcoat; struct MaterialTransmission; @@ -116,6 +117,7 @@ protected: aiReturn GetMatColor(const aiMaterial &mat, glTF2::vec4 &prop, const char *propName, int type, int idx) const; aiReturn GetMatColor(const aiMaterial &mat, glTF2::vec3 &prop, const char *propName, int type, int idx) const; bool GetMatSpecGloss(const aiMaterial &mat, glTF2::PbrSpecularGlossiness &pbrSG); + bool GetMatSpecular(const aiMaterial &mat, glTF2::MaterialSpecular &specular); bool GetMatSheen(const aiMaterial &mat, glTF2::MaterialSheen &sheen); bool GetMatClearcoat(const aiMaterial &mat, glTF2::MaterialClearcoat &clearcoat); bool GetMatTransmission(const aiMaterial &mat, glTF2::MaterialTransmission &transmission); diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index 947edc8d5..1757026fe 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -283,8 +283,9 @@ static aiMaterial *ImportMaterial(std::vector &embeddedTexIdxs, Asset &r, M aimat->AddProperty(&mat.alphaCutoff, 1, AI_MATKEY_GLTF_ALPHACUTOFF); // pbrSpecularGlossiness - if (mat.pbrSpecularGlossiness.isPresent) { + if (mat.pbrSpecularGlossiness.isPresent) { // TODO: Consider importing this only if KHR_materials_specular isn't also defined PbrSpecularGlossiness &pbrSG = mat.pbrSpecularGlossiness.value; + aimat->AddProperty(new int(1), 1, AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS); SetMaterialColorProperty(r, pbrSG.diffuseFactor, aimat, AI_MATKEY_COLOR_DIFFUSE); SetMaterialColorProperty(r, pbrSG.specularFactor, aimat, AI_MATKEY_COLOR_SPECULAR); @@ -307,6 +308,18 @@ static aiMaterial *ImportMaterial(std::vector &embeddedTexIdxs, Asset &r, M aimat->AddProperty(&shadingMode, 1, AI_MATKEY_SHADING_MODEL); + // KHR_materials_specular + if (mat.materialSpecular.isPresent) { + MaterialSpecular &specular = mat.materialSpecular.value; + // Default values of zero disables Specular + if (std::memcmp(specular.specularColorFactor, defaultSpecularColorFactor, sizeof(glTFCommon::vec3)) != 0 || specular.specularFactor != 0.0f) { + SetMaterialColorProperty(r, specular.specularColorFactor, aimat, AI_MATKEY_COLOR_SPECULAR); + aimat->AddProperty(&specular.specularFactor, 1, AI_MATKEY_SPECULAR_FACTOR); + SetMaterialTextureProperty(embeddedTexIdxs, r, specular.specularTexture, aimat, aiTextureType_SPECULAR); + SetMaterialTextureProperty(embeddedTexIdxs, r, specular.specularColorTexture, aimat, aiTextureType_SPECULAR); + } + } + // KHR_materials_sheen if (mat.materialSheen.isPresent) { MaterialSheen &sheen = mat.materialSheen.value; diff --git a/include/assimp/material.h b/include/assimp/material.h index 0052888d1..92f7ab8ca 100644 --- a/include/assimp/material.h +++ b/include/assimp/material.h @@ -1000,6 +1000,7 @@ extern "C" { // Specular Color. // Note: Metallic/Roughness may also have a Specular Color // AI_MATKEY_COLOR_SPECULAR +#define AI_MATKEY_USE_PBR_SPECULAR_GLOSSINESS "$mat.useGltfPbrSpecularGlossiness", 0, 0 #define AI_MATKEY_SPECULAR_FACTOR "$mat.specularFactor", 0, 0 // Glossiness factor. 0.0 = Completely Rough, 1.0 = Perfectly Smooth #define AI_MATKEY_GLOSSINESS_FACTOR "$mat.glossinessFactor", 0, 0 From ffaf378ee6107221c3a731039d3872910801137f Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 10 Nov 2022 20:37:46 +0200 Subject: [PATCH 10/28] fixed misnamed matkey --- include/assimp/material.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/assimp/material.h b/include/assimp/material.h index 92f7ab8ca..3a623d10e 100644 --- a/include/assimp/material.h +++ b/include/assimp/material.h @@ -1000,7 +1000,7 @@ extern "C" { // Specular Color. // Note: Metallic/Roughness may also have a Specular Color // AI_MATKEY_COLOR_SPECULAR -#define AI_MATKEY_USE_PBR_SPECULAR_GLOSSINESS "$mat.useGltfPbrSpecularGlossiness", 0, 0 +#define AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS "$mat.useGltfPbrSpecularGlossiness", 0, 0 #define AI_MATKEY_SPECULAR_FACTOR "$mat.specularFactor", 0, 0 // Glossiness factor. 0.0 = Completely Rough, 1.0 = Perfectly Smooth #define AI_MATKEY_GLOSSINESS_FACTOR "$mat.glossinessFactor", 0, 0 From 1cd5841b2f331ba630d7a873a4e59dbf3449d711 Mon Sep 17 00:00:00 2001 From: Adam Date: Fri, 18 Nov 2022 17:24:37 +0200 Subject: [PATCH 11/28] . --- code/AssetLib/glTF2/glTF2AssetWriter.inl | 1 + 1 file changed, 1 insertion(+) diff --git a/code/AssetLib/glTF2/glTF2AssetWriter.inl b/code/AssetLib/glTF2/glTF2AssetWriter.inl index c57603bb0..de1e25fc1 100644 --- a/code/AssetLib/glTF2/glTF2AssetWriter.inl +++ b/code/AssetLib/glTF2/glTF2AssetWriter.inl @@ -435,6 +435,7 @@ namespace glTF2 { if (!materialSpecular.ObjectEmpty()) { exts.AddMember("KHR_materials_specular", materialSpecular, w.mAl); + } } if (m.materialSheen.isPresent) { From 371d5c78f494af360333c6246e9ff91c3d2520f6 Mon Sep 17 00:00:00 2001 From: Justice Colby Date: Mon, 21 Nov 2022 15:49:48 -0800 Subject: [PATCH 12/28] Updated ConvertMaterials function to assign appopriate material index when using material references. --- code/AssetLib/X/XFileImporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/AssetLib/X/XFileImporter.cpp b/code/AssetLib/X/XFileImporter.cpp index b96b041aa..d777eee32 100644 --- a/code/AssetLib/X/XFileImporter.cpp +++ b/code/AssetLib/X/XFileImporter.cpp @@ -580,7 +580,7 @@ void XFileImporter::ConvertMaterials( aiScene* pScene, std::vectormMaterials[b]->Get( AI_MATKEY_NAME, name); if( strcmp( name.C_Str(), oldMat.mName.data()) == 0 ) { - oldMat.sceneIndex = a; + oldMat.sceneIndex = b; break; } } From fa00571049b601d970ebc61331fc9fbb66e217ce Mon Sep 17 00:00:00 2001 From: Adam Beili <54665621+Beilinson@users.noreply.github.com> Date: Sun, 26 Mar 2023 14:52:44 +0200 Subject: [PATCH 13/28] fixed compilation bug --- code/AssetLib/glTF2/glTF2Exporter.cpp | 35 ++++++++++++--------------- code/AssetLib/glTF2/glTF2Importer.cpp | 25 +++++++++---------- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Exporter.cpp b/code/AssetLib/glTF2/glTF2Exporter.cpp index 464a045b9..17140aa6a 100644 --- a/code/AssetLib/glTF2/glTF2Exporter.cpp +++ b/code/AssetLib/glTF2/glTF2Exporter.cpp @@ -640,14 +640,16 @@ aiReturn glTF2Exporter::GetMatColor(const aiMaterial &mat, vec3 &prop, const cha return result; } +// This extension has been deprecated, only export with the specific flag enabled, defaults to false. Uses KHR_material_specular default. bool glTF2Exporter::GetMatSpecGloss(const aiMaterial &mat, glTF2::PbrSpecularGlossiness &pbrSG) { - bool result = false; - // If has Glossiness, a Specular Color or Specular Texture, use the KHR_materials_pbrSpecularGlossiness extension - // NOTE: This extension is being considered for deprecation (Dec 2020), may be replaced by KHR_material_specular - // This extension has been deprecated, only export with the specific flag enabled, defaults to false. Uses KHR_material_specular default. - if (mat.Get(AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS) != AI_SUCCESS) { + int usePbrSpecGloss; + if (mat.Get(AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS, usePbrSpecGloss) != AI_SUCCESS) { return false; } + + bool result = false; + + // If has Glossiness, a Specular Color or Specular Texture, use the KHR_materials_pbrSpecularGlossiness extension if (mat.Get(AI_MATKEY_GLOSSINESS_FACTOR, pbrSG.glossinessFactor) == AI_SUCCESS) { result = true; } else { @@ -678,21 +680,16 @@ bool glTF2Exporter::GetMatSpecGloss(const aiMaterial &mat, glTF2::PbrSpecularGlo } bool glTF2Exporter::GetMatSpecular(const aiMaterial &mat, glTF2::MaterialSpecular &specular) { - // Specular requires either/or - if (GetMatColor(mat, specular.specularColorFactor, AI_MATKEY_COLOR_SPECULAR) != AI_SUCCESS && mat.Get(AI_MATKEY_SPECULAR_FACTOR, specular.specularFactor) != AI_SUCCESS) { - return false; + // Specular requires either/or, default factors of zero disables specular, so do not export + bool result = false; + if (GetMatColor(mat, specular.specularColorFactor, AI_MATKEY_COLOR_SPECULAR) == AI_SUCCESS && specular.specularFactor != 0.0f) { + GetMatTex(mat, specular.specularColorTexture, aiTextureType_SPECULAR); + result = true; + } else if (mat.Get(AI_MATKEY_SPECULAR_FACTOR, specular.specularFactor) == AI_SUCCESS && !(specular.specularColorFactor[0] == defaultSpecularColorFactor[0] && specular.specularColorFactor[1] == defaultSpecularColorFactor[1] && specular.specularColorFactor[2] == defaultSpecularColorFactor[2])) { + GetMatTex(mat, specular.specularTexture, aiTextureType_SPECULAR); + result = true; } - - // default factors of zero disables specular, so do not export - if (specular.specularFactor == 0.0f && (specular.specularColorFactor[0] == defaultSpecularColorFactor[0] && specular.specularColorFactor[1] == defaultSpecularColorFactor[1] && specular.specularColorFactor[2] == defaultSpecularColorFactor[2])) { - return false; - } - - // Add any appropriate textures - GetMatTex(mat, specular.specularTexture, aiTextureType_SPECULAR); - GetMatTex(mat, specular.specularColorTexture, aiTextureType_SPECULAR); - - return true; + return result; } bool glTF2Exporter::GetMatSheen(const aiMaterial &mat, glTF2::MaterialSheen &sheen) { diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index 805ae0548..f298e7037 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -278,8 +278,19 @@ static aiMaterial *ImportMaterial(std::vector &embeddedTexIdxs, Asset &r, M aimat->AddProperty(&alphaMode, AI_MATKEY_GLTF_ALPHAMODE); aimat->AddProperty(&mat.alphaCutoff, 1, AI_MATKEY_GLTF_ALPHACUTOFF); + // KHR_materials_specular + if (mat.materialSpecular.isPresent) { + MaterialSpecular &specular = mat.materialSpecular.value; + // Default values of zero disables Specular + if (std::memcmp(specular.specularColorFactor, defaultSpecularColorFactor, sizeof(glTFCommon::vec3)) != 0 || specular.specularFactor != 0.0f) { + SetMaterialColorProperty(r, specular.specularColorFactor, aimat, AI_MATKEY_COLOR_SPECULAR); + aimat->AddProperty(&specular.specularFactor, 1, AI_MATKEY_SPECULAR_FACTOR); + SetMaterialTextureProperty(embeddedTexIdxs, r, specular.specularTexture, aimat, aiTextureType_SPECULAR); + SetMaterialTextureProperty(embeddedTexIdxs, r, specular.specularColorTexture, aimat, aiTextureType_SPECULAR); + } + } // pbrSpecularGlossiness - if (mat.pbrSpecularGlossiness.isPresent) { // TODO: Consider importing this only if KHR_materials_specular isn't also defined + else if (mat.pbrSpecularGlossiness.isPresent) { PbrSpecularGlossiness &pbrSG = mat.pbrSpecularGlossiness.value; aimat->AddProperty(new int(1), 1, AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS); @@ -304,18 +315,6 @@ static aiMaterial *ImportMaterial(std::vector &embeddedTexIdxs, Asset &r, M aimat->AddProperty(&shadingMode, 1, AI_MATKEY_SHADING_MODEL); - // KHR_materials_specular - if (mat.materialSpecular.isPresent) { - MaterialSpecular &specular = mat.materialSpecular.value; - // Default values of zero disables Specular - if (std::memcmp(specular.specularColorFactor, defaultSpecularColorFactor, sizeof(glTFCommon::vec3)) != 0 || specular.specularFactor != 0.0f) { - SetMaterialColorProperty(r, specular.specularColorFactor, aimat, AI_MATKEY_COLOR_SPECULAR); - aimat->AddProperty(&specular.specularFactor, 1, AI_MATKEY_SPECULAR_FACTOR); - SetMaterialTextureProperty(embeddedTexIdxs, r, specular.specularTexture, aimat, aiTextureType_SPECULAR); - SetMaterialTextureProperty(embeddedTexIdxs, r, specular.specularColorTexture, aimat, aiTextureType_SPECULAR); - } - } - // KHR_materials_sheen if (mat.materialSheen.isPresent) { MaterialSheen &sheen = mat.materialSheen.value; From 83053f3d564a0ba624c0165364403e8b2e6bcba3 Mon Sep 17 00:00:00 2001 From: Adam Beili <54665621+Beilinson@users.noreply.github.com> Date: Sun, 26 Mar 2023 16:55:38 +0200 Subject: [PATCH 14/28] Made usePbrSpecGloss a exportproperty, fixed mat_specular to spec --- code/AssetLib/glTF2/glTF2Exporter.cpp | 34 +++++++++++++-------------- code/AssetLib/glTF2/glTF2Importer.cpp | 1 - include/assimp/config.h.in | 11 +++++++++ include/assimp/material.h | 1 - test/unit/utglTF2ImportExport.cpp | 4 +++- 5 files changed, 31 insertions(+), 20 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Exporter.cpp b/code/AssetLib/glTF2/glTF2Exporter.cpp index 17140aa6a..3a1441ad9 100644 --- a/code/AssetLib/glTF2/glTF2Exporter.cpp +++ b/code/AssetLib/glTF2/glTF2Exporter.cpp @@ -642,13 +642,7 @@ aiReturn glTF2Exporter::GetMatColor(const aiMaterial &mat, vec3 &prop, const cha // This extension has been deprecated, only export with the specific flag enabled, defaults to false. Uses KHR_material_specular default. bool glTF2Exporter::GetMatSpecGloss(const aiMaterial &mat, glTF2::PbrSpecularGlossiness &pbrSG) { - int usePbrSpecGloss; - if (mat.Get(AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS, usePbrSpecGloss) != AI_SUCCESS) { - return false; - } - bool result = false; - // If has Glossiness, a Specular Color or Specular Texture, use the KHR_materials_pbrSpecularGlossiness extension if (mat.Get(AI_MATKEY_GLOSSINESS_FACTOR, pbrSG.glossinessFactor) == AI_SUCCESS) { result = true; @@ -681,15 +675,21 @@ bool glTF2Exporter::GetMatSpecGloss(const aiMaterial &mat, glTF2::PbrSpecularGlo bool glTF2Exporter::GetMatSpecular(const aiMaterial &mat, glTF2::MaterialSpecular &specular) { // Specular requires either/or, default factors of zero disables specular, so do not export - bool result = false; - if (GetMatColor(mat, specular.specularColorFactor, AI_MATKEY_COLOR_SPECULAR) == AI_SUCCESS && specular.specularFactor != 0.0f) { - GetMatTex(mat, specular.specularColorTexture, aiTextureType_SPECULAR); - result = true; - } else if (mat.Get(AI_MATKEY_SPECULAR_FACTOR, specular.specularFactor) == AI_SUCCESS && !(specular.specularColorFactor[0] == defaultSpecularColorFactor[0] && specular.specularColorFactor[1] == defaultSpecularColorFactor[1] && specular.specularColorFactor[2] == defaultSpecularColorFactor[2])) { - GetMatTex(mat, specular.specularTexture, aiTextureType_SPECULAR); - result = true; + if (GetMatColor(mat, specular.specularColorFactor, AI_MATKEY_COLOR_SPECULAR) != AI_SUCCESS || mat.Get(AI_MATKEY_SPECULAR_FACTOR, specular.specularFactor) != AI_SUCCESS) { + return false; } - return result; + // The spec states that the default is 1.0 and [1.0, 1.0, 1.0]. We if both are 0, which should disable specular. Otherwise, if one is 0, set to 1.0 + const bool colorFactorIsZero = specular.specularColorFactor[0] == defaultSpecularColorFactor[0] && specular.specularColorFactor[1] == defaultSpecularColorFactor[1] && specular.specularColorFactor[2] == defaultSpecularColorFactor[2]; + if (specular.specularFactor == 0.0f && colorFactorIsZero) { + return false; + } else if (specular.specularFactor == 0.0f) { + specular.specularFactor = 1.0f; + } else if (colorFactorIsZero) { + specular.specularColorFactor[0] = specular.specularColorFactor[1] = specular.specularColorFactor[2] = 1.0f; + } + GetMatTex(mat, specular.specularColorTexture, aiTextureType_SPECULAR); + GetMatTex(mat, specular.specularTexture, aiTextureType_SPECULAR); + return true; } bool glTF2Exporter::GetMatSheen(const aiMaterial &mat, glTF2::MaterialSheen &sheen) { @@ -755,7 +755,7 @@ bool glTF2Exporter::GetMatEmissiveStrength(const aiMaterial &mat, glTF2::Materia return mat.Get(AI_MATKEY_EMISSIVE_INTENSITY, emissiveStrength.emissiveStrength) == aiReturn_SUCCESS; } -void glTF2Exporter::ExportMaterials() { +void glTF2Exporter::ExportMaterials(ExportProperties *pProperties) { aiString aiName; for (unsigned int i = 0; i < mScene->mNumMaterials; ++i) { ai_assert(mScene->mMaterials[i] != nullptr); @@ -836,9 +836,9 @@ void glTF2Exporter::ExportMaterials() { m->alphaMode = alphaMode.C_Str(); } - { + // This extension has been deprecated, only export with the specific flag enabled, defaults to false. Uses KHR_material_specular default. + if (pProperties->GetPropertyBool(AI_CONFIG_USE_GLTF_PBR_SPECULAR_GLOSSINESS)) { // KHR_materials_pbrSpecularGlossiness extension - // This extension has been deprecated, only export with the specific flag enabled, defaults to false. Uses KHR_material_specular default. PbrSpecularGlossiness pbrSG; if (GetMatSpecGloss(mat, pbrSG)) { mAsset->extensionsUsed.KHR_materials_pbrSpecularGlossiness = true; diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index f298e7037..4a3ee4928 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -292,7 +292,6 @@ static aiMaterial *ImportMaterial(std::vector &embeddedTexIdxs, Asset &r, M // pbrSpecularGlossiness else if (mat.pbrSpecularGlossiness.isPresent) { PbrSpecularGlossiness &pbrSG = mat.pbrSpecularGlossiness.value; - aimat->AddProperty(new int(1), 1, AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS); SetMaterialColorProperty(r, pbrSG.diffuseFactor, aimat, AI_MATKEY_COLOR_DIFFUSE); SetMaterialColorProperty(r, pbrSG.specularFactor, aimat, AI_MATKEY_COLOR_SPECULAR); diff --git a/include/assimp/config.h.in b/include/assimp/config.h.in index ad16fa88c..9e843a20d 100644 --- a/include/assimp/config.h.in +++ b/include/assimp/config.h.in @@ -1065,6 +1065,17 @@ enum aiComponent */ #define AI_CONFIG_EXPORT_POINT_CLOUDS "EXPORT_POINT_CLOUDS" +/** @brief Specifies whether to use the deprecated KHR_materials_pbrSpecularGlossiness extension + * + * When this flag is undefined any material with specularity will use the new KHR_materials_specular + * extension. Enabling this flag will revert to the deprecated extension. Note that exporting + * KHR_materials_pbrSpecularGlossiness with extensions other than KHR_materials_unlit is unsupported, + * including the basic pbrMetallicRoughness spec. + * + * Property type: Bool. Default value: false. + */ +#define AI_CONFIG_USE_GLTF_PBR_SPECULAR_GLOSSINESS "USE_GLTF_PBR_SPECULAR_GLOSSINESS" + /** * @brief Specifies the blob name, assimp uses for exporting. * diff --git a/include/assimp/material.h b/include/assimp/material.h index f583ad8a8..80551e53d 100644 --- a/include/assimp/material.h +++ b/include/assimp/material.h @@ -1000,7 +1000,6 @@ extern "C" { // Specular Color. // Note: Metallic/Roughness may also have a Specular Color // AI_MATKEY_COLOR_SPECULAR -#define AI_MATKEY_USE_GLTF_PBR_SPECULAR_GLOSSINESS "$mat.useGltfPbrSpecularGlossiness", 0, 0 #define AI_MATKEY_SPECULAR_FACTOR "$mat.specularFactor", 0, 0 // Glossiness factor. 0.0 = Completely Rough, 1.0 = Perfectly Smooth #define AI_MATKEY_GLOSSINESS_FACTOR "$mat.glossinessFactor", 0, 0 diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp index ef3fc4137..c7d01fbcb 100644 --- a/test/unit/utglTF2ImportExport.cpp +++ b/test/unit/utglTF2ImportExport.cpp @@ -220,7 +220,9 @@ TEST_F(utglTF2ImportExport, importglTF2AndExport_KHR_materials_pbrSpecularGlossi aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); // Export - EXPECT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxTextured-glTF-pbrSpecularGlossiness/BoxTextured_out.glb")); + ExportProperties props; + props.SetPropertyBool(, true); + EXPECT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxTextured-glTF-pbrSpecularGlossiness/BoxTextured_out.glb", 0, &props)); // And re-import EXPECT_TRUE(importerMatTest(ASSIMP_TEST_MODELS_DIR "/glTF2/BoxTextured-glTF-pbrSpecularGlossiness/BoxTextured_out.glb", true)); From b6ecba9114e4bfa5edcf93b9f7ceea5c73b4fd9b Mon Sep 17 00:00:00 2001 From: Adam Beili <54665621+Beilinson@users.noreply.github.com> Date: Sun, 26 Mar 2023 17:03:46 +0200 Subject: [PATCH 15/28] fix --- code/AssetLib/glTF2/glTF2Exporter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Exporter.cpp b/code/AssetLib/glTF2/glTF2Exporter.cpp index 3a1441ad9..777df50fd 100644 --- a/code/AssetLib/glTF2/glTF2Exporter.cpp +++ b/code/AssetLib/glTF2/glTF2Exporter.cpp @@ -755,7 +755,7 @@ bool glTF2Exporter::GetMatEmissiveStrength(const aiMaterial &mat, glTF2::Materia return mat.Get(AI_MATKEY_EMISSIVE_INTENSITY, emissiveStrength.emissiveStrength) == aiReturn_SUCCESS; } -void glTF2Exporter::ExportMaterials(ExportProperties *pProperties) { +void glTF2Exporter::ExportMaterials() { aiString aiName; for (unsigned int i = 0; i < mScene->mNumMaterials; ++i) { ai_assert(mScene->mMaterials[i] != nullptr); @@ -837,7 +837,7 @@ void glTF2Exporter::ExportMaterials(ExportProperties *pProperties) { } // This extension has been deprecated, only export with the specific flag enabled, defaults to false. Uses KHR_material_specular default. - if (pProperties->GetPropertyBool(AI_CONFIG_USE_GLTF_PBR_SPECULAR_GLOSSINESS)) { + if (mProperties->GetPropertyBool(AI_CONFIG_USE_GLTF_PBR_SPECULAR_GLOSSINESS)) { // KHR_materials_pbrSpecularGlossiness extension PbrSpecularGlossiness pbrSG; if (GetMatSpecGloss(mat, pbrSG)) { From 8ac0af5c58788e48412dc19fca49808fdd357c3f Mon Sep 17 00:00:00 2001 From: Adam Beili <54665621+Beilinson@users.noreply.github.com> Date: Sun, 26 Mar 2023 17:13:16 +0200 Subject: [PATCH 16/28] . --- code/AssetLib/glTF2/glTF2Asset.inl | 2 +- code/AssetLib/glTF2/glTF2AssetWriter.inl | 2 +- test/unit/utglTF2ImportExport.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Asset.inl b/code/AssetLib/glTF2/glTF2Asset.inl index db773513b..3a3ba376e 100644 --- a/code/AssetLib/glTF2/glTF2Asset.inl +++ b/code/AssetLib/glTF2/glTF2Asset.inl @@ -1236,7 +1236,7 @@ inline void Material::Read(Value &material, Asset &r) { ReadMember(material, "alphaCutoff", this->alphaCutoff); if (Value *extensions = FindObject(material, "extensions")) { - if (r.extensionsUsed.KHR_materials_pbrSpecularGlossiness) { //TODO: Maybe ignore this if KHR_materials_specular is also defined + if (r.extensionsUsed.KHR_materials_pbrSpecularGlossiness) { if (Value *curPbrSpecularGlossiness = FindObject(*extensions, "KHR_materials_pbrSpecularGlossiness")) { PbrSpecularGlossiness pbrSG; diff --git a/code/AssetLib/glTF2/glTF2AssetWriter.inl b/code/AssetLib/glTF2/glTF2AssetWriter.inl index ef1c66b3b..f6fc7adff 100644 --- a/code/AssetLib/glTF2/glTF2AssetWriter.inl +++ b/code/AssetLib/glTF2/glTF2AssetWriter.inl @@ -424,7 +424,7 @@ namespace glTF2 { MaterialSpecular &specular = m.materialSpecular.value; - if (specular.specularFactor != 0.f) { + if (specular.specularFactor != 0.0f) { WriteFloat(materialSpecular, specular.specularFactor, "specularFactor", w.mAl); WriteTex(materialSpecular, specular.specularTexture, "specularTexture", w.mAl); } diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp index c7d01fbcb..b63fd3944 100644 --- a/test/unit/utglTF2ImportExport.cpp +++ b/test/unit/utglTF2ImportExport.cpp @@ -221,7 +221,7 @@ TEST_F(utglTF2ImportExport, importglTF2AndExport_KHR_materials_pbrSpecularGlossi EXPECT_NE(nullptr, scene); // Export ExportProperties props; - props.SetPropertyBool(, true); + props.SetPropertyBool(AI_CONFIG_USE_GLTF_PBR_SPECULAR_GLOSSINESS, true); EXPECT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxTextured-glTF-pbrSpecularGlossiness/BoxTextured_out.glb", 0, &props)); // And re-import From 9ea37b5330937a8aa4e99b87b847a6a376130557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lutz=20H=C3=B6ren?= Date: Thu, 11 May 2023 08:00:25 +0200 Subject: [PATCH 17/28] copy aiMetadata objects in scene combiner --- code/Common/SceneCombiner.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/Common/SceneCombiner.cpp b/code/Common/SceneCombiner.cpp index 0f5386b26..0188f5dea 100644 --- a/code/Common/SceneCombiner.cpp +++ b/code/Common/SceneCombiner.cpp @@ -1349,6 +1349,9 @@ void SceneCombiner::Copy(aiMetadata **_dest, const aiMetadata *src) { case AI_AIVECTOR3D: out.mData = new aiVector3D(*static_cast(in.mData)); break; + case AI_AIMETADATA: + out.mData = new aiMetadata(*static_cast(in.mData)); + break; default: ai_assert(false); break; From 9d6b32f5c5118e67e99d4be3087f49ad0b7660de Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 15 May 2023 09:18:30 +0200 Subject: [PATCH 18/28] Update: Enable export for fuzzer tests --- code/Common/Exporter.cpp | 2 +- fuzz/assimp_fuzzer.cc | 8 +++++ test/unit/utExport.cpp | 64 +++++++++++++++++++++++++++++----------- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/code/Common/Exporter.cpp b/code/Common/Exporter.cpp index b8dabfff4..0a5927ce7 100644 --- a/code/Common/Exporter.cpp +++ b/code/Common/Exporter.cpp @@ -483,7 +483,7 @@ aiReturn Exporter::Export( const aiScene* pScene, const char* pFormatId, const c } pimpl->mProgressHandler->UpdateFileWrite(3, 4); - + if(must_join_again) { JoinVerticesProcess proc; proc.Execute(scenecopy.get()); diff --git a/fuzz/assimp_fuzzer.cc b/fuzz/assimp_fuzzer.cc index edb5fdbb5..e347f59cb 100644 --- a/fuzz/assimp_fuzzer.cc +++ b/fuzz/assimp_fuzzer.cc @@ -40,6 +40,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ #include #include +#include #include #include @@ -53,6 +54,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t dataSize) { const aiScene *sc = importer.ReadFileFromMemory(data, dataSize, aiProcessPreset_TargetRealtime_Quality, nullptr ); + if (sc == nullptr) { + return 0; + } + + Exporter exporter; + exporter.ExportToBlob(sc, "fbx"); + aiDetachLogStream(&stream); return 0; diff --git a/test/unit/utExport.cpp b/test/unit/utExport.cpp index b3ab5e372..cb7826bfc 100644 --- a/test/unit/utExport.cpp +++ b/test/unit/utExport.cpp @@ -1,38 +1,71 @@ -#include "UnitTestPCH.h" +/* +--------------------------------------------------------------------------- +Open Asset Import Library (assimp) +--------------------------------------------------------------------------- +Copyright (c) 2006-2022, assimp team + +All rights reserved. + +Redistribution and use of this software in source and binary forms, +with or without modification, are permitted provided that the following +conditions are met: + +* Redistributions of source code must retain the above +copyright notice, this list of conditions and the +following disclaimer. + +* Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the +following disclaimer in the documentation and/or other +materials provided with the distribution. + +* Neither the name of the assimp team, nor the names of its +contributors may be used to endorse or promote products +derived from this software without specific prior +written permission of the assimp team. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +--------------------------------------------------------------------------- +*/ +#include "UnitTestPCH.h" #include #include - #ifndef ASSIMP_BUILD_NO_EXPORT class ExporterTest : public ::testing::Test { public: - - virtual void SetUp() - { + void SetUp() override { ex = new Assimp::Exporter(); im = new Assimp::Importer(); pTest = im->ReadFile(ASSIMP_TEST_MODELS_DIR "/X/test.x", aiProcess_ValidateDataStructure); } - virtual void TearDown() - { + void TearDown() override { delete ex; delete im; } protected: - const aiScene* pTest; Assimp::Exporter* ex; Assimp::Importer* im; }; // ------------------------------------------------------------------------------------------------ -TEST_F(ExporterTest, testExportToFile) -{ +TEST_F(ExporterTest, testExportToFile) { const char* file = "unittest_output.dae"; EXPECT_EQ(AI_SUCCESS,ex->Export(pTest,"collada",file)); @@ -41,8 +74,7 @@ TEST_F(ExporterTest, testExportToFile) } // ------------------------------------------------------------------------------------------------ -TEST_F(ExporterTest, testExportToBlob) -{ +TEST_F(ExporterTest, testExportToBlob) { const aiExportDataBlob* blob = ex->ExportToBlob(pTest,"collada"); ASSERT_TRUE(blob); EXPECT_TRUE(blob->data); @@ -56,8 +88,7 @@ TEST_F(ExporterTest, testExportToBlob) } // ------------------------------------------------------------------------------------------------ -TEST_F(ExporterTest, testCppExportInterface) -{ +TEST_F(ExporterTest, testCppExportInterface) { EXPECT_TRUE(ex->GetExportFormatCount() > 0); for(size_t i = 0; i < ex->GetExportFormatCount(); ++i) { const aiExportFormatDesc* const desc = ex->GetExportFormatDescription(i); @@ -71,14 +102,13 @@ TEST_F(ExporterTest, testCppExportInterface) } // ------------------------------------------------------------------------------------------------ -TEST_F(ExporterTest, testCExportInterface) -{ +TEST_F(ExporterTest, testCExportInterface) { EXPECT_TRUE(aiGetExportFormatCount() > 0); for(size_t i = 0; i < aiGetExportFormatCount(); ++i) { const aiExportFormatDesc* const desc = aiGetExportFormatDescription(i); EXPECT_TRUE(desc); - // rest has already been validated by testCppExportInterface } } #endif + From 478ce8af5a38b8536bd0b507aa071be6a9054898 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 15 May 2023 09:25:44 +0200 Subject: [PATCH 19/28] Update Exporter.cpp --- code/Common/Exporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/Common/Exporter.cpp b/code/Common/Exporter.cpp index 0a5927ce7..b8dabfff4 100644 --- a/code/Common/Exporter.cpp +++ b/code/Common/Exporter.cpp @@ -483,7 +483,7 @@ aiReturn Exporter::Export( const aiScene* pScene, const char* pFormatId, const c } pimpl->mProgressHandler->UpdateFileWrite(3, 4); - + if(must_join_again) { JoinVerticesProcess proc; proc.Execute(scenecopy.get()); From 121f09b62a81255b8a2d169de121a6df5e5be6f6 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 15 May 2023 11:02:58 +0200 Subject: [PATCH 20/28] Fix: Fix the build. --- code/AssetLib/FBX/FBXParser.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/code/AssetLib/FBX/FBXParser.h b/code/AssetLib/FBX/FBXParser.h index 67707342e..5f231738d 100644 --- a/code/AssetLib/FBX/FBXParser.h +++ b/code/AssetLib/FBX/FBXParser.h @@ -64,10 +64,9 @@ class Parser; class Element; // XXX should use C++11's unique_ptr - but assimp's need to keep working with 03 -typedef std::vector< Scope* > ScopeList; -typedef std::fbx_unordered_multimap< std::string, Element* > ElementMap; - -typedef std::pair ElementCollection; +using ScopeList = std::vector; +using ElementMap = std::fbx_unordered_multimap< std::string, Element*>; +using ElementCollection = std::pair; #define new_Scope new (allocator.Allocate(sizeof(Scope))) Scope #define new_Element new (allocator.Allocate(sizeof(Element))) Element @@ -84,12 +83,13 @@ typedef std::pair Element * @endverbatim * * As can be seen in this sample, elements can contain nested #Scope - * as their trailing member. **/ + * as their trailing member. +**/ class Element { public: Element(const Token& key_token, Parser& parser); - ~Element(): + ~Element(); const Scope* Compound() const { return compound; From ad399adf4bc677c84a62c89b32d4187b628d904c Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 15 May 2023 13:19:02 +0200 Subject: [PATCH 21/28] Add missing include --- code/Common/StackAllocator.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/code/Common/StackAllocator.h b/code/Common/StackAllocator.h index aff36fa47..813d8ed5b 100644 --- a/code/Common/StackAllocator.h +++ b/code/Common/StackAllocator.h @@ -4,7 +4,6 @@ Open Asset Import Library (assimp) Copyright (c) 2006-2022, assimp team - All rights reserved. Redistribution and use of this software in source and binary forms, @@ -36,7 +35,6 @@ DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - ---------------------------------------------------------------------- */ @@ -52,9 +50,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include -namespace Assimp -{ +namespace Assimp { /** @brief A very bare-bone allocator class that is suitable when * allocating many small objects, e.g. during parsing. From b5b64003200909408d422e9d1a0ec4155a062427 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 15 May 2023 15:08:52 +0200 Subject: [PATCH 22/28] Fix: Use vector. --- code/Common/StackAllocator.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/code/Common/StackAllocator.h b/code/Common/StackAllocator.h index 813d8ed5b..f0b712b24 100644 --- a/code/Common/StackAllocator.h +++ b/code/Common/StackAllocator.h @@ -47,8 +47,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef AI_STACK_ALLOCATOR_H_INC #define AI_STACK_ALLOCATOR_H_INC - -#include +#include #include #include @@ -83,10 +82,9 @@ private: constexpr const static size_t g_startBytesPerBlock = 16 * 1024; // Size of the first block. Next blocks will double in size until maximum size of g_maxBytesPerBlock size_t m_blockAllocationSize = g_startBytesPerBlock; // Block size of the current block size_t m_subIndex = g_maxBytesPerBlock; // The current byte offset in the current block - std::deque m_storageBlocks; // A list of blocks + std::vector m_storageBlocks; // A list of blocks }; - } // namespace Assimp #include "StackAllocator.inl" From 8ad4bb0b2c54259a0206e63f888dcea036d46501 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 15 May 2023 15:09:54 +0200 Subject: [PATCH 23/28] Update StackAllocator.inl --- code/Common/StackAllocator.inl | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/code/Common/StackAllocator.inl b/code/Common/StackAllocator.inl index d973b7794..2c3164ca7 100644 --- a/code/Common/StackAllocator.inl +++ b/code/Common/StackAllocator.inl @@ -4,7 +4,6 @@ Open Asset Import Library (assimp) Copyright (c) 2006-2022, assimp team - All rights reserved. Redistribution and use of this software in source and binary forms, @@ -45,7 +44,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. using namespace Assimp; - inline StackAllocator::StackAllocator() { } @@ -60,7 +58,7 @@ inline void *StackAllocator::Allocate(size_t byteSize) { // Block size must be at least as large as byteSize, but we want to use this for small allocations anyway. m_blockAllocationSize = std::max(std::min(m_blockAllocationSize * 2, g_maxBytesPerBlock), byteSize); uint8_t *data = new uint8_t[m_blockAllocationSize]; - m_storageBlocks.push_back(data); + m_storageBlocks.emplace_back(data); m_subIndex = byteSize; return data; } @@ -76,7 +74,7 @@ inline void StackAllocator::FreeAll() { for (size_t i = 0; i < m_storageBlocks.size(); i++) { delete [] m_storageBlocks[i]; } - std::deque empty; + std::vector empty; m_storageBlocks.swap(empty); // start over: m_blockAllocationSize = g_startBytesPerBlock; From 71a4977dd104506e8af743f27e1db2b062ccc1a1 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 15 May 2023 15:46:21 +0200 Subject: [PATCH 24/28] Fix: Try unique_ptr --- code/Common/StackAllocator.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/Common/StackAllocator.h b/code/Common/StackAllocator.h index f0b712b24..33a8bb569 100644 --- a/code/Common/StackAllocator.h +++ b/code/Common/StackAllocator.h @@ -82,7 +82,8 @@ private: constexpr const static size_t g_startBytesPerBlock = 16 * 1024; // Size of the first block. Next blocks will double in size until maximum size of g_maxBytesPerBlock size_t m_blockAllocationSize = g_startBytesPerBlock; // Block size of the current block size_t m_subIndex = g_maxBytesPerBlock; // The current byte offset in the current block - std::vector m_storageBlocks; // A list of blocks + std::vector> m_storageBlocks; + //std::vector m_storageBlocks; // A list of blocks }; } // namespace Assimp From 94905d445fe658d69e2abc6cd4a740bf29ecb249 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Mon, 15 May 2023 16:06:02 +0200 Subject: [PATCH 25/28] Revert usage of unique_ptr - error. --- code/Common/StackAllocator.h | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/code/Common/StackAllocator.h b/code/Common/StackAllocator.h index 33a8bb569..191010cac 100644 --- a/code/Common/StackAllocator.h +++ b/code/Common/StackAllocator.h @@ -60,21 +60,21 @@ namespace Assimp { */ class StackAllocator { public: - // Constructs the allocator + /// @brief Constructs the allocator inline StackAllocator(); - // Destructs the allocator and frees all memory + /// @brief Destructs the allocator and frees all memory inline ~StackAllocator(); // non copyable StackAllocator(const StackAllocator &) = delete; StackAllocator &operator=(const StackAllocator &) = delete; - // Returns a pointer to byteSize bytes of heap memory that persists - // for the lifetime of the allocator (or until FreeAll is called). + /// @brief Returns a pointer to byteSize bytes of heap memory that persists + /// for the lifetime of the allocator (or until FreeAll is called). inline void *Allocate(size_t byteSize); - // Releases all the memory owned by this allocator. - // Memory provided through function Allocate is not valid anymore after this function has been called. + /// @brief Releases all the memory owned by this allocator. + // Memory provided through function Allocate is not valid anymore after this function has been called. inline void FreeAll(); private: @@ -82,8 +82,7 @@ private: constexpr const static size_t g_startBytesPerBlock = 16 * 1024; // Size of the first block. Next blocks will double in size until maximum size of g_maxBytesPerBlock size_t m_blockAllocationSize = g_startBytesPerBlock; // Block size of the current block size_t m_subIndex = g_maxBytesPerBlock; // The current byte offset in the current block - std::vector> m_storageBlocks; - //std::vector m_storageBlocks; // A list of blocks + std::vector m_storageBlocks; // A list of blocks }; } // namespace Assimp From 9c45f727e3249fa8eaa03957e481cb543be69640 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sat, 20 May 2023 14:27:57 +0200 Subject: [PATCH 26/28] Update ASELoader.cpp --- code/AssetLib/ASE/ASELoader.cpp | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/code/AssetLib/ASE/ASELoader.cpp b/code/AssetLib/ASE/ASELoader.cpp index 951e8539d..4617c9ed4 100644 --- a/code/AssetLib/ASE/ASELoader.cpp +++ b/code/AssetLib/ASE/ASELoader.cpp @@ -44,7 +44,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ #ifndef ASSIMP_BUILD_NO_ASE_IMPORTER - #ifndef ASSIMP_BUILD_NO_3DS_IMPORTER // internal headers @@ -322,21 +321,6 @@ void ASEImporter::BuildAnimations(const std::vector &nodes) { aiNodeAnim *nd = pcAnim->mChannels[iNum++] = new aiNodeAnim(); nd->mNodeName.Set(me->mName + ".Target"); - // If there is no input position channel we will need - // to supply the default position from the node's - // local transformation matrix. - /*TargetAnimationHelper helper; - if (me->mAnim.akeyPositions.empty()) - { - aiMatrix4x4& mat = (*i)->mTransform; - helper.SetFixedMainAnimationChannel(aiVector3D( - mat.a4, mat.b4, mat.c4)); - } - else helper.SetMainAnimationChannel (&me->mAnim.akeyPositions); - helper.SetTargetAnimationChannel (&me->mTargetAnim.akeyPositions); - - helper.Process(&me->mTargetAnim.akeyPositions);*/ - // Allocate the key array and fill it nd->mNumPositionKeys = (unsigned int)me->mTargetAnim.akeyPositions.size(); nd->mPositionKeys = new aiVectorKey[nd->mNumPositionKeys]; From 3a09fd0c859cafea48dd272ac3a66bced00b5ee8 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 23 May 2023 09:50:20 +0200 Subject: [PATCH 27/28] Fix review finding Test with glossiness disabled and enabled. --- test/unit/utglTF2ImportExport.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp index 56f79d307..91ac87ee5 100644 --- a/test/unit/utglTF2ImportExport.cpp +++ b/test/unit/utglTF2ImportExport.cpp @@ -219,7 +219,11 @@ TEST_F(utglTF2ImportExport, importglTF2AndExport_KHR_materials_pbrSpecularGlossi const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/BoxTextured-glTF-pbrSpecularGlossiness/BoxTextured.gltf", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - // Export + + // Export with specular glossiness disabled + EXPECT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxTextured-glTF-pbrSpecularGlossiness/BoxTextured_out.glb")); + + // Export with specular glossiness enabled ExportProperties props; props.SetPropertyBool(AI_CONFIG_USE_GLTF_PBR_SPECULAR_GLOSSINESS, true); EXPECT_EQ(aiReturn_SUCCESS, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxTextured-glTF-pbrSpecularGlossiness/BoxTextured_out.glb", 0, &props)); From b7a8c4ba75501554cec5069ad67c6fe6508db714 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 23 May 2023 10:33:14 +0200 Subject: [PATCH 28/28] Update glTF2Asset.inl --- code/AssetLib/glTF2/glTF2Asset.inl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/AssetLib/glTF2/glTF2Asset.inl b/code/AssetLib/glTF2/glTF2Asset.inl index da5ef182b..bd6435766 100644 --- a/code/AssetLib/glTF2/glTF2Asset.inl +++ b/code/AssetLib/glTF2/glTF2Asset.inl @@ -1250,7 +1250,7 @@ inline void Material::Read(Value &material, Asset &r) { ReadMember(material, "alphaCutoff", this->alphaCutoff); if (Value *extensions = FindObject(material, "extensions")) { - if (r.extensionsUsed.KHR_materials_pbrSpecularGlossiness) { + if (r.extensionsUsed.KHR_materials_pbrSpecularGlossiness) { if (Value *curPbrSpecularGlossiness = FindObject(*extensions, "KHR_materials_pbrSpecularGlossiness")) { PbrSpecularGlossiness pbrSG;