[Bf-blender-cvs] [4474c45e612] usd-importer-T81257-merge: USD Import: fix reader refcount error.
makowalski
noreply at git.blender.org
Fri Mar 12 05:00:23 CET 2021
Commit: 4474c45e6129461c34905ae96caa5cc0d73c37b4
Author: makowalski
Date: Thu Mar 11 22:53:39 2021 -0500
Branches: usd-importer-T81257-merge
https://developer.blender.org/rB4474c45e6129461c34905ae96caa5cc0d73c37b4
USD Import: fix reader refcount error.
Currently, USDPrimReader reference counts aren't correctly
maintained because the readers are stored in two separate
arrays, in the USDStageReader instance and also in the
import job data, but the reference counts for the readers
is incremented only once. I changed the logic to store
the readers only in the USDStageReader. Now also deleting
the stage reader instance, fixing a small memory leak. Also,
simplified the syntax for for looping over readers, to make
the code more readable.
===================================================================
M source/blender/io/usd/intern/usd_capi.cc
M source/blender/io/usd/intern/usd_reader_stage.cc
M source/blender/io/usd/intern/usd_reader_stage.h
===================================================================
diff --git a/source/blender/io/usd/intern/usd_capi.cc b/source/blender/io/usd/intern/usd_capi.cc
index 3742d96ed90..50335f25f07 100644
--- a/source/blender/io/usd/intern/usd_capi.cc
+++ b/source/blender/io/usd/intern/usd_capi.cc
@@ -483,7 +483,6 @@ struct ImportJobData {
ImportSettings settings;
USDStageReader *archive;
- std::vector<USDPrimReader *> readers;
short *stop;
short *do_update;
@@ -502,6 +501,7 @@ static void import_startjob(void *customdata, short *stop, short *do_update, flo
data->do_update = do_update;
data->progress = progress;
data->was_canceled = false;
+ data->archive = nullptr;
// G.is_rendering = true;
WM_set_locked_interface(data->wm, true);
@@ -581,11 +581,11 @@ static void import_startjob(void *customdata, short *stop, short *do_update, flo
*data->progress = 0.15f;
- data->readers = archive->collect_readers(data->bmain, data->params, data->settings);
+ archive->collect_readers(data->bmain, data->params, data->settings);
*data->progress = 0.2f;
- const float size = static_cast<float>(data->readers.size());
+ const float size = static_cast<float>(archive->readers().size());
size_t i = 0;
/* Setup parenthood */
@@ -601,7 +601,7 @@ static void import_startjob(void *customdata, short *stop, short *do_update, flo
}
/* TODO(makowalski): Here and below, should we call
- * read_object_data() with the actual time? */
+ * read_object_data() with the actual time? */
reader->read_object_data(data->bmain, 0.0);
Object *ob = reader->object();
@@ -618,17 +618,19 @@ static void import_startjob(void *customdata, short *stop, short *do_update, flo
}
}
- std::vector<USDPrimReader *>::iterator iter;
+ for (USDPrimReader *reader : archive->readers()) {
- for (iter = data->readers.begin(); iter != data->readers.end(); ++iter) {
- Object *ob = (*iter)->object();
+ if (!reader) {
+ continue;
+ }
+
+ Object *ob = reader->object();
- USDPrimReader *reader = (*iter);
reader->read_object_data(data->bmain, 0.0);
- USDPrimReader *parent = (*iter)->parent();
+ USDPrimReader *parent = reader->parent();
- if (parent == NULL /*|| !reader->inherits_xform()*/) {
+ if (parent == NULL) {
ob->parent = NULL;
}
else {
@@ -654,36 +656,34 @@ static void import_endjob(void *customdata)
{
ImportJobData *data = static_cast<ImportJobData *>(customdata);
- std::vector<USDPrimReader *>::iterator iter;
-
/* Delete objects on cancellation. */
- if (data->was_canceled) {
- for (iter = data->readers.begin(); iter != data->readers.end(); ++iter) {
- Object *ob = (*iter)->object();
+ if (data->was_canceled && data->archive) {
- /* It's possible that cancellation occurred between the creation of
- * the reader and the creation of the Blender object. */
- if (ob == NULL) {
+ for (USDPrimReader *reader : data->archive->readers()) {
+
+ if (!reader) {
continue;
}
- BKE_id_free_us(data->bmain, ob);
+ /* It's possible that cancellation occurred between the creation of
+ * the reader and the creation of the Blender object. */
+ if (Object *ob = reader->object()) {
+ BKE_id_free_us(data->bmain, ob);
+ }
}
- if (data->archive) {
- for (const auto &pair : data->archive->proto_readers()) {
- for (USDPrimReader *reader : pair.second) {
- Object *ob = reader->object();
- /* It's possible that cancellation occurred between the creation of
- * the reader and the creation of the Blender object. */
- if (ob != NULL) {
- BKE_id_free_us(data->bmain, ob);
- }
+ for (const auto &pair : data->archive->proto_readers()) {
+ for (USDPrimReader *reader : pair.second) {
+
+ /* It's possible that cancellation occurred between the creation of
+ * the reader and the creation of the Blender object. */
+ if (Object *ob = reader->object()) {
+ BKE_id_free_us(data->bmain, ob);
}
}
}
}
- else {
+ else if (data->archive) {
/* Add object to scene. */
Base *base;
LayerCollection *lc;
@@ -693,13 +693,25 @@ static void import_endjob(void *customdata)
lc = BKE_layer_collection_get_active(view_layer);
- if (data->archive && !data->archive->proto_readers().empty()) {
- create_proto_collections(
- data->bmain, view_layer, lc->collection, data->archive->proto_readers(), data->readers);
+ if (!data->archive->proto_readers().empty()) {
+ create_proto_collections(data->bmain,
+ view_layer,
+ lc->collection,
+ data->archive->proto_readers(),
+ data->archive->readers());
}
- for (iter = data->readers.begin(); iter != data->readers.end(); ++iter) {
- Object *ob = (*iter)->object();
+ for (USDPrimReader *reader : data->archive->readers()) {
+
+ if (!reader) {
+ continue;
+ }
+
+ Object *ob = reader->object();
+
+ if (!ob) {
+ continue;
+ }
BKE_collection_object_add(data->bmain, lc->collection, ob);
@@ -718,17 +730,6 @@ static void import_endjob(void *customdata)
DEG_relations_tag_update(data->bmain);
}
- for (iter = data->readers.begin(); iter != data->readers.end(); ++iter) {
- USDPrimReader *reader = *iter;
- reader->decref();
-
- if (reader->refcount() == 0) {
- delete reader;
- }
- }
-
- data->archive->clear_proto_readers(true);
-
WM_set_locked_interface(data->wm, false);
switch (data->error_code) {
@@ -748,7 +749,7 @@ static void import_freejob(void *user_data)
{
ImportJobData *data = static_cast<ImportJobData *>(user_data);
- // delete data->archive;
+ delete data->archive;
delete data;
}
diff --git a/source/blender/io/usd/intern/usd_reader_stage.cc b/source/blender/io/usd/intern/usd_reader_stage.cc
index 13492f94c19..e8b1f570998 100644
--- a/source/blender/io/usd/intern/usd_reader_stage.cc
+++ b/source/blender/io/usd/intern/usd_reader_stage.cc
@@ -71,7 +71,8 @@ USDStageReader::USDStageReader(struct Main *bmain, const char *filename)
USDStageReader::~USDStageReader()
{
- clear_readers();
+ clear_readers(true);
+ clear_proto_readers(true);
if (stage_) {
stage_->Unload();
@@ -240,14 +241,15 @@ static USDPrimReader *_handlePrim(Main *bmain,
return reader;
}
-std::vector<USDPrimReader *> USDStageReader::collect_readers(Main *bmain,
- const USDImportParams ¶ms,
- ImportSettings &settings)
+void USDStageReader::collect_readers(Main *bmain,
+ const USDImportParams ¶ms,
+ ImportSettings &settings)
{
params_ = params;
settings_ = settings;
- clear_readers();
+ clear_readers(true);
+ clear_proto_readers(true);
// Iterate through stage
pxr::UsdPrim root = stage_->GetPseudoRoot();
@@ -261,7 +263,7 @@ std::vector<USDPrimReader *> USDStageReader::collect_readers(Main *bmain,
if (prim.IsValid()) {
root = prim;
if (path.ContainsPrimVariantSelection()) {
- // TODO: This will not work properly with setting variants on child prims
+ // TODO(makowalski): This will not work properly with setting variants on child prims
while (path.ContainsPrimVariantSelection()) {
std::pair<std::string, std::string> variantSelection = path.GetVariantSelection();
root.GetVariantSet(variantSelection.first).SetVariantSelection(variantSelection.second);
@@ -284,16 +286,21 @@ std::vector<USDPrimReader *> USDStageReader::collect_readers(Main *bmain,
proto_readers_.insert(std::make_pair(proto_prim.GetPath(), proto_readers));
}
}
-
- return readers_;
}
-void USDStageReader::clear_readers()
+void USDStageReader::clear_readers(bool decref)
{
- std::vector<USDPrimReader *>::iterator iter;
- for (iter = readers_.begin(); iter != readers_.end(); ++iter) {
- if (((USDPrimReader *)*iter)->refcount() == 0) {
- delete *iter;
+ for (USDPrimReader *reader : readers_) {
+ if (!reader) {
+ continue;
+ }
+
+ if (decref) {
+ reader->decref();
+ }
+
+ if (reader->refcount() == 0) {
+ delete reader;
}
}
diff --git a/source/blender/io/usd/intern/usd_reader_stage.h b/source/blender/io/usd/intern/usd_reader_stage.h
index 5c7c5bf91d4..a41b6bc375d 100644
--- a/source/blender/io/usd/intern/usd_reader_stage.h
+++ b/source/blender/io/usd/intern/usd_reader_stage.h
@@ -34,11 +34,6 @@ struct ImportSettings;
namespace blender::io::usd {
-/* Wrappers around input and output archives. The goal is to be able to use
- * streams so that unicode paths work on Windows (T49112), and to make sure that
- * the stream objects remain valid as long as the archives are open.
- */
-
typedef std::map<pxr::SdfPath, std::vector<USDPrimReader *>> ProtoReaderMap;
class USDStageReader {
@@ -66,9 +61,9 @@ class USDStageReader {
// primitive types specified by the user in the import options.
static USDPrimReader *create_reader(class USDStageReader *archive, const pxr::UsdPrim &prim);
- std::vector<USDPrimReader *> collect_readers(struct Main *bmain,
- const USDImportParams ¶ms,
- ImportSettings &settings);
+ void collect_readers(struct Main *bmain,
+ const USDImportParams ¶ms,
+ ImportSettings &settings);
bool valid() const;
@@ -94,14 +89,19 @@ class USDStageReader {
settings_ = a_settings;
}
- void clear_readers();
+ void clear_readers(bool decref = true);
- void clear_proto_readers(bool decref);
+ void clear_proto_readers(bool decref = true);
const ProtoReaderMap &proto_readers() const
{
return proto_readers_;
};
+
+ const std::vector<USDPrimReader *> &readers() const
+ {
+ return readers_;
+ };
};
}; // namespace blender::io::usd
More information about the Bf-blender-cvs
mailing list