Skip to content

Conversation

elerac
Copy link

@elerac elerac commented Aug 12, 2025

Description

This PR fixes the weight (inverse PDF) calculation in Sensor::sample_wavelengths() when sampling through a user-defined SRF (m_srf), making its behavior consistent with the default path (sample_rgb_spectrum()).

Previously, the implementation directly returned m_srf->sample_spectrum(), which maps to RegularSpectrum::sample_spectrum() or IrregularSpectrum::sample_spectrum(). The second return value from that function was m_distr.integral(), which is not the inverse PDF.

With this change, I removed the redundant weight multiplication in SpecFilm::prepare_sample(), simplifying the code.

Furthermore, rendering with a user-defined SRF in the sensor now produces the correct RGB image. I tested using the following code:

import mitsuba as mi

# mi.set_variant("scalar_spectral")
mi.set_variant("llvm_ad_spectral")

scene_dict = {
    "type": "scene",
    "integrator": {"type": "path"},
    "emitter": {"type": "sunsky"},
    "sphere": {
        "type": "sphere",
        "bsdf": {"type": "diffuse", "reflectance": {"type": "rgb", "value": [0.7, 0.1, 0.1]}},
    },
    "floor": {
        "type": "rectangle",
        "to_world": mi.ScalarTransform4f().translate([0, 0, -1]).scale(40),
        "bsdf": {
            "type": "diffuse",
            "reflectance": {"type": "checkerboard", "color0": 0.1, "color1": 0.8, "to_uv": mi.ScalarTransform4f().scale(8.0)},
        },
    },
    "sensor": {
        "type": "perspective",
        "to_world": mi.ScalarTransform4f().look_at(origin=[3, 4, 1.5], target=[0.5, 0.5, 0], up=[0, 0, 1]),
        "fov": 45,
        "film": {"type": "hdrfilm", "width": 400, "height": 400},
        "srf": {
            "type": "regular",
            "wavelength_min": mi.MI_CIE_MIN,
            "wavelength_max": mi.MI_CIE_MAX,
            "values": "1.0, 0.1, 1.0",
        },  # User-defined (extreme) SRF
    },
}

scene = mi.load_dict(scene_dict)
result = mi.render(scene, spp=64)
mi.Bitmap(result).write("output.exr")
Previous (default) Previous (with user-defined SRF) Fixed (with user-defined SRF)
hdrfilm_default_prev hdrfilm_srf_prev hdrfilm_srf_new

While providing a custom SRF is an uncommon scenario, this fix ensures correctness for those cases.

Checklist

  • My code follows the style guidelines of this project
  • My changes generate no new warnings
  • My code also compiles for cuda_* and llvm_* variants. If you can't test this, please leave below
  • I have commented my code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I cleaned the commit history and removed any "Merge" commits
  • I give permission that the Mitsuba 3 project may redistribute my contributions under the terms of its license

@mcrescas
Copy link
Member

Hi @elerac,

It seems there might be a few misunderstandings. I'll try to clarify them for you:

  • A sensor response function (SRF) models the sensitivity of a sensor to a certain wavelength. This means that some wavelengths will have a lower weight, resulting in less perceived energy. Since we want to use importance sampling, the sampling is done according to the SRF's distribution, or in other words, its sensitivity per wavelength.
  • During wavelength sampling, we need to compute the spectral weight, which is defined as $w = \frac{\text{valueSRF}}{\text{pdf}}$. To expand on this, assuming a specific distribution, the equation becomes $w = \frac{\text{eval}}{(\frac{\text{eval}}{\text{integral}})} = \text{integral}$. We simplify this operation and return the final value directly.
  • Specfilm supports multiple SRFs that can overlap in the same wavelength range. The importance sampling routine attempts to merge this information to ensure good coverage, but the generated weights aren't the exact ones used in the per-band channel computation. The code you removed accounts for this, ensuring the final weights are correct.

Based on your test, if you change the sensor's SRF to something other than the RGB sensitivities, it makes sense that the final RGB image will be different. This is because you are changing the sensitivity. The hdrfilm always converts spectral data to RGB after computing the samples, based on whatever sensitivity the user requested.

To summarize, SRFs define a sensor's sensitivity, and we use this information to create a tailored importance sampling scheme. I believe the misunderstanding stems from thinking that SRFs only model the sampling weights.

I hope this helps clarify things.

@elerac
Copy link
Author

elerac commented Aug 13, 2025

Thank you, @mcrescas, for the clarification!

If I understand correctly, I know the behavior of importance sampling and the weighting scheme in specfilm. My modification does not change the final rendered images in specfilm.

To explain the attached images, I specify one SRF (not multiple) in the sensor plugin with hdrfilm (not specfilm). As you mentioned, the importance sampling scheme is changed based on the provided SRF distribution. However, there is a mismatch of the corresponding weights; the current weight is an integral value, which is a constant that does not depend on wavelength. For sure, inside specfilm, appropriate wavelength-dependent weights are multiplied. But hdrfilme does not account for this, which results in a color shift. If I’ve misunderstood, please correct me.

But anyway, the main point I’m raising is the discrepancy in Sensor::sample_wavelengths() behavior when m_srf is present. As shown in the following figure, I compared the returned values of wavelengths and weights for cases with and without SRF. For the "With SRF" case, I provided discretized PDF values from mi.pdf_rgb_spectrum(), which is identical to the default PDF. Therefore, both cases should yield the same wavelengths and weights. From the plots, the wavelengths match exactly, but the weights differ.

I’m wondering if this discrepancy is intentional? If it is, then my PR conflicts with the intended design, and I will close it.

srf_comparison

Thanks again for your time. I may have overlooked something important.

Please click here to see my plotting code.
import numpy as np
import matplotlib.pyplot as plt
import mitsuba as mi

mi.set_variant("scalar_spectral")


def sample_wavelengths(sensor, samples):
    wavelengths = []
    weights = []
    si = mi.SurfaceInteraction3f()
    for sample in samples:
        wavelength_4, weight_4 = sensor.sample_wavelengths(si, float(sample))
        wavelength, weight = wavelength_4[0], weight_4[0]  # Extract the first items
        wavelengths.append(wavelength)
        weights.append(weight)
    return wavelengths, weights


# Sensor without SRF (default behavior)
sensor_dict_default = {
    "type": "perspective",
    "film": {"type": "hdrfilm", "width": 400, "height": 400},
}
sensor_default: mi.ProjectiveCamera = mi.load_dict(sensor_dict_default)

# Sensor with SRF
# The SRF mimics the default PDF, so it should yield similar results
wavelenths = np.linspace(mi.MI_CIE_MIN, mi.MI_CIE_MAX, 200)
pdf = [mi.pdf_rgb_spectrum(wvl) for wvl in wavelenths]
sensor_dict_srf = {
    "type": "perspective",
    "film": {"type": "hdrfilm", "width": 400, "height": 400},
    "srf": {
        "type": "irregular",
        "wavelengths": ", ".join(map(str, wavelenths)),
        "values": ", ".join(map(str, pdf)),
    },
}
sensor_srf: mi.ProjectiveCamera = mi.load_dict(sensor_dict_srf)

# Sample wavelengths from both sensors
samples = np.linspace(0, 1, 100)
wavelengths_default, weights_default = sample_wavelengths(sensor_default, samples)
wavelengths_srf, weights_srf = sample_wavelengths(sensor_srf, samples)

# --------------------------------------------------

# Plotting the results
fig, (ax1, ax2) = plt.subplots(1, 2, figsize=(12, 5))

# Plot default sensor
color = "tab:red"
ax1.set_xlabel("Samples")
ax1.set_ylabel("Wavelengths", color=color)
ax1.plot(samples, wavelengths_default, color=color, label="Wavelengths")
ax1.tick_params(axis="y", labelcolor=color)
ax1.set_title("Without SRF (default PDF)")

ax1_twin = ax1.twinx()
color = "tab:blue"
ax1_twin.set_ylabel("Weights", color=color)
ax1_twin.plot(samples, weights_default, color=color, label="Weights")
ax1_twin.tick_params(axis="y", labelcolor=color)

ax1.legend(loc="upper left")
ax1_twin.legend(loc="upper right")

# Plot SRF sensor
color = "tab:red"
ax2.set_xlabel("Samples")
ax2.set_ylabel("Wavelengths", color=color)
ax2.plot(samples, wavelengths_srf, color=color, label="Wavelengths")
ax2.tick_params(axis="y", labelcolor=color)
ax2.set_title("With SRF (mimics default PDF)")

ax2_twin = ax2.twinx()
color = "tab:blue"
ax2_twin.set_ylabel("Weights", color=color)
ax2_twin.plot(samples, weights_srf, color=color, label="Weights")
ax2_twin.tick_params(axis="y", labelcolor=color)

ax2.legend(loc="upper left")
ax2_twin.legend(loc="upper right")

fig.tight_layout()
plt.show()

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants