IO: Import multiple alembic files at once #121492

Guillermo Venegas · 2024-05-06T17:55:57+02:00

Guillermo Venegas commented

2024-05-06 17:55:57 +02:00

Allows to import multiple alembic files in single operator call,
either by file explorer import window or by drag-and-drop from external sources.

When importing multiple alembic files as background job, import progress will be divided
by all files, so if 5 files are imported, each file will make a progress of 20%.
This can be improved if the job text can be customized to display for example
Import Alembic 1/5 and using 100% progress status display for each file, but that is out
of the scope of this pr.

They are few options like sequence_min_frame,sequence_max_frame that only changes
Scene min and max frame, this set the maximum and minimum detected in all file paths.

Extra notes:
While debugging i get a few error messages related to the importer, but not related to the patch.

Allows to import multiple alembic files in single operator call, either by file explorer import window or by drag-and-drop from external sources. When importing multiple alembic files as background job, import progress will be divided by all files, so if `5` files are imported, each file will make a progress of `20%`. This can be improved if the job text can be customized to display for example `Import Alembic 1/5` and using 100% progress status display for each file, but that is out of the scope of this pr. They are few options like `sequence_min_frame`,`sequence_max_frame` that only changes Scene min and max frame, this set the maximum and minimum detected in all file paths. ---- Extra notes: While debugging i get a few error messages related to the importer, but not related to the patch. ![image](/attachments/fe0b63a5-f0f9-4495-980b-7689b59add25)

image.png

9.6 KiB

Guillermo Venegas changed title from ~~WIP: IO: Import multiple alembic files at once~~ to IO: Import multiple alembic files at once

2024-05-07 16:07:46 +02:00

Iliya Katushenock added this to the Pipeline, Assets & IO project 2024-05-07 17:53:32 +02:00

Jesse Yurkovich requested review from Jesse Yurkovich 2024-05-17 23:24:29 +02:00

Jesse Yurkovich commented

2024-05-28 19:00:07 +02:00

I'll start checking with others if they're ok with the general approach used here. In the meantime the progress bar calculation may need a bit of work. When trying to import in 3 large alembics (just a bunch of primitives with 1-4 frames of animation data) you might see the following (it is repeated for each of the 3):

I'll start checking with others if they're ok with the general approach used here. In the meantime the progress bar calculation may need a bit of work. When trying to import in 3 large alembics (just a bunch of primitives with 1-4 frames of animation data) you might see the following (it is repeated for each of the 3): ![progressbar.png](/attachments/e8045dc9-e204-4950-8c65-f0669ca47fa9)

progressbar.png

4.8 KiB

~~Jesse Yurkovich referenced this pull request 2024-05-29 02:07:18 +02:00~~

Complete File Handler support #119488

Jesse Yurkovich requested changes 2024-06-05 04:16:49 +02:00

Jesse Yurkovich left a comment

Just 2 minor comments. The rest of the code seems to work alright from what I've tested.

source/blender/editors/io/io_alembic.cc Outdated

						
				@ -633,15 +636,16 @@ static int wm_alembic_import_exec(bContext *C, wmOperator *op)

				  }

				  AlembicImportParams params = {0};

Jesse Yurkovich commented

2024-06-05 04:16:21 +02:00

Now that AlembicImportParams is defined in a full c++ header, use params{}; to initialize because = {0}; has slightly different semantics.

Now that AlembicImportParams is defined in a full c++ header, use `params{};` to initialize because `= {0};` has slightly different semantics.

Guillermo Venegas commented

2024-06-05 17:52:53 +02:00

ABC_alembic.h needs to be moved to ABC_alembic.hh to follow conventions too
Don't know if that can add a lot of noise in this pr

Edit: They are other c++ files in .h that requires ABC_alembic.h (abc_archive.h and abc_hierarchy_iterator.h), so i think its fine by now

`ABC_alembic.h` needs to be moved to `ABC_alembic.hh` to follow conventions too Don't know if that can add a lot of noise in this pr Edit: They are other c++ files in `.h` that requires `ABC_alembic.h` (`abc_archive.h` and `abc_hierarchy_iterator.h`), so i think its fine by now

source/blender/io/alembic/intern/alembic_capi.cc Outdated

						
				@ -699,3 +725,3 @@

				  job->wm = CTX_wm_manager(C);

				  job->import_ok = false;

				  STRNCPY(job->filepath, filepath);

				  job->paths = std::move(params->paths);

Jesse Yurkovich commented

2024-06-05 03:35:19 +02:00

This produces a few warnings for me with a variety of tools because params is const:
Warning C26478 Don't use std::move on constant variables. (es.56)

This produces a few warnings for me with a variety of tools because `params` is `const`: `Warning C26478 Don't use std::move on constant variables. (es.56)`

guishe marked this conversation as resolved