From faaea5224f0704d85eef57f9e55dfd8df771e768 Mon Sep 17 00:00:00 2001 From: acgessler Date: Sat, 5 Oct 2013 16:18:36 +0200 Subject: [PATCH 1/4] Exporter: update documentation. --- include/assimp/Exporter.hpp | 8 +++++++- include/assimp/cexport.h | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/include/assimp/Exporter.hpp b/include/assimp/Exporter.hpp index 1a993db48..b9318ae29 100644 --- a/include/assimp/Exporter.hpp +++ b/include/assimp/Exporter.hpp @@ -198,7 +198,13 @@ public: * redundant as exporters would apply them anyhow. A good example * is triangulation - whilst you can enforce it by specifying * the #aiProcess_Triangulate flag, most export formats support only - * triangulate data so they would run the step even if it wasn't requested. + * triangulate data so they would run the step even if it wasn't requested. + * + * If assimp detects that the input scene was directly taken from the importer side of + * the library (i.e. not copied using aiCopyScene and potetially modified afterwards), + * any postprocessing steps already applied to the scene will not be applied again, unless + * they show non-idempotent behaviour (#aiProcess_MakeLeftHanded, #aiProcess_FlipUVs and + * #aiProcess_FlipWindingOrder). * @return AI_SUCCESS if everything was fine. * @note Use aiCopyScene() to get a modifiable copy of a previously * imported scene.*/ diff --git a/include/assimp/cexport.h b/include/assimp/cexport.h index 797d5db1a..5916c2da9 100644 --- a/include/assimp/cexport.h +++ b/include/assimp/cexport.h @@ -143,6 +143,12 @@ ASSIMP_API void aiFreeScene(const C_STRUCT aiScene* pIn); * is triangulation - whilst you can enforce it by specifying * the #aiProcess_Triangulate flag, most export formats support only * triangulate data so they would run the step anyway. +* +* If assimp detects that the input scene was directly taken from the importer side of +* the library (i.e. not copied using aiCopyScene and potetially modified afterwards), +* any postprocessing steps already applied to the scene will not be applied again, unless +* they show non-idempotent behaviour (#aiProcess_MakeLeftHanded, #aiProcess_FlipUVs and +* #aiProcess_FlipWindingOrder). * @return a status code indicating the result of the export * @note Use aiCopyScene() to get a modifiable copy of a previously * imported scene. From 3bee2818e4584e401358615f37c29511f96f0690 Mon Sep 17 00:00:00 2001 From: acgessler Date: Sat, 5 Oct 2013 16:19:03 +0200 Subject: [PATCH 2/4] Exporter: stop non intuitive automatic applying of "reverse" pp steps if user specifies no postprocessing steps. --- code/Exporter.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/code/Exporter.cpp b/code/Exporter.cpp index 3a2a5ec95..0f7c8f6d5 100644 --- a/code/Exporter.cpp +++ b/code/Exporter.cpp @@ -289,15 +289,16 @@ aiReturn Exporter :: Export( const aiScene* pScene, const char* pFormatId, const const unsigned int nonIdempotentSteps = aiProcess_FlipWindingOrder | aiProcess_FlipUVs | aiProcess_MakeLeftHanded; // Erase all pp steps that were already applied to this scene - unsigned int pp = (exp.mEnforcePP | pPreprocessing) & ~(priv + const unsigned int pp = (exp.mEnforcePP | pPreprocessing) & ~(priv ? (priv->mPPStepsApplied & ~nonIdempotentSteps) : 0u); // If no extra postprocessing was specified, and we obtained this scene from an // Assimp importer, apply the reverse steps automatically. - if (!pPreprocessing && priv) { - pp |= (nonIdempotentSteps & priv->mPPStepsApplied); - } + // TODO: either drop this, or document it. Otherwise it is just a bad surprise. + //if (!pPreprocessing && priv) { + // pp |= (nonIdempotentSteps & priv->mPPStepsApplied); + //} // If the input scene is not in verbose format, but there is at least postprocessing step that relies on it, // we need to run the MakeVerboseFormat step first. From 9d4f7c32e49f75bd84fc77eb9d539190a5d59d00 Mon Sep 17 00:00:00 2001 From: acgessler Date: Sat, 5 Oct 2013 16:26:16 +0200 Subject: [PATCH 3/4] Scene: keep an internal flag whether the scene was copied using aiCopyScene() (and potentially modified by the user afterwards) --- code/AssimpCExport.cpp | 1 + code/ScenePrivate.h | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/code/AssimpCExport.cpp b/code/AssimpCExport.cpp index 9483eb2d3..053e1b9f4 100644 --- a/code/AssimpCExport.cpp +++ b/code/AssimpCExport.cpp @@ -73,6 +73,7 @@ ASSIMP_API void aiCopyScene(const aiScene* pIn, aiScene** pOut) } SceneCombiner::CopyScene(pOut,pIn,true); + ScenePriv(*pOut)->mIsCopy = true; } diff --git a/code/ScenePrivate.h b/code/ScenePrivate.h index dc11bb4fa..8e9fde7ac 100644 --- a/code/ScenePrivate.h +++ b/code/ScenePrivate.h @@ -53,6 +53,7 @@ struct ScenePrivateData { ScenePrivateData() : mOrigImporter() , mPPStepsApplied() + , mIsCopy() {} // Importer that originally loaded the scene though the C-API @@ -61,6 +62,13 @@ struct ScenePrivateData { // List of postprocessing steps already applied to the scene. unsigned int mPPStepsApplied; + + // true if the scene is a copy made with aiCopyScene() + // or the corresponding C++ API. This means that user code + // may have made modifications to it, so mPPStepsApplied + // and mOrigImporter are no longer safe to rely on and only + // serve informative purposes. + bool mIsCopy; }; // Access private data stored in the scene From ad0b2146562b28a6bea13e2a0511b314aabeb423 Mon Sep 17 00:00:00 2001 From: acgessler Date: Sat, 5 Oct 2013 16:26:52 +0200 Subject: [PATCH 4/4] Exporter: do not make any assumptions on previously run PP steps if the scene is a copy. --- code/Exporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/Exporter.cpp b/code/Exporter.cpp index 0f7c8f6d5..306ed824a 100644 --- a/code/Exporter.cpp +++ b/code/Exporter.cpp @@ -289,7 +289,7 @@ aiReturn Exporter :: Export( const aiScene* pScene, const char* pFormatId, const const unsigned int nonIdempotentSteps = aiProcess_FlipWindingOrder | aiProcess_FlipUVs | aiProcess_MakeLeftHanded; // Erase all pp steps that were already applied to this scene - const unsigned int pp = (exp.mEnforcePP | pPreprocessing) & ~(priv + const unsigned int pp = (exp.mEnforcePP | pPreprocessing) & ~(priv && !priv->mIsCopy ? (priv->mPPStepsApplied & ~nonIdempotentSteps) : 0u);