Dev - #51
Merged
Merged
Conversation
The cos-map conversion that turned equirect ray-distance depth into
per-face planar-Z used to live as a 'convert_distance_to_planar'
toggle on SharpPanoramaIcosahedronSplit. That node is supposed to be
a generic pano->faces resampler; depth semantics aren't its concern.
Move the conversion onto SharpPredictGaussiansFromMetricDepth, which
is the actual depth consumer and already receives the per-face
intrinsics it needs to build the same cos-map. Add a
'depth_convention' combo input there ('planar_z' default, or
'ray_distance' for panorama-derived depth). cos-map is applied at
the depth's native resolution before the resize to 1536^2, so the
convention boundary isn't smeared by bilinear interp. Falls back to
a synthetic K from focal_length_mm + (width,height) if intrinsics
isn't wired -- never silently skips when the user asked for the
conversion.
SharpPanoramaIcosahedronSplit loses the toggle, the conditional
cos-map block, and the parameter -- back to being a pure resampler.
Workflow JSONs updated: pano.json (split id=48 / predict id=53) and
final.json (splits 87 + 134, predicts 83 + 118). The two splits that
previously had convert_distance_to_planar=True now feed predicts
configured with depth_convention='ray_distance'; the one split
(node 134) that had it False loses the toggle without downstream
changes.
Pre-existing bug NOT touched here: final.json node id=118's
widgets_values has been misaligned vs its schema since
snap_to_external_depth was added -- value 0 reads as 30 at the
snap slot. My edit inserted 'ray_distance' at the correct
structural position; the schema mismatch on the OTHER positions
predates this commit.
Two fixes in one commit since they're related to the user's panorama
pipeline:
1. predict_gaussians_from_depth.py: detect normalized intrinsics.
PanoPack's PanoramaSplit emits K via utils3d.np.intrinsics_from_fov
which returns NORMALIZED K (fx=0.5, cx=0.5 for 90 deg fov, units in
[0,1]). Sharp's predict node assumed pixel-K (fx=hundreds). With the
new ray_distance cos_map this produced:
x_cam at uu=1535 = (1535 - 0.5)/0.5 = 3069
cos_map at corner = 1/sqrt(2*3069^2 + 1) = 0.0002
The depth was then multiplied by ~0 and SHARP's unprojection
exploded to NaN. Add a one-line sniff (fx < 2.0 => normalized) at
the top of the intrinsics path that rescales to pixel-K once. All
downstream uses (f_px, cos_map, unprojection K) see the same
pixel-K convention.
2. Delete three nodes per user request:
- SharpPanoramaIcosahedronSplit (panorama_icosahedron_split.py)
- SharpDepthMerge (depth_merge.py)
- SharpBuildMesh (build_mesh.py)
Their functionality lives in ComfyUI-PanoPack (PanoramaSplit,
PanoramaDepthMerge) and the mesh path moved elsewhere. nodes/__init__.py
loses six import lines + six update lines for the three modules. The
prior commit on this branch had already pruned the imports + the
NODE_CLASS_MAPPINGS.update calls, but left orphan
NODE_DISPLAY_NAME_MAPPINGS.update lines referring to the now-undefined
variables -- which broke the pack's import chain at startup. Pruned
those too.
Workflows that referenced the deleted classes will show broken nodes
until edited to use PanoPack equivalents -- this is expected and
matches the explicit deletion request.
Same root cause as the prior NaN-cascade fix in predict_gaussians_from_depth.py: PanoPack's PanoramaSplit emits K via utils3d.np.intrinsics_from_fov which is normalized (fx≈0.5, cx≈0.5 for 90° fov in [0,1] units). Sharp's predict path assumes pixel-K (fx≈hundreds). With normalized K: f_px = K[0,0] * (1536 / width) = 0.5 * 1 = 0.5 disparity_factor_scalar = 0.5 / 1536 ≈ 3.3e-4 metric_depth = 3.3e-4 / disparity ≈ 0 Result: depth maps come out blank (all near-zero). Add the same fx<2.0 sniff at the top of execute(), in the else-branch of 'if intrinsics is None'. Rescales K to pixel-K for the face image's (width, height) before any downstream code touches it -- so f_px, the per-face disparity-to-depth division, and _rescale_K to 1536² all see the same convention. Also retitle the no-K-wired log line from 'SharpPanoramaIcosahedronSplit' to 'PanoramaSplit' since the former was deleted in the previous commit.
predict_gaussian_attrs.py had the same f_px = K[0,0] * (1536/width) pattern as predict_metric_depth.py and predict_gaussians_from_depth.py -- silent collapse to ~0 depth when fed PanoPack's normalized K. Same fix: rescale to pixel-K once before computing f_px. Logged via the existing _p() helper. Now all three predict nodes accept either pixel-K (Sharp's native convention) or normalized-K (PanoPack / utils3d) without manual conversion in the workflow. predict.py (single-image SharpPredict) uses K[0,0] directly with no rescale, so it'd fail differently with normalized K -- but that path isn't used from panorama workflows so leaving it alone.
Single-responsibility conversion node that sits between a depth source
(e.g. PanoramaSplit on an equirect depth pano) and SHARP's predict
node. Makes the convention boundary visible in the graph instead of
hiding it as a 'depth_convention' toggle inside the predict node.
Inputs:
image IMAGE — per-face depth in ray-distance convention
intrinsics INTRINSICS — per-face K (normalized or pixel, auto-detected)
extrinsics EXTRINSICS — pass-through (optional, not used in the math)
Outputs:
image IMAGE — per-face depth in planar-Z convention
extrinsics EXTRINSICS — pass-through
intrinsics INTRINSICS — pass-through, rescaled to pixel-K if input
was normalized (consistent units downstream)
Math: planar_z = ray_distance × cos_map where
cos_map[u, v] = 1 / sqrt(((u-cx)/fx)² + ((v-cy)/fy)² + 1)
Inherits the same fx<2.0 normalized-K sniff added to the three Sharp
predict nodes earlier this cycle, so workflows that wire PanoPack's
utils3d-style normalized intrinsics get auto-rescaled to pixel-K.
Predict-side note: SharpPredictGaussiansFromMetricDepth still has the
'depth_convention' input -- now redundant when this node is used.
Workflows can drop the toggle (use default planar_z) and instead wire
ray-distance depth through SharpRayToPlanarDepth first. Keeping the
toggle in place so existing workflows don't break; deprecation /
removal is a later cleanup.
Adds two new IMAGE outputs (B, 1536, 1536, 3) — per-pixel (X, Y, Z) in camera space — alongside the existing scalar metric_depth outputs. Mirrors MoGe2Inference's depth_raw / points_raw pairing so SHARP can feed PanoramaDepthMerge.face_points directly. Motivation: PanoramaDepthMerge.face_points expects a 3D point map, not a depth scalar. Wiring layer_0_metric_depth (which is depth replicated to 3 channels for IMAGE compatibility) into face_points silently produced wrong geometry: the merger's ||points|| computation gave ||(d, d, d)|| = d × sqrt(3) per pixel and treated it as ray distance, inflating everything by ~73% with no shape-mismatch error. The new points are computed as: x_cam = (u - cx) / fx y_cam = (v - cy) / fy points[u, v] = (x_cam * Z, y_cam * Z, Z) By construction ||points|| equals the correct ray distance — what PanoramaDepthMerge consumes via np.linalg.norm. Intrinsics rescaled to the 1536² depth grid via the same sx/sy ratio used elsewhere; relies on the existing fx<2.0 normalized-K sniff at the top of execute() so PanoPack-style normalized K is auto-converted before the math runs. Output socket order: layer_0_metric_depth, layer_1_metric_depth, extrinsics_mdepth, intrinsics_mdepth, layer_0_points_raw, layer_1_points_raw. New outputs appended at the END so existing workflows that wired extrinsics_mdepth / intrinsics_mdepth by slot index continue to work — those slots (2, 3) are unchanged. Log line gains a ||v|| range for the layer-0 point map so the user can sanity-check that depth × unprojection produced scene-scale 3D positions (||v|| ≈ Z at face centers, ~sqrt(3)·Z at 90° face corners).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.