Fix #108495: Pasting a material crashes (back-port from main) #108567

Closed
Campbell Barton wants to merge 1 commits from ideasman42:pr-material-clipboard-paste-backport into blender-v3.6-release

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
6 changed files with 208 additions and 210 deletions

View File

@ -173,20 +173,6 @@ void ramp_blend(int type, float r_col[3], float fac, const float col[3]);
/** \} */
/* -------------------------------------------------------------------- */
/** \name Copy/Paste
* \{ */
void BKE_material_copybuf_clear(void);
void BKE_material_copybuf_free(void);
void BKE_material_copybuf_copy(struct Main *bmain, struct Material *ma);
/**
* \return true when the material was modified.
*/
bool BKE_material_copybuf_paste(struct Main *bmain, struct Material *ma);
/** \} */
/* -------------------------------------------------------------------- */
/** \name Default Materials
* \{ */

View File

@ -970,18 +970,32 @@ void BKE_blendfile_workspace_config_data_free(WorkspaceConfigFileData *workspace
/** \name Blend File Write (Partial)
* \{ */
void BKE_blendfile_write_partial_begin(Main *bmain_src)
static void blendfile_write_partial_clear_flags(Main *bmain_src)
{
BKE_main_id_tag_all(bmain_src, LIB_TAG_NEED_EXPAND | LIB_TAG_DOIT, false);
ListBase *lbarray[INDEX_ID_MAX];
int a = set_listbasepointers(bmain_src, lbarray);
while (a--) {
LISTBASE_FOREACH (ID *, id, lbarray[a]) {
id->tag &= ~(LIB_TAG_NEED_EXPAND | LIB_TAG_DOIT);
id->flag &= ~(LIB_CLIPBOARD_MARK);
}
}
}
void BKE_blendfile_write_partial_begin(Main *bmain)
{
blendfile_write_partial_clear_flags(bmain);
}
void BKE_blendfile_write_partial_tag_ID(ID *id, bool set)
{
if (set) {
id->tag |= LIB_TAG_NEED_EXPAND | LIB_TAG_DOIT;
id->flag |= LIB_CLIPBOARD_MARK;
}
else {
id->tag &= ~(LIB_TAG_NEED_EXPAND | LIB_TAG_DOIT);
id->flag &= ~LIB_CLIPBOARD_MARK;
}
}
@ -1080,7 +1094,7 @@ bool BKE_blendfile_write_partial(Main *bmain_src,
void BKE_blendfile_write_partial_end(Main *bmain_src)
{
BKE_main_id_tag_all(bmain_src, LIB_TAG_NEED_EXPAND | LIB_TAG_DOIT, false);
blendfile_write_partial_clear_flags(bmain_src);
}
/** \} */

View File

@ -75,8 +75,6 @@
static CLG_LogRef LOG = {"bke.material"};
static void material_clear_data(ID *id);
static void material_init_data(ID *id)
{
Material *material = (Material *)id;
@ -132,26 +130,6 @@ static void material_copy_data(Main *bmain, ID *id_dst, const ID *id_src, const
/* TODO: Duplicate Engine Settings and set runtime to nullptr. */
}
/**
* Ensure pointers to allocated memory is cleared
* (the kind of data that would have to be copied).
*
* \note Keep in sync with #material_free_data.
* \note Doesn't handle animation data (`ma->adt`).
*/
static void material_clear_data(ID *id)
{
Material *material = (Material *)id;
BLI_listbase_clear(&material->gpumaterial);
material->texpaintslot = nullptr;
material->gp_style = nullptr;
material->nodetree = nullptr;
material->preview = nullptr;
id->icon_id = 0;
}
static void material_free_data(ID *id)
{
Material *material = (Material *)id;
@ -1920,170 +1898,6 @@ void ramp_blend(int type, float r_col[3], const float fac, const float col[3])
}
}
/* -------------------------------------------------------------------- */
/** \name Material Copy/Paste
*
* As materials may reference other data, the clipboard only stores a subset of all possible data.
* The material and it's node-tree are stored and nothing else.
* Notably the following variables are *not* part of the clipboard.
*
* - The #ID (name, fake-user, custom-properties .. etc).
* - Animation data (#Material::adt, #bNodeTree::adt).
* - Texture paint slots (#Material::texpaintslot)
* as this is cache and references other ID's.
* - Grease pencil style (#Material::gp_style)
* could be supported but ID's and pointers would need to be handled carefully.
*
* When pasting, some data in the destination material is left as-is:
* - The #ID.
* - Animation data, with the exception that pasting a material without a node-tree
* will clear the existing materials node-tree & its animation.
* Note that applying existing animation to the pasted material might not make sense
* and may reference data-paths that don't resolve (depending on the kind of material).
* The user might need to clear the animation in this case.
*
* \{ */
static Material matcopybuf;
static short matcopied = 0;
void BKE_material_copybuf_clear(void)
{
if (matcopied) {
BKE_material_copybuf_free();
}
matcopybuf = blender::dna::shallow_zero_initialize();
matcopied = 0;
}
/**
* Some members should *never* be set, ensure this is always the case.
* Call at the beginning & end of functions that manipulate the clipboard.
*/
static void material_copybuf_assert_is_valid()
{
BLI_assert(!matcopybuf.id.icon_id);
BLI_assert(!matcopybuf.id.py_instance);
BLI_assert(!matcopybuf.adt);
BLI_assert(!matcopybuf.preview);
if (matcopybuf.nodetree) {
BLI_assert(!matcopybuf.nodetree->id.py_instance);
BLI_assert(!matcopybuf.nodetree->adt);
BLI_assert(!matcopybuf.nodetree->owner_id);
}
}
void BKE_material_copybuf_free(void)
{
material_copybuf_assert_is_valid();
material_free_data(&matcopybuf.id);
matcopied = 0;
}
void BKE_material_copybuf_copy(Main *bmain, Material *ma)
{
material_copybuf_assert_is_valid();
if (matcopied) {
BKE_material_copybuf_free();
}
matcopybuf = blender::dna::shallow_copy(*ma);
/* Not essential, but we never want to use any ID values from the source,
* this ensures that never happens. */
memset(&matcopybuf.id, 0, sizeof(ID));
/* Ensure dangling pointers are never copied back into a material. */
material_clear_data(&matcopybuf.id);
/* Unhandled by materials generic data functions. */
matcopybuf.adt = nullptr;
if (ma->nodetree != nullptr) {
/* Never copy animation data. */
struct {
AnimData *adt;
} backup;
backup.adt = ma->nodetree->adt;
ma->nodetree->adt = nullptr;
matcopybuf.nodetree = blender::bke::ntreeCopyTree_ex(ma->nodetree, bmain, false);
matcopybuf.nodetree->owner_id = nullptr;
ma->nodetree->adt = backup.adt;
}
/* TODO: Duplicate Engine Settings and set runtime to nullptr. */
matcopied = 1;
material_copybuf_assert_is_valid();
}
bool BKE_material_copybuf_paste(Main *bmain, Material *ma)
{
material_copybuf_assert_is_valid();
if (matcopied == 0) {
return false;
}
/* `matcopybuf` never has animation data, no need to check. */
const bool has_animdata = (ma->adt != nullptr || (ma->nodetree && ma->nodetree->adt));
const bool has_node_tree = (ma->nodetree || matcopybuf.nodetree);
AnimData *backup_nodetree_adt = nullptr;
if (ma->nodetree && matcopybuf.nodetree) {
/* Keep data to apply back to the new node-tree. */
std::swap(backup_nodetree_adt, ma->nodetree->adt);
}
/* Handles freeing nodes and and other run-time data (previews) for e.g. */
material_free_data(&ma->id);
/* Copy from `matcopybuf` preserving some members.
* NOTE: animation data isn't stored in the clipboard, any existing animation will be left as-is.
* Any undesired animation will have to be manually cleared by the user. */
{
struct {
ID id;
AnimData *adt;
} backup;
backup.id = ma->id;
backup.adt = ma->adt;
*ma = blender::dna::shallow_copy(matcopybuf);
ma->id = backup.id;
ma->adt = backup.adt;
}
if (matcopybuf.nodetree != nullptr) {
BLI_assert(matcopybuf.nodetree->adt == nullptr);
ma->nodetree = blender::bke::ntreeCopyTree_ex(matcopybuf.nodetree, bmain, false);
ma->nodetree->owner_id = &ma->id;
/* Restore animation pointer (if set). */
BLI_assert(ma->nodetree->adt == nullptr);
ma->nodetree->adt = backup_nodetree_adt;
}
if (has_node_tree || has_animdata) {
/* Important to run this when the embedded tree if freed,
* otherwise the depsgraph holds a reference to the (now freed) `ma->nodetree`.
* Also run this when a new node-tree is set to ensure it's accounted for.
* This also applies to animation data which is likely to be stored in the depsgraph. */
DEG_relations_tag_update(bmain);
}
material_copybuf_assert_is_valid();
return true;
}
/** \} */
void BKE_material_eval(struct Depsgraph *depsgraph, Material *material)
{
DEG_debug_print_eval(depsgraph, __func__, material->id.name, material);

View File

@ -10,6 +10,7 @@
#include "MEM_guardedalloc.h"
#include "DNA_ID.h"
#include "DNA_curve_types.h"
#include "DNA_light_types.h"
#include "DNA_lightprobe_types.h"
@ -23,12 +24,15 @@
#include "BLI_listbase.h"
#include "BLI_math_vector.h"
#include "BLI_path_util.h"
#include "BLI_utildefines.h"
#include "BLT_translation.h"
#include "BKE_anim_data.h"
#include "BKE_animsys.h"
#include "BKE_appdir.h"
#include "BKE_blender_copybuffer.h"
#include "BKE_brush.h"
#include "BKE_context.h"
#include "BKE_curve.h"
@ -37,6 +41,8 @@
#include "BKE_image.h"
#include "BKE_layer.h"
#include "BKE_lib_id.h"
#include "BKE_lib_query.h"
#include "BKE_lib_remap.h"
#include "BKE_lightprobe.h"
#include "BKE_linestyle.h"
#include "BKE_main.h"
@ -2730,8 +2736,7 @@ void TEXTURE_OT_slot_move(wmOperatorType *ot)
/** \name Material Copy Operator
* \{ */
/* material copy/paste */
static int copy_material_exec(bContext *C, wmOperator * /*op*/)
static int copy_material_exec(bContext *C, wmOperator *op)
{
Material *ma = static_cast<Material *>(
CTX_data_pointer_get_type(C, "material", &RNA_Material).data);
@ -2740,7 +2745,19 @@ static int copy_material_exec(bContext *C, wmOperator * /*op*/)
return OPERATOR_CANCELLED;
}
BKE_material_copybuf_copy(CTX_data_main(C), ma);
char filepath[FILE_MAX];
Main *bmain = CTX_data_main(C);
/* Mark is the material to use (others may be expanded). */
BKE_copybuffer_copy_begin(bmain);
BKE_copybuffer_copy_tag_ID(&ma->id);
BLI_path_join(filepath, sizeof(filepath), BKE_tempdir_base(), "copybuffer_material.blend");
BKE_copybuffer_copy_end(bmain, filepath, op->reports);
/* We are all done! */
BKE_report(op->reports, RPT_INFO, "Copied material to internal clipboard");
return OPERATOR_FINISHED;
}
@ -2766,8 +2783,47 @@ void MATERIAL_OT_copy(wmOperatorType *ot)
/** \name Material Paste Operator
* \{ */
/**
* Clear ID's as freeing the data-block doesn't handle reference counting.
*/
static int paste_material_nodetree_ids_decref(LibraryIDLinkCallbackData *cb_data)
{
if (cb_data->cb_flag & IDWALK_CB_USER) {
id_us_min(*cb_data->id_pointer);
}
*cb_data->id_pointer = nullptr;
return IDWALK_RET_NOP;
}
/**
* Re-map ID's from the clipboard to ID's in `bmain`, by name.
*/
static int paste_material_nodetree_ids_relink_or_clear(LibraryIDLinkCallbackData *cb_data)
{
Main *bmain = static_cast<Main *>(cb_data->user_data);
ID **id_p = cb_data->id_pointer;
if (*id_p) {
if (cb_data->cb_flag & IDWALK_CB_USER) {
id_us_min(*id_p);
}
ListBase *lb = which_libbase(bmain, GS((*id_p)->name));
ID *id_local = static_cast<ID *>(
BLI_findstring(lb, (*id_p)->name + 2, offsetof(ID, name) + 2));
*id_p = id_local;
if (cb_data->cb_flag & IDWALK_CB_USER) {
id_us_plus(id_local);
}
else if (cb_data->cb_flag & IDWALK_CB_USER_ONE) {
id_us_ensure_real(id_local);
}
id_lib_extern(id_local);
}
return IDWALK_RET_NOP;
}
static int paste_material_exec(bContext *C, wmOperator *op)
{
Main *bmain = CTX_data_main(C);
Material *ma = static_cast<Material *>(
CTX_data_pointer_get_type(C, "material", &RNA_Material).data);
@ -2776,11 +2832,132 @@ static int paste_material_exec(bContext *C, wmOperator *op)
return OPERATOR_CANCELLED;
}
if (!BKE_material_copybuf_paste(CTX_data_main(C), ma)) {
BKE_report(op->reports, RPT_WARNING, "No material in the internal clipboard to paste");
/* Read copy buffer .blend file. */
char filepath[FILE_MAX];
Main *temp_bmain = BKE_main_new();
STRNCPY(temp_bmain->filepath, BKE_main_blendfile_path_from_global());
BLI_path_join(filepath, sizeof(filepath), BKE_tempdir_base(), "copybuffer_material.blend");
/* NOTE(@ideasman42) The node tree might reference different kinds of ID types.
* It's not clear-cut which ID types should be included, although it's unlikely
* users would want an entire scene & it's objects to be included.
* Filter a subset of ID types with some reasons for including them. */
const uint64_t ntree_filter = (
/* Material is necessary for reading the clipboard. */
FILTER_ID_MA |
/* Node-groups. */
FILTER_ID_NT |
/* Image textures. */
FILTER_ID_IM |
/* Internal text (scripts). */
FILTER_ID_TXT |
/* Texture coordinates may reference objects.
* Note that object data is *not* included. */
FILTER_ID_OB);
if (!BKE_copybuffer_read(temp_bmain, filepath, op->reports, ntree_filter)) {
BKE_report(op->reports, RPT_ERROR, "Internal clipboard is empty");
BKE_main_free(temp_bmain);
return OPERATOR_CANCELLED;
}
/* There may be multiple materials,
* check for a property that marks this as the active material. */
Material *ma_from = nullptr;
LISTBASE_FOREACH (Material *, ma_iter, &temp_bmain->materials) {
if (ma_iter->id.flag & LIB_CLIPBOARD_MARK) {
ma_from = ma_iter;
break;
}
}
/* Make sure data from this file is usable for material paste. */
if (ma_from == nullptr) {
BKE_report(op->reports, RPT_ERROR, "Internal clipboard is not from a material");
BKE_main_free(temp_bmain);
return OPERATOR_CANCELLED;
}
/* Keep animation by moving local animation to the paste node-tree. */
if (ma->nodetree && ma_from->nodetree) {
BLI_assert(ma_from->nodetree->adt == nullptr);
std::swap(ma->nodetree->adt, ma_from->nodetree->adt);
}
/* Needed to update #SpaceNode::nodetree else a stale pointer is used. */
if (ma->nodetree) {
bNodeTree *nodetree = ma->nodetree;
BKE_libblock_remap(bmain, ma->nodetree, ma_from->nodetree, ID_REMAP_FORCE_UI_POINTERS);
/* Free & clear data here, so user counts are handled, otherwise it's
* freed as part of #BKE_main_free which doesn't handle user-counts. */
/* Walk over all the embedded nodes ID's (non-recursively). */
BKE_library_foreach_ID_link(
bmain, &nodetree->id, paste_material_nodetree_ids_decref, nullptr, IDWALK_NOP);
ntreeFreeEmbeddedTree(nodetree);
MEM_freeN(nodetree);
ma->nodetree = nullptr;
}
/* Swap data-block content, while swapping isn't always needed,
* it means memory is properly freed in the case of allocations.. */
#define SWAP_MEMBER(member) std::swap(ma->member, ma_from->member)
/* Intentionally skip:
* - Texture painting slots.
* - Preview render.
* - Grease pencil styles (we could although they reference many ID's themselves).
*/
SWAP_MEMBER(flag);
SWAP_MEMBER(r);
SWAP_MEMBER(g);
SWAP_MEMBER(b);
SWAP_MEMBER(a);
SWAP_MEMBER(specr);
SWAP_MEMBER(specg);
SWAP_MEMBER(specb);
SWAP_MEMBER(spec);
SWAP_MEMBER(roughness);
SWAP_MEMBER(metallic);
SWAP_MEMBER(use_nodes);
SWAP_MEMBER(index);
SWAP_MEMBER(nodetree);
SWAP_MEMBER(line_col);
SWAP_MEMBER(line_priority);
SWAP_MEMBER(vcol_alpha);
SWAP_MEMBER(alpha_threshold);
SWAP_MEMBER(refract_depth);
SWAP_MEMBER(blend_method);
SWAP_MEMBER(blend_shadow);
SWAP_MEMBER(blend_flag);
SWAP_MEMBER(lineart);
#undef SWAP_MEMBER
/* The node-tree from the clipboard is now assigned to the local material,
* however the ID's it references are still part of `temp_bmain`.
* These data-blocks references must be cleared or replaces with references to `bmain`.
* TODO(@ideasman42): support merging indirectly referenced data-blocks besides the material,
* this would be useful for pasting materials with node-groups between files. */
if (ma->nodetree) {
ma->nodetree->owner_id = nullptr;
/* Map remote ID's to local ones. */
BKE_library_foreach_ID_link(
bmain, &ma->nodetree->id, paste_material_nodetree_ids_relink_or_clear, bmain, IDWALK_NOP);
}
BKE_main_free(temp_bmain);
/* Important to run this when the embedded tree if freed,
* otherwise the depsgraph holds a reference to the (now freed) `ma->nodetree`.
* Also run this when a new node-tree is set to ensure it's accounted for.
* This also applies to animation data which is likely to be stored in the depsgraph.
* Always call instead of checking when it *might* be needed. */
DEG_relations_tag_update(bmain);
DEG_id_tag_update(&ma->id, ID_RECALC_COPY_ON_WRITE);
WM_event_add_notifier(C, NC_MATERIAL | ND_SHADING_LINKS, ma);

View File

@ -709,6 +709,15 @@ enum {
* was kept around (because e.g. detected as user-edited).
*/
LIB_LIB_OVERRIDE_RESYNC_LEFTOVER = 1 << 13,
/**
* This `id` was explicitly copied as part of a clipboard copy operation.
* When reading the clipboard back, this can be used to check which ID's are
* intended to be part of the clipboard, compared with ID's that were indirectly referenced.
*
* While the flag is typically cleared, a saved file may have this set for some data-blocks,
* so it must be treated as dirty.
*/
LIB_CLIPBOARD_MARK = 1 << 14,
};
/**

View File

@ -349,7 +349,6 @@ void WM_init(bContext *C, int argc, const char **argv)
}
}
BKE_material_copybuf_clear();
ED_render_clear_mtex_copybuf();
wm_history_file_read();
@ -608,7 +607,6 @@ void WM_exit_ex(bContext *C, const bool do_python, const bool do_user_exit_actio
DRW_subdiv_free();
}
BKE_material_copybuf_free();
ANIM_fcurves_copybuf_free();
ANIM_drivers_copybuf_free();
ANIM_driver_vars_copybuf_free();