WM: Add runtime struct #118157

Hans Goudey · 2024-02-12T20:24:42+01:00

Hans Goudey commented

2024-02-12 20:24:42 +01:00

Currently most of the data stored in wmWindowManager is runtime
data not saved to files. It's confusing that it's declared in DNA then. That
also prevents us from using C++ features. This commit adds an initial
runtime struct. Moving data there can be done as a separate step.
Initially I wanted to look at moving the ReportList system to C++.

The runtime struct has to be defined in the blenkernel module because
the members are (will be) used there in a few places.

Currently most of the data stored in `wmWindowManager` is runtime data not saved to files. It's confusing that it's declared in DNA then. That also prevents us from using C++ features. This commit adds an initial runtime struct. Moving data there can be done as a separate step. Initially I wanted to look at moving the `ReportList` system to C++. The runtime struct has to be defined in the blenkernel module because the members are (will be) used there in a few places.

Hans Goudey added 1 commit 2024-02-12 20:24:52 +01:00

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

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

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

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

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

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

992c145217 WM: Add empty runtime struct

Currently most of the data stored in `wmWindowManager` is runtime
data not saved to files. It's confusing that it's declared in DNA then. That
also prevents us from using C++ features. This commit adds an initial
runtime struct. Moving data there can be done as a separate step.
Initially I wanted to look at moving the `ReportList` system to C++.

The runtime struct has to be defined in the blenkernel module because
the members are (will be) used there in a few places.

Hans Goudey commented

2024-02-12 20:25:05 +01:00

@blender-bot build

Hans Goudey requested review from Sergey Sharybin 2024-02-12 20:25:17 +01:00

Sergey Sharybin reviewed 2024-02-13 17:19:58 +01:00

source/blender/windowmanager/intern/wm.cc

						
				@ -113,6 +113,8 @@ static void window_manager_blend_write(BlendWriter *writer, ID *id, const void *

				{

				  wmWindowManager *wm = (wmWindowManager *)id;

				  wm->runtime = nullptr;

Sergey Sharybin commented

2024-02-13 17:19:33 +01:00

There are two weird aspects to this:

Modifying something that is supposed to be const during .blend file save. Not sure why do we even need to do anything special about it here: the field is assigned to nullptr on load anyway.
Even if the modification is needed for some reason, the function should not leave the data modified at return. Otherwise the runtime field in the wm will become nullptr, and also memory will leak.

There are two weird aspects to this: - Modifying something that is supposed to be `const` during .blend file save. Not sure why do we even need to do anything special about it here: the field is assigned to `nullptr` on load anyway. - Even if the modification is needed for some reason, the function should not leave the data modified at return. Otherwise the runtime field in the wm will become nullptr, and also memory will leak.

Hans Goudey commented

2024-02-13 17:33:25 +01:00

Yeah, this is confusing, I should have mentioned it in the PR. This blend_write callback is called with a shallow copy of the ID data-block:

id_type->blend_write(&writer, static_cast<ID *>(id_buffer->temp_id), id);

Clearing this runtime data is often done to make its runtime status clear, and to avoid comparing the pointers in global undo. For example, mesh->runtime = nullptr; in mesh_blend_write.

Yeah, this is confusing, I should have mentioned it in the PR. This `blend_write` callback is called with a shallow copy of the ID data-block: ```cpp id_type->blend_write(&writer, static_cast<ID *>(id_buffer->temp_id), id); ``` Clearing this runtime data is often done to make its runtime status clear, and to avoid comparing the pointers in global undo. For example, `mesh->runtime = nullptr;` in `mesh_blend_write`.

Sergey Sharybin commented

2024-02-13 17:37:36 +01:00

I see. I'd call it temp_id as the input parameter then, but thats a separate topic.
And we'd need to assign the sce->runtime to nullptr in scene_blend_write.

I see. I'd call it `temp_id` as the input parameter then, but thats a separate topic. And we'd need to assign the `sce->runtime` to nullptr in `scene_blend_write`.

Sergey Sharybin approved these changes 2024-02-13 17:38:55 +01:00

Hans Goudey merged commit 21cea65ea6 into main

2024-02-13 17:45:14 +01:00

Hans Goudey referenced this issue from a commit

2024-02-13 17:45:15 +01:00

WM: Add runtime struct

Hans Goudey deleted branch wm-runtime-struct

2024-02-13 17:45:16 +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

WM: Add runtime struct #118157