Fix #119427: UI: The Metadata is gone for the rendered image #119503

Sergey Sharybin · 2024-03-15T11:21:02+01:00

Sergey Sharybin commented

2024-03-15 11:21:02 +01:00

This is a regression since d579ac2b3f

Ensure that the image buffers used by render passes have metadata at the
end of render, similarly to how stamping happens. This solves the reported
issue, and makes the metadata behave consistently, independent on how the
image buffer is accessed.

Thanks Philipp Oeser for investigation and pin-pointing the bad commit!

This is a regression since d579ac2b3f Ensure that the image buffers used by render passes have metadata at the end of render, similarly to how stamping happens. This solves the reported issue, and makes the metadata behave consistently, independent on how the image buffer is accessed. Thanks Philipp Oeser for investigation and pin-pointing the bad commit!

Sergey Sharybin added 1 commit 2024-03-15 11:21:10 +01:00

bdcae32daf Fix #119427 : UI: The Metadata is gone for the rendered image

This is a regression since d579ac2b3f

Ensure that the image buffers used by render passes have metadata at the
end of render, similarly to how stamping happens. This solves the reported
issue, and makes the metadata behave consistently, independent on how the
image buffer is accessed.

Thanks Philipp Oeser for investigation and pin-pointing the bad commit!

Sergey Sharybin requested review from Brecht Van Lommel 2024-03-15 11:21:25 +01:00

Sergey Sharybin requested review from Philipp Oeser 2024-03-15 11:21:25 +01:00

Philipp Oeser approved these changes 2024-03-15 11:47:18 +01:00

Philipp Oeser left a comment

works fine for me

Brecht Van Lommel requested changes 2024-03-15 14:00:28 +01:00

Dismissed

Brecht Van Lommel left a comment

This doesn't look thread safe, as far as I can tell the render thread can write and the drawing code can read this metadata at the same time.

Any reason not to do it in image_get_render_result as before?

This doesn't look thread safe, as far as I can tell the render thread can write and the drawing code can read this metadata at the same time. Any reason not to do it in `image_get_render_result` as before?

Philipp Oeser commented

2024-03-15 14:32:50 +01:00

Any reason not to do it in image_get_render_result as before?

#119495 (comment)

> Any reason not to do it in `image_get_render_result` as before? https://projects.blender.org/blender/blender/pulls/119495#issuecomment-1146991

Sergey Sharybin added 1 commit 2024-03-15 14:34:18 +01:00

7661e4b875 Fix potential threading issue

Acquire the result mutex, similarly how it is done for stamping, but do
it once for all render passes.

Brecht Van Lommel requested changes 2024-03-15 14:38:19 +01:00

Dismissed

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

						
				@ -1331,0 +1332,4 @@

				{

				  RenderResult *render_result = re->result;

				  BLI_rw_mutex_lock(&re->resultmutex, THREAD_LOCK_READ);

Brecht Van Lommel commented

2024-03-15 14:38:13 +01:00

THREAD_LOCK_WRITE?

`THREAD_LOCK_WRITE`?

Sergey Sharybin commented

2024-03-15 14:38:53 +01:00

@brecht Long story short: yes :) In practice doing it at the of the render allows:

Make access to a metadata of an ImBuf of a render pass consistent, regardless of whether you access image buffer via RenderResult structure, or via Render Result image data-block
Avoid re-allocation of ibuf->metadata on every redraw

Speculatively, it also improves const-correctness of the ImBuf, and makes it possible to acquire render result's ImBuf, increment ref-counter, and access the buffer in a thread-safe manner, without worrying someone will yank metadata out of your nose. A bit more theoretical, since it is not really what we know is ever currently happening in practice, but still.

@brecht Long story short: yes :) In practice doing it at the of the render allows: - Make access to a metadata of an `ImBuf` of a render pass consistent, regardless of whether you access image buffer via `RenderResult` structure, or via `Render Result` image data-block - Avoid re-allocation of `ibuf->metadata` on every redraw Speculatively, it also improves const-correctness of the `ImBuf`, and makes it possible to acquire render result's ImBuf, increment ref-counter, and access the buffer in a thread-safe manner, without worrying someone will yank metadata out of your nose. A bit more theoretical, since it is not really what we know is ever currently happening in practice, but still.

Sergey Sharybin added 1 commit 2024-03-15 14:39:42 +01:00

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

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

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

d0e589b471 Fix incorrect mutex acquisition

Brecht Van Lommel approved these changes 2024-03-15 14:39:52 +01:00

Sergey Sharybin commented

2024-03-15 14:41:01 +01:00

@blender-bot build

Sergey Sharybin merged commit e2d7bd199e into blender-v4.1-release

2024-03-15 15:24:43 +01:00

Sergey Sharybin referenced this issue from a commit

2024-03-15 15:24:44 +01:00

Fix #119427: UI: The Metadata is gone for the rendered image

Sergey Sharybin deleted branch fix_119427

2024-03-15 15:24:50 +01:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #119427: UI: The Metadata is gone for the rendered image #119503