mitsuba-renderer / mitsuba3

Mitsuba 3: A Retargetable Forward and Inverse Renderer
https://www.mitsuba-renderer.org/
Other
2.1k stars 246 forks source link

Fix for sampling_weight emitter attribute #1173

Closed WeiPhil closed 5 months ago

WeiPhil commented 6 months ago

Hi, We encountered a small bug in the emitter plugin that caused the sampling_weight attribute to not be updated when it was changed in the scene parameters. Calling dr::set_attr(this, "sampling_weight", m_sampling_weight) in the constructor and parameters_changed seems to fix the issue. Below is a small reproducer of the issue in case it helps. Just change num_emitters to 1 or 12, the outputs should be identical:

import matplotlib.pyplot as plt
import mitsuba as mi
import numpy as np
import drjit as dr

mi.set_variant('llvm_ad_rgb')

from mitsuba import ScalarTransform4f as T

# Changing this to num_emitters = 1 should give the same result as num_emitters = 12
num_emitters = 12

sensor = {
    'type': 'perspective',
    'fov': 39.3077,
    'to_world': T.look_at(origin=[0, -4, 0], target=[0, 0, 0], up=[0, 0, 1]),
    'sampler': {'type': 'independent', 'sample_count': 16},
    'film': {
        'type': 'hdrfilm',
        'width': 256,
        'height': 256,
        'pixel_format': 'rgb',
    },
}

scene_dict = {
    'type': 'scene',
    'head': {
        'type': 'sphere',
        'to_world': (
            T.scale(0.5).translate([0, 1.5, 0])
        ),
        'bsdf': {
            'type': 'diffuse',
        },
    },
    'sensor': sensor,
}

emitter_radiance = 100.0

for i in range(num_emitters):
  rotation = i * 360.0 / num_emitters
  scene_dict[f'emitter_{i:03d}'] = {
      'type': 'sphere',
      'emitter': {
          'type': 'area',
          'radiance': {
              'type': 'rgb',
              'value': emitter_radiance,
          },
      },
      'to_world': T.rotate([0, 1, 0], rotation).translate([0, 0, 1]).scale(0.1),
  }

scene = mi.load_dict(scene_dict)

params = mi.traverse(scene)

def switch_on_emitter(emitter_idx):
  for e_idx in range(num_emitters):
    if e_idx == emitter_idx:
      params['emitter_' + f'{e_idx:03d}' + '.emitter.sampling_weight'] = 1.0
      params['emitter_' + f'{e_idx:03d}' + '.emitter.radiance.value'] = emitter_radiance
    else:
      params['emitter_' + f'{e_idx:03d}' + '.emitter.sampling_weight'] = 0.0
      params['emitter_' + f'{e_idx:03d}' + '.emitter.radiance.value'] = 0.0

  params.update()

if(num_emitters != 1):
  switch_on_emitter(0)

integrator = mi.load_dict({
  'type': 'direct',
  'emitter_samples': 1,
  'bsdf_samples': 1,
})

image = mi.render(scene, integrator=integrator)

plt.imshow(image)

Best, Philippe

merlinND commented 6 months ago

Hi @WeiPhil,

Thanks for investigating this issue and opening a PR! Could you please add a tiny unit test that checks the fix is effective? (could be adapted from your reproducer script above)

njroussel commented 5 months ago

Hi @WeiPhil

Thanks for the PR :+1:

(I've also pushed a unit/regression test in https://github.com/mitsuba-renderer/mitsuba3/commit/cdd489c717b385bae7ff24601422879ea41d5bf5)

WeiPhil commented 5 months ago

Thanks @njroussel ! Sorry for the delay in adding that test..