Realtime Compositor: cache render pass GPU textures #108818

Brecht Van Lommel · 2023-06-09T18:55:31+02:00

Brecht Van Lommel commented

2023-06-09 18:55:31 +02:00

Store per RenderPass in RenderResult.
Caches are cleared when starting rendering, to make more memory available
to GPU rendering.
Caches are cleared on UI changes, when no compositing node editor and no
image editor with a render result or viewer node image is visible.
Store 3 channel RGB passes as such, and set alpha 1 in shader.

This is an intermediate step before implementing GPU backed ImBuf, to
improve performance and figure out cache eviction.

* Store per RenderPass in RenderResult. * Caches are cleared when starting rendering, to make more memory available to GPU rendering. * Caches are cleared on UI changes, when no compositing node editor and no image editor with a render result or viewer node image is visible. * Store 3 channel RGB passes as such, and set alpha 1 in shader. This is an intermediate step before implementing GPU backed ImBuf, to improve performance and figure out cache eviction.

Brecht Van Lommel added 1 commit 2023-06-09 18:55:42 +02:00

ab5803d799 Realtime Compositor: cache render pass GPU textures

* Store per RenderPass in RenderResult.
* Caches are cleared when starting rendering, to make more memory available
  to GPU rendering.
* Caches are cleared on UI changes, when no compositing node editor and no
  image editor with a render result or viewer node image is visible.
* Store 3 channel RGB passes as such, and set alpha 1 in shader.

This is an intermediate step before implementing GPU backed ImBuf, to
improve performance and figure out cache eviction.

Brecht Van Lommel requested review from Sergey Sharybin 2023-06-09 18:57:38 +02:00

Brecht Van Lommel requested review from Omar Emara 2023-06-09 18:57:38 +02:00

Omar Emara requested changes 2023-06-10 15:06:42 +02:00

Omar Emara left a comment

I get the following crash when rendering a Cycles scene, maybe a missing nullptr initialization?

* thread #59, name = 'blender', stop reason = signal SIGSEGV: invalid address (fault address: 0x7fff0000000c)
    frame #0: 0x0000000010019538 blender`::GPU_texture_free(tex_=0x00007fff00000000) at gpu_texture.cc:672:16
   669  void GPU_texture_free(GPUTexture *tex_)
   670  {
   671    Texture *tex = reinterpret_cast<Texture *>(tex_);
-> 672    tex->refcount--;
   673
   674    if (tex->refcount < 0) {
   675      fprintf(stderr, "GPUTexture: negative refcount\n");
(lldb) bt
* thread #59, name = 'blender', stop reason = signal SIGSEGV: invalid address (fault address: 0x7fff0000000c)
  * frame #0: 0x0000000010019538 blender`::GPU_texture_free(tex_=0x00007fff00000000) at gpu_texture.cc:672:16
    frame #1: 0x00000000087dcc8b blender`void render_buffer_data_free<RenderBuffer>(render_buffer=0x00007fffaf2982e8) at render_result.cc:1260:5
    frame #2: 0x00000000087d9485 blender`::RE_RenderBuffer_data_free(render_buffer=0x00007fffaf2982e8) at render_result.cc:1308:3
    frame #3: 0x00000000087d9384 blender`::render_result_free(rr=0x00007fffaf20e1c8) at render_result.cc:79:7
    frame #4: 0x00000000087c8350 blender`::RE_engine_end_result(engine=0x00007ffface5d008, result=0x00007fffaf20e1c8, cancel=false, highlight=false, merge_results=true) at engine.cc:479:3
    frame #5: 0x000000000782eb5c blender`RenderEngine_end_result_func(_self=0x00007ffface5d008, result=0x00007fffaf20e1c8, cancel=false, highlight=false, do_merge_results=true) at rna_render_gen.c:1291:2
    frame #6: 0x0000000009f58674 blender`BL::RenderEngine::end_result(this=0x00007ffface3cd28, result=0x00007fffa57ab4d0, cancel=false, highlight=false, do_merge_results=true) at RNA_blender_cpp.h:67087:3
    frame #7: 0x0000000009f578cb blender`ccl::BlenderOutputDriver::write_render_tile(this=0x00007ffface3cd20, tile=0x00007fffa57ab878) at output_driver.cpp:104:13

I get the following crash when rendering a Cycles scene, maybe a missing nullptr initialization? ``` * thread #59, name = 'blender', stop reason = signal SIGSEGV: invalid address (fault address: 0x7fff0000000c) frame #0: 0x0000000010019538 blender`::GPU_texture_free(tex_=0x00007fff00000000) at gpu_texture.cc:672:16 669 void GPU_texture_free(GPUTexture *tex_) 670 { 671 Texture *tex = reinterpret_cast<Texture *>(tex_); -> 672 tex->refcount--; 673 674 if (tex->refcount < 0) { 675 fprintf(stderr, "GPUTexture: negative refcount\n"); (lldb) bt * thread #59, name = 'blender', stop reason = signal SIGSEGV: invalid address (fault address: 0x7fff0000000c) * frame #0: 0x0000000010019538 blender`::GPU_texture_free(tex_=0x00007fff00000000) at gpu_texture.cc:672:16 frame #1: 0x00000000087dcc8b blender`void render_buffer_data_free<RenderBuffer>(render_buffer=0x00007fffaf2982e8) at render_result.cc:1260:5 frame #2: 0x00000000087d9485 blender`::RE_RenderBuffer_data_free(render_buffer=0x00007fffaf2982e8) at render_result.cc:1308:3 frame #3: 0x00000000087d9384 blender`::render_result_free(rr=0x00007fffaf20e1c8) at render_result.cc:79:7 frame #4: 0x00000000087c8350 blender`::RE_engine_end_result(engine=0x00007ffface5d008, result=0x00007fffaf20e1c8, cancel=false, highlight=false, merge_results=true) at engine.cc:479:3 frame #5: 0x000000000782eb5c blender`RenderEngine_end_result_func(_self=0x00007ffface5d008, result=0x00007fffaf20e1c8, cancel=false, highlight=false, do_merge_results=true) at rna_render_gen.c:1291:2 frame #6: 0x0000000009f58674 blender`BL::RenderEngine::end_result(this=0x00007ffface3cd28, result=0x00007fffa57ab4d0, cancel=false, highlight=false, do_merge_results=true) at RNA_blender_cpp.h:67087:3 frame #7: 0x0000000009f578cb blender`ccl::BlenderOutputDriver::write_render_tile(this=0x00007ffface3cd20, tile=0x00007fffa57ab878) at output_driver.cpp:104:13 ```

source/blender/compositor/realtime_compositor/COM_context.hh Outdated

						
				@ -84,1 +84,3 @@

				  virtual GPUTexture *get_input_texture(int view_layer, const char *pass_name) = 0;

				  struct InputTexture {

				    GPUTexture *texture;

				    int num_channels;

Omar Emara commented

2023-06-10 15:02:39 +02:00

This num_channels seems redundant since it is only used to distinguish non-alpha RGB colors where alpha will be set to 1. But this is already the default for texture samplers in shaders. So this can probably be removed and a single compositor_read_pass_color shader can be used.

This `num_channels` seems redundant since it is only used to distinguish non-alpha RGB colors where alpha will be set to 1. But this is already the default for texture samplers in shaders. So this can probably be removed and a single `compositor_read_pass_color` shader can be used.

brecht marked this conversation as resolved

source/blender/nodes/composite/nodes/node_composite_image.cc

						
				@ -832,0 +836,4 @@

				        execute_pass(image_result, combined.texture, "compositor_read_pass_rgba");

				      }

				      if (alpha_result.should_compute()) {

				        execute_pass(alpha_result, combined.texture, "compositor_read_pass_rgba");

Omar Emara commented

2023-06-10 15:03:12 +02:00

Should this be compositor_read_pass_alpha instead?

Should this be `compositor_read_pass_alpha` instead?

brecht marked this conversation as resolved

source/blender/render/intern/pipeline.cc Outdated

						
				@ -653,0 +680,4 @@

				              do_free = false;

				            }

				          }

				          else if (space.spacetype == SPACE_NODE) {

Omar Emara commented

2023-06-10 15:04:56 +02:00

Should this be SPACE_IMAGE?

Should this be `SPACE_IMAGE`?

brecht marked this conversation as resolved

Brecht Van Lommel added 2 commits 2023-06-12 10:47:16 +02:00

a8cd82ed19 Address comments

buildbot/vexp-code-patch-coordinator Build done. Details

35110d2df2 Merge branch 'main' into cache-gpu-texture

Brecht Van Lommel commented

2023-06-12 10:49:51 +02:00

Thanks, should all be fixed now.

Omar Emara approved these changes 2023-06-12 18:09:29 +02:00

Omar Emara left a comment

I feel like implementing this as a CachedResource in the compositor context might be more robust solution, but since the we still don't have a persistent context for the render compositor, this should be okay as an intermediate step.

I feel like implementing this as a `CachedResource` in the compositor context might be more robust solution, but since the we still don't have a persistent context for the render compositor, this should be okay as an intermediate step.

Brecht Van Lommel commented

2023-06-12 18:35:42 +02:00

The idea is that these GPU textures will not just be used for the compositor.

The plan in #108618 is that render engines will be able to create and write to them, and the image editor will be able to use them for drawing, also when no compositor is involved. In such cases they will not be just cached resources, they can be the actual render result until there is a reason to copy them from GPU to CPU memory.

This would become part of ImBuf, so that image textures and sequencer can share some of the code as well.

The idea is that these GPU textures will not just be used for the compositor. The plan in #108618 is that render engines will be able to create and write to them, and the image editor will be able to use them for drawing, also when no compositor is involved. In such cases they will not be just cached resources, they can be the actual render result until there is a reason to copy them from GPU to CPU memory. This would become part of `ImBuf`, so that image textures and sequencer can share some of the code as well.

Brecht Van Lommel commented

2023-06-12 18:37:35 +02:00

@blender-bot build