appleseedhq / appleseed

A modern open source rendering engine for animation and visual effects
https://appleseedhq.net/
MIT License
2.19k stars 329 forks source link

Enable entities to build importance map for OSL shaders in on_render_begin() #2808

Closed lorentzo closed 4 years ago

lorentzo commented 4 years ago

Motivation:

This issue occurred while solving the issue #701 in PR #2784. Segmentation error occured if build_importance_map() is called from on_render_begin() in OSLEnvironmentEDF. The reason is that OSL shaders (that are required for building importance map) haven't been compiled.

@est77:

Compiling shaders shouldn't be done in on_render_begin(). Shaders have different lifetime than other entities. It is desirable to keep them alive as much as possible.

NB: on_render_begin() should be used opposed to on_frame_begin() when possible. See this discussion on Discord for further explanation. But not in this case.

@est77:

When creating a master renderer, one shading system is created in CPURenderDevice. Instead of creating a new one, the shading system should be reused. The problem with creating a new shading system is seen for example when entities (light, environment, ...) build an importance map (e.g. one shading system per light is not acceptable).

Solution

Proposed by @est77

  1. In masterernderer.cpp, shader compilation (line 359) should be moved before on_render_begin() is called so that shaders are compiled when on_render_begin() is called.

  2. Then, building an importance map for entities should be moved in on_render_begin().

PROBLEM:

  1. Finally, shading system should be passed as an argument in on_render_begin() to all entities.