iss: 1877 - image import camera transform broken #1878
Conversation
This should bring back 5.15 bottles for our macOS runner.
|
Ah crap.. I must have forgotten to test those cases. It's been such a long time since I did this PR. I'll give it a look with a fresh pair of eyes soon. Thanks for testing |
|
I've made some adjustments, tested all cases and they all work as expected now. In addition I've also removed getImportView from methods that doesn't make use of the import position dialog. There should preferably be no temp state regarding importing, as such it's also been removed from ViewManager. Another observation I noticed is that we might have had a bug where the movie importer would use whatever GetImportView was set to, which could result in it being placed at (0,0) I've also made a small change regarding the default behavior for the Movie Importer, which now uses the Camera Followed option since you can't pick the setting yourself and it's the one that works closest to what one might expect. |
There was a problem hiding this comment.
Thanks, wonderful! The ViewManager cleanup and other fixes are welcome additions. There’s just one small issue left now, which is that when you import an image sequence with the “center of camera, current frame” option, it actually uses the most recent (relative to target position) camera keyframe rather than the current frame. So when you have multiple camera keyframes it actually works more like the “follow camera” option minus interpolation, which is probably not what we want here. Other than that it looks good to me.
|
Ah, you're right. That's a misunderstanding on my part.. I got confused by the "center of camera, current frame" 😅 Should be fixed now though. edit: actually it's still not quite the same but I'd argue the new logic works better since it takes the transform into account but still uses the actual current frame. |
* iss: #1877 - Fix import image using camera transform not working * Remove dead code * Fix crash when importing and the scrubber is not on a keyframe * Fix import to center of camera and camera followed not working * MovieImporter: Simplify generate frames * Avoid taking const reference of enum (integer) * Fix CenterOfCamera should use the import position keyframe, not the current * Get view of the current frame, not the current keyframe * Fix mismatched header guard and add missing licence header --------- Co-authored-by: Jakob Gahde <j5lx@fmail.co.uk>
|
Thanks for reviewing once again and thanks for fixing the header stuff. I always forget to add the license to new files. 😅 |
Fixes #1877
It's worth noticing that this PR pushes to a 0.7.1 branch.
I think this bug is a good contender to start such a branch, but there are already a bunch of other things we could include as well.