neverhood311 / Stop-motion-OBJ

A Blender add-on for importing a sequence of OBJ meshes as frames
GNU General Public License v3.0
687 stars 52 forks source link

Added better support for alembic export #154

Closed ChristopherRemde closed 2 years ago

ChristopherRemde commented 2 years ago

Dear neverhood,

Thank you for your work on this plugin! I use it mainly to export .obj sequences to alembic files and as you said yourself on the wiki, export for alembic files isn't too reliable in the current state. As I need to convert lots of sequences to alembic for my work project, I tried to improve this process a bit and maybe you're interested in integrating these changes into the plugin.

Problems encountered: I figured out that the main problem when exporting sequences to alembic is the way the exporter expects the structure of the scene hierachy. In the current state, the exporter detects each frame as a single unique mesh. They are exported as individual "tracks" (objects) in alembic terms. So for a sequence of 900 frames, 900 tracks with a single frame each are created in the alembic file. This creates two problems:

1: On reimport of the alembic file, all the tracks will start at the same time, so you get something that looks like this (Animation with 5 frames):

grafik

2: Some importers, like the Unreal Engine importer can't really handle this many tracks and they crash.

Improvements To fix the problems with the scene hierachy, I added an option in the importer menu, to display all of the frames on the same mesh. Before, the mesh object data was always swapped, this led to the alembic exporter interpreting each frame as new mesh/track. With the option enabled, the mesh object stays the same, but all it's vertices/polygons/uvs ect. are changed. This causes the exporter to recognize that all the frames are belonging to the same track/mesh.

grafik

There is a small workaround however that is needed. Blender needs to see a modifier on the object to recognize it has an animation, so I added an array modifier with the count set to 0 to make it work properly. This should be relativly harmless.

With the changes, I can also successfully import alembic files into Unreal Engine!

Testing

I tested the new changes with caching, streaming, one material per frame on/off, saving/opening as .blend file, deleting and baking.

Known problems

The changes work perfectly with version 2.11, but on version 2.2 the added line 50 in the init file: bpy.app.handlers.depsgraph_update_pre.append(updateFrame) causes a crash when exporting to alembic. I don't really understand what this does well enough to have a solution for that problem right now, so I think I need your input here.

I'm also not too sure if the name "Show as single mesh" really describes what these changes do well enough. There probably is a better name.

If you got any questions, let me know! I think there are a lot of people who want to use this plugin for exporting to alembic, so I hope it's a welcome addition :)

ChristopherRemde commented 2 years ago

Just as a quick comparision, here are two alembic files, created with the "Show as single mesh" option turned off (before) and on (after). alembic_export.zip

neverhood311 commented 2 years ago

Thanks for the PR. Looks like you've been digging pretty deep into the addon.

Regarding the depsgraph_update_pre vs _post, I found that it was necessary to do pre instead of post for when you're using the Keyframe mode for a mesh sequence. If pre is used, the mesh sequence will change frames in the viewport as you're moving keyframes around in the graph editor. If post is used, you have to do a frame change to see the update.

I was able to reproduce the crash you mentioned. In the error log, it says EXCEPTION_ACCESS_VIOLATION which usually means that you have a pointer to something that is no longer valid. It's hard to tell exactly what's causing this, but do you know whether it's the BMesh code you added (lines 754-757) or the copy materials code (lines 760-762)? If it turns out to be the BMesh code, I wonder if there's any other way to do this without BMesh.

ChristopherRemde commented 2 years ago

Thank you for the quick response and looking into the issue! I was able to narrow the error down to the material copy block (line 760-762), especially the "append" function seems to cause the crash. It's a bit curious, because the copying also happens a bit further down in line 771 too, and there it doesn't cause a crash. I'll try to find the cause.

ChristopherRemde commented 2 years ago

Hey! So after days of trying out every fix I could imagine for this problem, I condensed the code down to a few lines and it still crashes. Therefore I think it might be a bug in Blender itself, so I created a Bug report: https://developer.blender.org/T95196

neverhood311 commented 2 years ago

Dang. That's a bummer.

Ok, what about this (somewhat ugly) workaround: Create a second setFrameObj method. This one is just for your showAsSingleMesh sequences. Then call that function in the despgraph_update_post handler, but only on sequences where showAsSingleMesh is set to True. Obviously you'd also have to modify the existing setFrame method (or some function it calls) so that it only updates sequences where showAsSingleMesh is False. It's pretty ugly, but it will probably work. It's better than hoping a Blender developer will address your bug report.

ChristopherRemde commented 2 years ago

Hey neverhood, thank you for your input, seems like a good idea! I kinda wanted to avoid workarounds, but for the moment it's probably the best option. I'll submit the update later.

neverhood311 commented 2 years ago

Sounds good. Once that commit is in, I'll probably make a few notes on your PR to help you meet all the code guidelines for the project (which I should probably publish somewhere, huh?).

ChristopherRemde commented 2 years ago

Alright I implemented the workaround and tested it again with all possible settings and features! The crash has gone away, thanks again for your suggestion with the workaround. And yes I'd be happy to align the code with your guidelines!

I'll have an eye on the bug report and if it ever gets fixed, I'll submit a new PR to remove the workaround.

neverhood311 commented 2 years ago

One more change: you'll need to increment the version numbers found in __init__.py and version.py. When the project is packaged up on Travis-CI, it will fail if you try to re-use an old version number. Just bump it to the next alpha version. See https://github.com/neverhood311/Stop-motion-OBJ/pull/150/files for an example.

neverhood311 commented 2 years ago

I saw you resolved all of the suggestions. Is there another commit coming?

ChristopherRemde commented 2 years ago

I just waited for your response regarding the mesh naming, commited it now! :)

neverhood311 commented 2 years ago

I pulled your code and tested it out. Looks good to me. Just rename that property's description and I think it'll be good to merge.

ChristopherRemde commented 2 years ago

Yeah the naming is much better this way! If any problem with this code pops up in the future don't hesitate to contact me! Thank you again for your support and care for this addon :)

neverhood311 commented 2 years ago

Hey @ChristopherRemde, I just tried out this code and it's still crashing Blender. Have you tried it since the merge?

ChristopherRemde commented 2 years ago

Hey @neverhood311 , I just tried an alembic export and it seems to work fine on my side. Tested with the newest Blender Version 3.0.1 and the merged version-2.2 branch. Can you tell me the steps you're doing so I can replicate it?

However I noticed that I forgot to rename the button in the panel in panels,py on line 187. (It's still named Show as single mesh)

neverhood311 commented 2 years ago

It's hard to nail down exactly what's happening. I've been getting it to crash most of the time when I test with a certain (well-formed, but no materials) mesh sequence. I'll attach it here: numbers.zip. But I can't really get it to crash when I use the Horse Gallop example (included in the repo). I'm going to keep poking around. Usually, when you get a EXCEPTION_ACCESS_VIOLATION, it's caused by holding onto an object reference. This page in the Blender documentation seems related: https://docs.blender.org/api/current/info_gotcha.html#help-my-script-crashes-blender

The steps I've used to replicate it are:

  1. Import the "numbers" sequence. DON'T check the Material per Frame box, but DO check the Show as Single Mesh box
  2. Don't play through the sequence, don't select/deselect anything
  3. Export the scene as Alembic, without changing any export settings 4.. Blender usually (but not always) crashes

Yeah, I noticed that the wrong property was renamed. Not a problem. That'll get fixed in the next PR.

ChristopherRemde commented 2 years ago

Thanks for your detailed instructions! I'm a bit busy right now, but I'll try to look into this issue at the end of the week!

neverhood311 commented 2 years ago

No rush. I'm going to be offline for a few days anyways. Today's my last day at my job and I don't start my next job until next Monday.

ChristopherRemde commented 2 years ago

Hey, I hope you had a good start at your new job!

I just took a look at the problem, and I can confirm that it also crashes on my side, but very unregularly. The exception is the same I got with the update_pre event. Unfortunately I'm kind of at my wits end here, I don't think I understand the Blender scripting well enough yet to be able to find the cause.

One workaround could be to delete the depsgraph_update_post event completly for the single mesh export. The alembic export still works perfectly with only the frame_change_pre event, and it doesn't seem to crash anymore. Would this be an option, or does the depsgraph_update_post perform a critical function?

I can totally understand however if this PR is just to unstable right now and you want to roll it back. Maybe some time later when I got a better grasp on Blender scripting and / or the blender team fixes the bugs for the underlying crash I'll be able to commit a more stable version of this code!

neverhood311 commented 2 years ago

@ChristopherRemde Sorry to say it, but I think I'm going to revert this PR for now.

Sorry for the delay. It's been a crazy month.