Fix #108263: Show warning when cache baking wants to overwrite data #108288

Merged
Lukas Tönne merged 2 commits from LukasTonne/blender:warn_on_bake_dir_conflict into blender-v3.6-release 2023-06-16 10:10:27 +02:00
1 changed files with 160 additions and 5 deletions

View File

@ -49,6 +49,10 @@
#include "object_intern.h"
#include "WM_api.h"
#include "UI_interface.h"
namespace blender::ed::object::bake_simulation {
static bool calculate_to_frame_poll(bContext *C)
@ -250,10 +254,6 @@ static void bake_simulation_job_startjob(void *customdata,
if (md->type == eModifierType_Nodes) {
NodesModifierData *nmd = reinterpret_cast<NodesModifierData *>(md);
nmd->simulation_cache->ptr->reset();
if (StringRef(nmd->simulation_bake_directory).is_empty()) {
nmd->simulation_bake_directory = BLI_strdup(
bke::sim::get_default_modifier_bake_directory(*job.bmain, *object, *md).c_str());
}
char absolute_bake_dir[FILE_MAX];
STRNCPY(absolute_bake_dir, nmd->simulation_bake_directory);
BLI_path_abs(absolute_bake_dir, base_path);
@ -360,7 +360,7 @@ static void bake_simulation_job_endjob(void *customdata)
WM_main_add_notifier(NC_OBJECT | ND_MODIFIER, nullptr);
}
static int bake_simulation_invoke(bContext *C, wmOperator *op, const wmEvent * /*event*/)
static int bake_simulation_execute(bContext *C, wmOperator *op)
{
wmWindowManager *wm = CTX_wm_manager(C);
Scene *scene = CTX_data_scene(C);
@ -403,6 +403,160 @@ static int bake_simulation_invoke(bContext *C, wmOperator *op, const wmEvent * /
return OPERATOR_RUNNING_MODAL;
}
struct PathStringHash {
uint64_t operator()(const StringRef s) const
{
/* Normalize the paths so we can compare them. */
DynamicStackBuffer<256> norm_buf(s.size() + 1, 8);
memcpy(norm_buf.buffer(), s.data(), s.size() + 1);
char *norm = static_cast<char *>(norm_buf.buffer());
BLI_path_slash_native(norm);
/* Strip ending slash. */
LukasTonne marked this conversation as resolved Outdated

Looks like this is leaking. You could use DynamicStackBuffer.

Looks like this is leaking. You could use `DynamicStackBuffer`.
BLI_path_slash_rstrip(norm);
BLI_path_normalize(norm);
return get_default_hash(norm);
}
};
LukasTonne marked this conversation as resolved Outdated

Can use get_default_hash.

Can use `get_default_hash`.
struct PathStringEquality {
bool operator()(const StringRef a, const StringRef b) const
{
return BLI_path_cmp_normalized(a.data(), b.data()) == 0;
}
};
static bool bake_directory_has_data(const StringRefNull absolute_bake_dir)
{
char meta_dir[FILE_MAX];
BLI_path_join(meta_dir, sizeof(meta_dir), absolute_bake_dir.c_str(), "meta");
char bdata_dir[FILE_MAX];
BLI_path_join(bdata_dir, sizeof(bdata_dir), absolute_bake_dir.c_str(), "bdata");
LukasTonne marked this conversation as resolved Outdated

bool bake_directory_has_data(...)

`bool bake_directory_has_data(...)`
if (!BLI_is_dir(meta_dir) || !BLI_is_dir(bdata_dir)) {
return false;
}
return true;
}
static void bake_simulation_validate_paths(bContext *C,
wmOperator *op,
const Span<Object *> objects)
{
Main *bmain = CTX_data_main(C);
for (Object *object : objects) {
if (!BKE_id_is_editable(bmain, &object->id)) {
continue;
}
LISTBASE_FOREACH (ModifierData *, md, &object->modifiers) {
if (md->type != eModifierType_Nodes) {
continue;
}
NodesModifierData *nmd = reinterpret_cast<NodesModifierData *>(md);
if (StringRef(nmd->simulation_bake_directory).is_empty()) {
BKE_reportf(op->reports,
RPT_INFO,
"Bake directory of object %s, modifier %s is empty, setting default path",
object->id.name + 2,
md->name);
LukasTonne marked this conversation as resolved Outdated

Use early continue to avoid indentation.

Use early `continue` to avoid indentation.
nmd->simulation_bake_directory = BLI_strdup(
bke::sim::get_default_modifier_bake_directory(*bmain, *object, *md).c_str());
}
}
}
}
/* Map for counting path references. */
using PathUsersMap = Map<std::string,
int,
default_inline_buffer_capacity(sizeof(std::string)),
DefaultProbingStrategy,
PathStringHash,
PathStringEquality>;
static PathUsersMap bake_simulation_get_path_users(bContext *C, const Span<Object *> objects)
{
Main *bmain = CTX_data_main(C);
PathUsersMap path_users;
for (const Object *object : objects) {
const char *base_path = ID_BLEND_PATH(bmain, &object->id);
LISTBASE_FOREACH (const ModifierData *, md, &object->modifiers) {
if (md->type != eModifierType_Nodes) {
continue;
}
const NodesModifierData *nmd = reinterpret_cast<const NodesModifierData *>(md);
if (StringRef(nmd->simulation_bake_directory).is_empty()) {
continue;
}
char absolute_bake_dir[FILE_MAX];
STRNCPY(absolute_bake_dir, nmd->simulation_bake_directory);
BLI_path_abs(absolute_bake_dir, base_path);
path_users.add_or_modify(
absolute_bake_dir, [](int *value) { *value = 1; }, [](int *value) { ++(*value); });
}
}

Think there should be confirmation dialog in this case as well.

Think there should be confirmation dialog in this case as well.

Had some discussion with @dfelinto about this issue, agreed on:

  1. Bake directory conflict should be handled as error. If we detect this there should be an error dialog and the operator won't run.
  2. After that, if the output directory already has cache data we show the confirmation dialog. Users can proceed at their discretion.

We also discussed the possibility of fixing directory paths automatically. Currently, when copying an object the bake path remains the same, leading to a path conflict. We can clear the path on copy to avoid frequent conflicts. The paths might then be auto-generated to the default on running the bake operator.
Fixing paths by appending a ".001" suffix is difficult because we don't know all the objects/modifiers that might have conflicts (e.g. in linked files). We only know a concrete set of paths to check when actually running the operator.

Had some discussion with @dfelinto about this issue, agreed on: 1. Bake directory conflict should be handled as error. If we detect this there should be an error dialog and the operator won't run. 2. After that, if the output directory already has cache data we show the confirmation dialog. Users can proceed at their discretion. We also discussed the possibility of fixing directory paths automatically. Currently, when copying an object the bake path remains the same, leading to a path conflict. We can clear the path on copy to avoid frequent conflicts. The paths might then be auto-generated to the default on running the bake operator. Fixing paths by appending a ".001" suffix is difficult because we don't know all the objects/modifiers that might have conflicts (e.g. in linked files). We only know a concrete set of paths to check when actually running the operator.
return path_users;
}
static int bake_simulation_invoke(bContext *C, wmOperator *op, const wmEvent * /*event*/)
{
Vector<Object *> objects;
if (RNA_boolean_get(op->ptr, "selected")) {
CTX_DATA_BEGIN (C, Object *, object, selected_objects) {
objects.append(object);
}
CTX_DATA_END;
}
else {
if (Object *object = CTX_data_active_object(C)) {
objects.append(object);
}
}
/* Set empty paths to default. */
bake_simulation_validate_paths(C, op, objects);
PathUsersMap path_users = bake_simulation_get_path_users(C, objects);
bool has_path_conflict = false;
bool has_existing_bake_data = false;
for (const auto &item : path_users.items()) {
/* Check if multiple caches are writing to the same bake directory. */
if (item.value > 1) {
BKE_reportf(op->reports,
RPT_ERROR,
"Path conflict: %d caches set to path %s",
item.value,
item.key.data());
has_path_conflict = true;
}
/* Check if path exists and contains bake data already. */
if (bake_directory_has_data(item.key.data())) {
has_existing_bake_data = true;
}
}
if (has_path_conflict) {
UI_popup_menu_reports(C, op->reports);
return OPERATOR_CANCELLED;
}
if (has_existing_bake_data) {
return WM_operator_confirm_message(C, op, "Overwrite existing bake data");
}
return bake_simulation_execute(C, op);
}
static int bake_simulation_modal(bContext *C, wmOperator * /*op*/, const wmEvent * /*event*/)
{
if (!WM_jobs_test(CTX_wm_manager(C), CTX_data_scene(C), WM_JOB_TYPE_BAKE_SIMULATION_NODES)) {
@ -491,6 +645,7 @@ void OBJECT_OT_simulation_nodes_cache_bake(wmOperatorType *ot)
ot->description = "Bake simulations in geometry nodes modifiers";
ot->idname = __func__;
ot->exec = bake_simulation_execute;
ot->invoke = bake_simulation_invoke;
ot->modal = bake_simulation_modal;
ot->poll = bake_simulation_poll;