Skip to content

Conversation

@cdtwigg
Copy link
Contributor

@cdtwigg cdtwigg commented Oct 22, 2025

Summary:
build_cameras_for_body has bothered me for a long time:

  1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
  2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
  3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
  4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
  5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera"). Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 22, 2025
@meta-codesync
Copy link
Contributor

meta-codesync bot commented Oct 22, 2025

@cdtwigg has exported this pull request. If you are a Meta employee, you can view the originating Diff in D85170861.

cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:
Pull Request resolved: #710

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:
Pull Request resolved: #710

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:
Pull Request resolved: #710

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:
Pull Request resolved: #710

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
@cdtwigg cdtwigg force-pushed the export-D85170861 branch 2 times, most recently from 3482811 to e6e2b82 Compare October 23, 2025 15:48
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).  
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:
Pull Request resolved: #710

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
@cdtwigg cdtwigg force-pushed the export-D85170861 branch 2 times, most recently from 815f891 to ff04b8c Compare October 23, 2025 15:59
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:
Pull Request resolved: #710

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).  
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).  
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
@cdtwigg cdtwigg force-pushed the export-D85170861 branch 2 times, most recently from 7adc9c6 to 12c63bb Compare October 23, 2025 23:34
cdtwigg added a commit that referenced this pull request Oct 23, 2025
…uild_cameras_for_body. (#710)

Summary:

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).  
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
cdtwigg added a commit that referenced this pull request Oct 24, 2025
…uild_cameras_for_body. (#710)

Summary:

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).  
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
cdtwigg added a commit that referenced this pull request Oct 24, 2025
…uild_cameras_for_body. (#710)

Summary:
Pull Request resolved: #710

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
jeongseok and others added 2 commits October 24, 2025 21:01
…oad the correct commit data.

Differential Revision: D85488266
Summary:
Fixed incorrect template-id syntax in the destructor definition of `MultiposeSolverFunctionT<T>`. The destructor was incorrectly defined as `~MultiposeSolverFunctionT<T>()` but should be `~MultiposeSolverFunctionT()` per C++ standard - template-ids are not allowed after the destructor operator.

This error was caught by conda-forge builds but not by Meta's internal diff signals because the external pymomentum CI job is currently disabled in GitHub workflows to reduce costs, and internal Buck builds apparently succeed with different compiler flags.

Differential Revision: D85457060
jeongseok and others added 2 commits October 24, 2025 21:01
Differential Revision: D85474531
…oad the correct commit data.

Differential Revision: D85481382
cdtwigg added a commit that referenced this pull request Oct 25, 2025
…uild_cameras_for_body. (#710)

Summary:

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).  
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
…uild_cameras_for_body. (#710)

Summary:
Pull Request resolved: #710

build_cameras_for_body has bothered me for a long time:
1. It uses torch.Tensors even though it's not differentiable, ideally we're moving the rendering code toward numpy
2. It is batched even though I have literally never seen anyone use it that way, we'd like to move the renderer away from batched operations and encourage people to use multithreading instead if they want to render in parallel (because batching in rendering is basically impossible to use anyway).
3. Because of (2), it is very confusing when you want to create a camera for a multi-frame animation, you have to add a fictional batch dimension or the tensor will get treated as batched and return n_frames cameras, and since you access cameras[0] anyway at the end failures are silent.
4. It uses joint_parameters even though other renderer operations operate on skel_states, and this is annoying when you only have skel_states (I have seen cases where we inverted skel_states to joint_params just to create a camera).
5. The naming isn't consistent with other renderer functions like create_z_buffer.

Therefore I'm proposing this new function "create_camera_for_body" (note the singular "camera").  Since I'm changing the name (to address complaints (2) and (5)), we might as well just add it alongside the existing function, and after we've replaced all uses we can retire the other function.

Reviewed By: cstollmeta, jeongseok-meta

Differential Revision: D85170861
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. fb-exported meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants