patrikhuber
commented
1 year ago Hi Omar,
I've pushed a slight tweak to set_landmark-definitions() - we don't need to (and don't want to) pass an optional here, we can directly pass in the underlying type, and then assign that to the optional (which now/then contains that value).
About the fit_shape_and_pose(...) overload (for the fitting without contour fitting): How would you feel about leaving the current function as-is (i.e. keep passing the image_points and vertex_indices), but adding an overload for passing landmarks and a landmark mapper, and this overload then computes the mappings and calls the original fit_shape_and_pose(...) function? I think that should work, or would it not? (We can then add Python bindings for both variants.)
Note: for the modified fit_shape_and_pose, I allowed users to also pass in pca_coeffs and blendshape_coeffs. This makes it easier to control what is being fitted. I can do the same for the other bindings if you're happy with the approach.
You mean in the Python bindings, correct? If yes, then yes we could add that for the other fit_shape_and_pose(...) binding too. However I wonder whether this will actually work (in your overload too). The C++ function takes these parameters by reference, i.e. the caller has to have allocated them, and the function then modifies them in-place. Actually I think the way you've done it, it might work, because in the Python bindings, you wrote a custom lambda that takes std::vector<float> (is that the reason you wrote the custom lambda and are not binding it directly via &fit_shape_and_pose? You could bind it directly, you'll need py::overload_cast<...> (which will have a long list of types, see https://pybind11.readthedocs.io/en/stable/classes.html#overloaded-methods for details) - but this may not actually work as I don't know how the reference to vector<float> will behave - if that doesn't work then we will indeed need your approach with the lambda. In the original fit_shape_and_pose binding, I've also created these vectors locally, and then returned them with make_tuple - perhaps it didn't work to bind them directly.)).
Also it would be nice if we could set the default values of these arguments, I think should work. But:
Also I think the default arguments (py::arg("pca_coeffs") = std::vector<float>()) will not work with references anymore, I think they might only work if we take a vector<float> as input in the bindings and return the result with make_tuple like you're doing it now. The thing that I would like to avoid is that a user can't call the function anymore from Python without passing these values, i.e. they should still be callable without these extra parameters.
Well... maybe you could play around with it a bit, but if you get stuck, let me know and I can try it too. Actually, having written all this now, I'm fairly convinced that the way you're doing it now in this PR is probably the best and (almost) only way to do it, so it's probably not worth spending much more time on it. Let me know your thoughts.
omar-h-omar
Added set_landmark_definitions and altered fit_shape_and_pose to allow fitting without contour.
Note: for the modified fit_shape_and_pose, I allowed users to also pass in pca_coeffs and blendshape_coeffs. This makes it easier to control what is being fitted. I can do the same for the other bindings if you're happy with the approach.