Use variant-specific registry domains

[merlinND]

Description

Previously, we could easily segfault when performing a vcall if an object was previously created with another variant. For example, a spectral_polarized BSDF and an rgb BSDF.

Other than fixing the case below, I don't think this change could hurt since we never need or want instances from different variants to participate in the same vcalls (?).

Testing

import drjit as dr
import mitsuba as mi

def test01_test_variants_switching():
    mi.set_variant('cuda_ad_mono_polarized')
    # Unused, but registers objects into the registry
    scene1 = mi.load_dict(mi.cornell_box())

    mi.set_variant('cuda_ad_rgb')
    scene2 = mi.load_dict(mi.cornell_box())
   
    # Before this PR, this would segfault when e.g. collecting the results of the BSDF vcall,
    # because the `Spectrum` types of the BSDF instances were not compatible.
    image = mi.render(scene2)
    print(image)


if __name__ == "__main__":
    test01_test_variants_switching()

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

[merlinND]

This ended up being a fair bit more complicated that I expected, because now that the domain name depends on the Float, Spectrum template arguments, it cannot directly be set in the call support macros from the Name macro parameter.

But because call_support<>::Domain is used as a static constexpr field in a few places, we still have to compute the domain name in a constexpr way.
In C++17, it's possible to do constexpr string concatenation via std::array<>.

If we find a simpler solution (either to the original issue or any of the implementation steps of this PR), I would be all for it.

[njroussel]

I agree that we should have some kind of a safety net for this issue.

Your suggested implementation seems fine. Changing the Domain is what makes the most sense, in my opinion.

Could you change the macros to be prefixed with MI_ instead of MTS_.

Also the CI is currently failing due to the variant_str declaration being non-constexpr. I believe that you should be able to fix that with a std::array again.

[merlinND]

Thanks for having a look @njroussel!

I updated the macros to use MI_ instead of MTS and used domain_() instead of __domain() (matching the other DrJit methods like abs_, sin_, etc).

I tried another implementation of the constexpr string concatenation (using more std::array<char, N>) that will hopefully work across compilers.

[merlinND]

CI fails look like:

• MVSC: internal compiler error ( C:\BuildAgent\work\c1e5bf5b71462fab\include\mitsuba\core\class.h(254): fatal error C1001: Internal compiler error. [C:\BuildAgent\work\c1e5bf5b71462fab\build\src\mitsuba.vcxproj])
  • I had a simplified version working on Compiler Explorer though: https://godbolt.org/z/bTT5ePPEG
• Others: volpath rendering tests failing. I don't know what's special about volpath since the other rendering tests are passing. I checked Medium and PhaseFunction, and they are the same as the other interfaces w.r.t. this new variant-specific domain. Maybe volpath is the only class in Mitsuba that triggers recursive vcalls, and that now somehow fails? 🤔
  • Edit: fixed (hopefully). Medium was the only class whose call_support helpers declared getters before methods. DRJIT_CALL_METHOD starts a private scope, which meant that what I declared in MI_CALL_TEMPLATE_END was private, so the custom domain name didn't get picked up. By correctly setting visibility in MI_CALL_TEMPLATE_END, this is fixed.

[merlinND]

Using fewer intermediate static constexpr std::array variables I can get MSVC build without crash locally, let's see if it works on the CI as well.

Edit: CI is green!

[merlinND]

Hi @njroussel,

Does the current state of this PR (+ the smaller one on the DrJit side: mitsuba-renderer/drjit#307) look good to you?