Importance sampling of OSL environment EDFs

[est77]

Importance sampling is already implemented for the LatLongMapEnvironmentEDF. The code needs to be refactored so that it can be reused in the OSLEnvironmentEDF.

[lorentzo]

Hi! I will tackle with this problem.

[lorentzo]

Hi!

About the progress and further work plan.

After examining LatLongEnvironmentEDF class and comparing it to the OSLEnvironmentEDF class I have concluded that:

• OSLEnvironmentEDF::on_frame_begin() needs to create an importance sampler. I am thinking of creating OSLImportanceSampler class in oslimportancesampler.h in /foundation/math/sampling/ (similar as there is ImageImportanceSampler used in LatLongEnvironmentEDF). OSLImportanceSampler class will be similar to ImageImportanceSampler sampler, that is:

  • it will contain rebuild() (as ImageImportanceSampler::rebuild())
  • it will contain sample() (as ImageImportanceSampler::sample())
  • it will contain get_pdf() (as ImageImportanceSampler::get_pedf())

• In order to implement those functions I have concluded that OSL's Background class has similar functions (thanks @est77!) which I can use for inspiration.

• After OSLImportanceSampler class will be implemented I can easily refactor OSLEnvironmentEDF class, that is, functions:

  • OSLImportanceSampler::sample()
  • OSLImportanceSampler::evaluate() (if needed)

• For refactoring I will reference to LatLongEnvironmentEDF

Let me know if you agree with the plan.

For now I have one doubt:

• To create a importance map for OSL background shaders, evaluation of OSL background shaders should be possible. We can see in OSL's SimpleRayTracer::prepare_render() how SimpleRayTracer::eval_background() (that is shadingsys->execute()) is used in calculation of importance map.
• Therefore, I would need ShadingContext::execute_osl_background() for evaluating OSL shaders in Appleseed. I have looked into test_tracer test how shading_context can be created. My idea is to create shading_context in that way for evaluating the background shaders. Please, let me know if I am on a correct path with this idea. Finally, I am planning to use shading_context to create OSLsampler. This is similar to creation of ImageSampler which is then used in creation of importance map.

[est77]

OSLShaderGroupExec is probably a better class to use when evaluating the background shader.

As for getting the OSLShadingSystem you need, there is a method in ShaderGroup that we call, ShaderGroup::create_optimized_osl_shader_group. After shader groups are optimized, we could add a similar method to EnvironmentEDFs and call it in a similar way, passing the OSLShadingSystem. There you can construct an OSLShaderGroupExec and build the importance sample map.

[lorentzo]

OK, thank you for the hint. It makes sense.

I have another doubt. How can I determine the resolution of sampling (importance map dimensions)?

In OSL's test render background resolution is parsed from scene file.

In latlongmapenvironmentedf.cpp we can see that m_inputs.source("radiance")->get_hints()contains information about the sampling resolution (importance map dimensions). To be honest, I do not quite understand this part. Also, I do not fully understand the meaning of modeling/input/source.h.

Therefore, should I obtain OSL background sampling resolution also by using m_inputs.source("radiance")->get_hints()?

I would be very useful for me to understand m_inputs and according source values. If I understand correctly, it depends on the scene file (.appleseed)?

[est77]

I guess it could be a parameter of the OSL environment entity with a reasonable default, maybe 1024 or something like that? You can use that for the width of the map and use height = width / 2. Or you can hardcode it for the first version of the issue, as long as it can be changed easily later.

[lorentzo]

OK, thank you for the suggestion. For now, I have hard-coded the value. First, I will perform some tests with hard-coded the value then I will try to get this value from m_inputs.

Currently, I have implemented creation of importance map and sampling which is using this importance map. Work can be seen on my github fork (I have modified only this file):

• I have written OSLEnvironmentSampler class which is using shader_group and osl_shadergroup_exec for sampling the OSL background. This class is similar to ImageSampler.
• Next, I have written OSLEnvironmentEDFImportanceSampler which is used for creation of the importance map by utilizing OSLEnvironmentSampler. Also it is used for sampling of created importance map. This class is similar to ImageImportanceSampler.
• Finally, I have modified OSLEnvironmentEDF::sample() so it is using importance map which was prepared and created by OSLEnvironmentEDF::on_render_begin() and OSLEnvironmentEDF::build_importance_map() (I have also modified those functions). This functions are similar to LatLongMapEnvironmentEDF functions.

There is still some work left. But, I will first perform some tests with current code and then I will add importance sampling to OSLEnvironmentEDF::evaluate() and OSLEnvironmentEDF::evaluate_pdf().

Some questions:

• Should OSLEnvironmentEDF::sample(), OSLEnvironmentEDF::evaluate() and OSLEnvironmentEDF::evaluate_pdf() have uniform sampling option if importance map is (somehow) not build or valid? In LatLongMapEnvironmentEDF only importance sampling is available.
• Currently, OSLEnvironmentEDFImportanceSampler class is in the same file as OSLEnvironmentEDF class. I have noticed that ImageImportanceSampler class is very similar to OSLEnvironmentEDFImportanceSampler class (only different sampler is used in ImageImportanceSampler::rebuild()). Therefore, I am not sure if I should create new file in foundation/math/sampling/ for OSLEnvironmentEDFImportanceSampler class or should I modify the ImageImportanceSampler class file?

[dictoon]

Should OSLEnvironmentEDF::sample() , OSLEnvironmentEDF::evaluate() and OSLEnvironmentEDF::evaluate_pdf() have uniform sampling option if importance map is (somehow) not build or valid? In LatLongMapEnvironmentEDF only importance sampling is available.

I would say no if we can avoid it? Why would the importance map be invalid? If there are legitimate cases, can we make it always valid such that there's no additional if in the actual rendering code path?

Currently, OSLEnvironmentEDFImportanceSampler class is in the same file as OSLEnvironmentEDF class. I have noticed that ImageImportanceSampler class is very similar to OSLEnvironmentEDFImportanceSampler class (only different sampler is used in ImageImportanceSampler::rebuild()). Therefore, I am not sure if I should create new file in foundation/math/sampling/ for OSLEnvironmentEDFImportanceSampler class or should I modify the ImageImportanceSampler class file?

No need to create new files. It's nice to have all related code in one file but that code doesn't need to be reused elsewhere.

Also there shouldn't be any OSL stuff in appleseed.foundation.

[dictoon]

One thing that isn't entirely clear to me: why can't you use foundation::ImageImportanceSampler directly, passing your own image sampler when calling rebuild()?

[lorentzo]

Should OSLEnvironmentEDF::sample() , OSLEnvironmentEDF::evaluate() and OSLEnvironmentEDF::evaluate_pdf() have uniform sampling option if importance map is (somehow) not build or valid? In LatLongMapEnvironmentEDF only importance sampling is available.

I would say no if we can avoid it? Why would the importance map be invalid? If there are legitimate cases, can we make it always valid such that there's no additional if in the actual rendering code path?

OK, I think the word "invalid" is inappropriate. In LatLongMapEnvironmentEDF I have seen this warning. So I was thinking if (for any reason) importance map is not available, then maybe uniform sampling should be enabled. But as more as I am thinking about it, the importance map will be always available.

Currently, OSLEnvironmentEDFImportanceSampler class is in the same file as OSLEnvironmentEDF class. I have noticed that ImageImportanceSampler class is very similar to OSLEnvironmentEDFImportanceSampler class (only different sampler is used in ImageImportanceSampler::rebuild()). Therefore, I am not sure if I should create new file in foundation/math/sampling/ for OSLEnvironmentEDFImportanceSampler class or should I modify the ImageImportanceSampler class file?

No need to create new files. It's nice to have all related code in one file but that code doesn't need to be reused elsewhere.

Also there shouldn't be any OSL stuff in appleseed.foundation.

OK.

[lorentzo]

One thing that isn't entirely clear to me: why can't you use foundation::ImageImportanceSampler directly, passing your own image sampler when calling rebuild()?

Yes, you are right it doesn't make sense. In my current implementation I am using foundation::ImageImportanceSampler. Sorry for not updating this issue comments thread with new information. I have opened PR #2784 where you can see the latest news, the code, and description. (Note for PR: I am still in progress of testing and updating some changes. Tomorrow those changes should be up)

[dictoon]

Now that #2784 is merged, what's left to do in this ticket?

[lorentzo]

I was discussing the next steps with @est77. Using conclusions from that discussion I have opened an issue: #2808.

So, the main idea is to transfer building importance map from on_frame_begin() to on_render_begin(). But in order to achieve that, we need some modifications as explained in #2808.

I will tackle with #2808. Also, any comments on the issue are welcome.

[est]

No dont discuss next steps with me.

[dictoon]

@est Sorry, wrong user! Let me know if you want me to delete your comment (to prevent you from receiving further notifications).

[est]

just kidding. please go on with your discussions. I'll unsubscribe this thread myself.