Skip to content

[LDM3D] Fix rgblike_to_depthmap truncating 16-bit depth back to 8 bits#14200

Open
chuenchen309 wants to merge 1 commit into
huggingface:mainfrom
chuenchen309:fix/ldm3d-rgblike-to-depthmap-16bit
Open

[LDM3D] Fix rgblike_to_depthmap truncating 16-bit depth back to 8 bits#14200
chuenchen309 wants to merge 1 commit into
huggingface:mainfrom
chuenchen309:fix/ldm3d-rgblike-to-depthmap-16bit

Conversation

@chuenchen309

Copy link
Copy Markdown

What does this PR do?

VaeImageProcessorLDM3D.rgblike_to_depthmap combines two 8-bit channels into a 16-bit depth value (green * 256 + blue, range 0–65535). It correctly casts up to a wider integer type for the multiplication, but then casts the result back to the input's dtype (uint8) via .to(original_dtype) / .astype(original_dtype), dropping the entire high byte — so it returned only depth % 256.

numpy_to_depth feeds this into Image.fromarray(..., mode="I;16"), which requires 16-bit-wide data, so with a uint8 buffer VaeImageProcessorLDM3D.postprocess(image, output_type="pil") — the normal way an LDM3D pipeline returns a depth image for a 6-channel output — crashes with ValueError: buffer is not large enough.

from diffusers.image_processor import VaeImageProcessorLDM3D
import torch, numpy as np
p = VaeImageProcessorLDM3D()

rgb = np.zeros((2, 2, 3), dtype=np.uint8); rgb[..., 1] = 5; rgb[..., 2] = 200
p.rgblike_to_depthmap(rgb).max()   # before: 200   after: 1480  (5*256+200)

img = torch.zeros(1, 6, 4, 4); img[:, 4] = 5/255.; img[:, 5] = 200/255.
p.postprocess(img, output_type="pil")   # before: ValueError: buffer is not large enough
                                         # after:  returns a valid mode="I;16" depth image

The fix keeps the 16-bit result instead of truncating: return the wider integer type directly (numpy → uint16 to match the mode="I;16" consumer, torch → the int32 combination). The function's whole purpose is to produce a 16-bit depth map from 8-bit RGB, so casting back to the 8-bit input dtype was never correct.

This regression was introduced by #12546 (which fixed a real NumPy 2.0 uint8 * 256 overflow but replaced it with this truncation). tests/others/test_image_processor.py had no VaeImageProcessorLDM3D coverage, which is why it shipped — this adds a small regression test class.

Note (kept out of this PR to stay focused): the output_type="np" branch of postprocess passes float [0, 1] arrays into rgblike_to_depthmap, where the int cast collapses them to 0. That's an independent caller-side issue in the same function's contract and is better handled separately.

Before submitting

  • Did you read the contributor guideline?
  • Did you make sure to update the documentation with your changes? (no user-facing doc change — internal correctness fix)
  • Did you write any new necessary tests? (added Ldm3dImageProcessorTest: 16-bit combination for numpy + torch, and the pil postprocess path; red before the fix, green after. Full test_image_processor.py: 12 passed. ruff 0.9.10 check + format clean.)

Who can review?

@yiyixuxu @sayakpaul


Found via an AI-assisted (Claude) review pass over image_processor.py; independently reproduced, fixed, and verified before submitting.

VaeImageProcessorLDM3D.rgblike_to_depthmap combines two 8-bit channels into
a 16-bit depth value (green * 256 + blue, range 0-65535). It correctly casts
up to a wider integer type for the multiplication, but then cast the result
back to the input's dtype (uint8) via `.to(original_dtype)` /
`.astype(original_dtype)`, dropping the entire high byte -- so it returned
only `depth % 256` (e.g. 1480 came back as 200).

numpy_to_depth feeds this into `Image.fromarray(..., mode="I;16")`, which
requires 16-bit-wide data, so with a uint8 buffer
`VaeImageProcessorLDM3D.postprocess(image, output_type="pil")` -- the normal
way an LDM3D pipeline returns a depth image for a 6-channel output -- crashed
with `ValueError: buffer is not large enough`.

Keep the 16-bit result instead of truncating: return the wider integer type
directly (numpy -> uint16 to match the `mode="I;16"` consumer, torch -> the
int32 combination). The function's whole purpose is to produce a 16-bit depth
map from 8-bit RGB, so casting back to the 8-bit input dtype was never
correct.

This regression was introduced by huggingface#12546 (which fixed a real NumPy 2.0
`uint8 * 256` overflow but replaced it with this truncation). The existing
tests/others/test_image_processor.py had no VaeImageProcessorLDM3D coverage,
which is why it shipped; this adds a small regression test class covering the
16-bit combination (numpy + torch) and the pil postprocess path.

Verified against the real code before/after: rgblike_to_depthmap(green=5,
blue=200) returned 200 (uint8) before and 1480 (uint16 / int32) after;
postprocess(output_type="pil") raised ValueError before and returns a valid
"I;16" depth image after. Full tests/others/test_image_processor.py: 12
passed. ruff 0.9.10 (repo-pinned) check + format: clean.

Note (separate, not addressed here to keep this focused): the
output_type="np" branch of postprocess passes float [0, 1] arrays into
rgblike_to_depthmap, where the int cast collapses them to 0; that is an
independent caller-side issue in the same function's contract and is better
handled in its own change.
@github-actions github-actions Bot added tests size/M PR with diff < 200 LOC labels Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Hi @chuenchen309, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. Fixes #1234) to the PR description so the issue is linked. See the contribution guide for more details. If this PR intentionally does not fix a tracked issue, a maintainer can add the no-issue-needed label to silence this reminder.

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

Labels

size/M PR with diff < 200 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant