ColorRamp Keyframes crash Blender #85870

New Issue

David · 2021-02-22T12:00:53+01:00

David commented

2021-02-22 12:00:53 +01:00

System Information
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Graphics card: GeForce RTX 2070/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 460.89

Blender Version
Broken: version: In any version the feature is integrated in. e.g. since version 2.90
Worked: None

Short description of error
Creating a keyframe of ColorRamp's position value crashes blender.

Exact steps for others to reproduce the error
This works in any file, with every mesh and every material. Example: Bug Report.blend

Open Blender and select General under "New File".
Select the default cube and Open the Shader Editor
Create a "ColorRamp" node and connect its "Color" output to the "Base Color" input of the Principled BSDF.
Go to Material Properties -> Surface and expand Base Color [ColorRamp]
Create a keyframe of "Pos" at any value by clicking the dot to the right.

**System Information** Operating system: Windows-10-10.0.19041-SP0 64 Bits Graphics card: GeForce RTX 2070/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 460.89 **Blender Version** Broken: version: In any version the feature is integrated in. e.g. since version 2.90 Worked: None **Short description of error** Creating a keyframe of ColorRamp's position value crashes blender. **Exact steps for others to reproduce the error** This works in any file, with every mesh and every material. Example: [Bug Report.blend](https://archive.blender.org/developer/F9827633/Bug_Report.blend) 1. Open Blender and select General under "New File". 2. Select the default cube and Open the Shader Editor 3. Create a "ColorRamp" node and connect its "Color" output to the "Base Color" input of the Principled BSDF. 4. Go to Material Properties -> Surface and expand Base Color [ColorRamp] ![Bug Report.png](https://archive.blender.org/developer/F9827631/Bug_Report.png) 5. Create a keyframe of "Pos" at any value by clicking the dot to the right.

David commented

2021-02-22 12:00:53 +01:00

Added subscriber: @FalseHeaven

blender-admin commented

2021-02-22 12:00:53 +01:00

#102212 was marked as duplicate of this issue

blender-admin commented

2021-02-22 12:00:53 +01:00

#101687 was marked as duplicate of this issue

blender-admin commented

2021-02-22 12:00:53 +01:00

#86057 was marked as duplicate of this issue

blender-admin commented

2021-02-22 12:00:53 +01:00

#97546 was marked as duplicate of this issue

blender-admin commented

2021-02-22 12:00:53 +01:00

#91340 was marked as duplicate of this issue

Charlie Jolly commented

2021-02-22 13:21:51 +01:00

Added subscriber: @CharlieJolly

Charlie Jolly commented

2021-02-22 13:21:51 +01:00

Confirmed. I believe keyframing POS shouldn't be allowed as the data structure for the items is not ordered to allow this. However, here is the call stack from Windows debug build if it helps. Seems that there is no check for NULL property in the rna_property_update function.

 	
 	blender.exe!rna_property_update(bContext * C=0x000001eaddcb9658, Main * bmain=0x000001eade6da208, Scene * scene=0x000001eade6dcf18, PointerRNA * ptr=0x0000008b343fe610, PropertyRNA * prop=0x0000000000000000) Line 2230	C
 	blender.exe!RNA_property_update(bContext * C=0x000001eaddcb9658, PointerRNA * ptr=0x0000008b343fe610, PropertyRNA * prop=0x0000000000000000) Line 2320	C
 	blender.exe!ui_apply_but_funcs_after(bContext * C=0x000001eaddcb9658) Line 946	C
 	blender.exe!ui_region_handler(bContext * C=0x000001eaddcb9658, const wmEvent * event=0x000001ea891380a8, void * UNUSED_userdata=0x0000000000000000) Line 10756	C
 	blender.exe!wm_handler_ui_call(bContext * C=0x000001eaddcb9658, wmEventHandler_UI * handler=0x000001eaf0e2f6f8, const wmEvent * event=0x000001ea891380a8, int always_pass=0) Line 663	C
 	blender.exe!wm_handlers_do_intern(bContext * C=0x000001eaddcb9658, wmEvent * event=0x000001ea891380a8, ListBase * handlers=0x000001eade6f3190) Line 2798	C
 	blender.exe!wm_handlers_do(bContext * C=0x000001eaddcb9658, wmEvent * event=0x000001ea891380a8, ListBase * handlers=0x000001eade6f3190) Line 2914	C
 	blender.exe!wm_event_do_handlers(bContext * C=0x000001eaddcb9658) Line 3410	C
 	blender.exe!WM_main(bContext * C=0x000001eaddcb9658) Line 640	C
 	blender.exe!main(int argc=1, const unsigned char * * UNUSED_argv_c=0x000001eada5e2640) Line 526	C
 	blender.exe!invoke_main() Line 79	C++
 	blender.exe!__scrt_common_main_seh() Line 288	C++
 	blender.exe!__scrt_common_main() Line 331	C++
 	blender.exe!mainCRTStartup() Line 17	C++
 	kernel32.dll!00007ff953f67c24()	Unknown
 	ntdll.dll!00007ff95514d4d1()	Unknown

Confirmed. I believe keyframing POS shouldn't be allowed as the data structure for the items is not ordered to allow this. However, here is the call stack from Windows debug build if it helps. Seems that there is no check for NULL property in the rna_property_update function. ``` blender.exe!rna_property_update(bContext * C=0x000001eaddcb9658, Main * bmain=0x000001eade6da208, Scene * scene=0x000001eade6dcf18, PointerRNA * ptr=0x0000008b343fe610, PropertyRNA * prop=0x0000000000000000) Line 2230 C blender.exe!RNA_property_update(bContext * C=0x000001eaddcb9658, PointerRNA * ptr=0x0000008b343fe610, PropertyRNA * prop=0x0000000000000000) Line 2320 C blender.exe!ui_apply_but_funcs_after(bContext * C=0x000001eaddcb9658) Line 946 C blender.exe!ui_region_handler(bContext * C=0x000001eaddcb9658, const wmEvent * event=0x000001ea891380a8, void * UNUSED_userdata=0x0000000000000000) Line 10756 C blender.exe!wm_handler_ui_call(bContext * C=0x000001eaddcb9658, wmEventHandler_UI * handler=0x000001eaf0e2f6f8, const wmEvent * event=0x000001ea891380a8, int always_pass=0) Line 663 C blender.exe!wm_handlers_do_intern(bContext * C=0x000001eaddcb9658, wmEvent * event=0x000001ea891380a8, ListBase * handlers=0x000001eade6f3190) Line 2798 C blender.exe!wm_handlers_do(bContext * C=0x000001eaddcb9658, wmEvent * event=0x000001ea891380a8, ListBase * handlers=0x000001eade6f3190) Line 2914 C blender.exe!wm_event_do_handlers(bContext * C=0x000001eaddcb9658) Line 3410 C blender.exe!WM_main(bContext * C=0x000001eaddcb9658) Line 640 C blender.exe!main(int argc=1, const unsigned char * * UNUSED_argv_c=0x000001eada5e2640) Line 526 C blender.exe!invoke_main() Line 79 C++ blender.exe!__scrt_common_main_seh() Line 288 C++ blender.exe!__scrt_common_main() Line 331 C++ blender.exe!mainCRTStartup() Line 17 C++ kernel32.dll!00007ff953f67c24() Unknown ntdll.dll!00007ff95514d4d1() Unknown ```

Philipp Oeser commented

2021-02-22 13:45:17 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2021-02-22 13:45:17 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Philipp Oeser commented

2021-02-22 13:45:17 +01:00

Can confirm.

Think keyframing is supported?
It works correctly when hitting {key I} over the Pos button -- it does so both in the Node Editor as well as the Node representation in the Properties Editor.
Looks like a problem with template_node_view just for the animation decorator

Can confirm. Think keyframing is supported? It works correctly when hitting {key I} over the `Pos` button -- it does so both in the Node Editor as well as the Node representation in the Properties Editor. Looks like a problem with `template_node_view` just for the animation decorator

Philipp Oeser self-assigned this 2021-02-22 13:45:37 +01:00

Philipp Oeser commented

2021-02-22 13:45:37 +01:00

Will check in a bit.

Philipp Oeser removed their assignment 2021-02-23 16:30:31 +01:00

Philipp Oeser commented

2021-02-23 16:30:31 +01:00

Added subscriber: @JulianEisel

Philipp Oeser commented

2021-02-23 16:30:31 +01:00

Looked into this a bit, but it turns out to be a bit hairy:

First things first, it turns out that (when using the animation decorator), the defined callback colorband_update_cb is never called.
The reason for this is still simple: we are defining it on the wrong button :)
We are getting the last button from the uiBlock (block->buttons.last), but this is then the decorator, not the Position button.

Usually (for most of the other buttons) this is not so much of a problem, but the colorband callback colorband_update_cb manipulates the button data (because of the sorting of colorband elements -- if you drag element 0 past element one, it automatically becomes 1)

bt->rnapoin.data = coba->data + coba->cur

When done from the Properties Editor (when the wrong button is in play) this automatic index update is already not working for said reasons.
Crash then happens because the buttons rnapoin & rnaprop are garbled -- we should not mess with the uiButDecorator's data it seems (havent checked in depth though).

What I tried:

- use the pure RNA update (but from RNA, the ColorRampElement does not have a reference to its ColorRamp, so we cant do the sorting then):

UI_but_funcN_set(bt, rna_update_cb, MEM_dupallocN(cb), NULL);

this doesnt rely on manipulating the button data, gets rid of the crash, but like I said, we are loosing the ability to sort the index (which is needed)

- get the right button like so:
  P1983: T85870_snippet



diff --git a/source/blender/editors/interface/interface_anim.c b/source/blender/editors/interface/interface_anim.c
index 3101b3222c4..24c89029db5 100644
--- a/source/blender/editors/interface/interface_anim.c
+++ b/source/blender/editors/interface/interface_anim.c
@@ -120,7 +120,7 @@ void ui_but_anim_flag(uiBut *but, const AnimationEvalContext *anim_eval_context)
   }
 }
 
-static uiBut *ui_but_anim_decorate_find_attached_button(uiButDecorator *but_decorate)
+uiBut *ui_but_anim_decorate_find_attached_button(uiButDecorator *but_decorate)
 {
   uiBut *but_iter = NULL;
 
diff --git a/source/blender/editors/interface/interface_intern.h b/source/blender/editors/interface/interface_intern.h
index 3da66d45abd..8b19133fd6a 100644
--- a/source/blender/editors/interface/interface_intern.h
+++ b/source/blender/editors/interface/interface_intern.h
@@ -1071,6 +1071,7 @@ bool ui_but_anim_expression_get(uiBut *but, char *str, size_t maxlen);
 bool ui_but_anim_expression_set(uiBut *but, const char *str);
 bool ui_but_anim_expression_create(uiBut *but, const char *str);
 void ui_but_anim_autokey(struct bContext *C, uiBut *but, struct Scene *scene, float cfra);
+uiBut *ui_but_anim_decorate_find_attached_button(struct uiButDecorator *but_decorate);
 
 void ui_but_anim_decorate_cb(struct bContext *C, void *arg_but, void *arg_dummy);
 void ui_but_anim_decorate_update_from_flag(uiButDecorator *but);
diff --git a/source/blender/editors/interface/interface_templates.c b/source/blender/editors/interface/interface_templates.c
index 67446ca681f..05683668322 100644
--- a/source/blender/editors/interface/interface_templates.c
+++ b/source/blender/editors/interface/interface_templates.c
@@ -3334,7 +3334,10 @@ static void colorband_buttons_layout(uiLayout *layout,
 
       row = uiLayoutRow(split, false);
       uiItemR(row, &ptr, "position", 0, IFACE_("Pos"), ICON_NONE);
-      bt = block->buttons.last;
+      bt = ui_but_anim_decorate_find_attached_button((uiButDecorator *)block->buttons.last);
+      if (!bt) {
+        bt = block->buttons.last;
+      }
       UI_but_func_set(bt, colorband_update_cb, bt, coba);
 
       row = uiLayoutRow(layout, false);
@@ -3365,7 +3368,10 @@ static void colorband_buttons_layout(uiLayout *layout,
 
       row = uiLayoutRow(subsplit, false);
       uiItemR(row, &ptr, "position", UI_ITEM_R_SLIDER, IFACE_("Pos"), ICON_NONE);
-      bt = block->buttons.last;
+      bt = ui_but_anim_decorate_find_attached_button((uiButDecorator *)block->buttons.last);
+      if (!bt) {
+        bt = block->buttons.last;
+      }
       UI_but_func_set(bt, colorband_update_cb, bt, coba);
 
       row = uiLayoutRow(split, false);

This gets rid of the crash, works in both Properties Editor and Node Editor [albeit the position slider is now a bit jumpy for some reason -- looses the connection to the drag sometimes?]
This might also have to be ifdef behind UI_PROP_DECORATE, but before spending more time, I would like to summon @JulianEisel to check if there is a better way to do this...
Until then, I will step down to not block others looking at this.

Looked into this a bit, but it turns out to be a bit hairy: First things first, it turns out that (when using the animation decorator), the defined callback `colorband_update_cb` is never called. The reason for this is still simple: we are defining it on the wrong button :) We are getting the last button from the uiBlock (`block->buttons.last`), but this is then the decorator, not the `Position` button. Usually (for most of the other buttons) this is not so much of a problem, but the colorband callback `colorband_update_cb` manipulates the button data (because of the sorting of colorband elements -- if you drag element 0 past element one, it automatically becomes 1) ``` bt->rnapoin.data = coba->data + coba->cur ``` When done from the Properties Editor (when the wrong button is in play) this automatic index update is already not working for said reasons. Crash then happens because the buttons rnapoin & rnaprop are garbled -- we should not mess with the uiButDecorator's data it seems (havent checked in depth though). What I tried: - - [x] use the pure RNA update (but from RNA, the `ColorRampElement` does not have a reference to its ColorRamp, so we cant do the sorting then): ``` UI_but_funcN_set(bt, rna_update_cb, MEM_dupallocN(cb), NULL); ``` this doesnt rely on manipulating the button data, gets rid of the crash, but like I said, we are loosing the ability to sort the index (which is needed) - - [x] get the right button like so: [P1983: T85870_snippet](https://archive.blender.org/developer/P1983.txt) ``` diff --git a/source/blender/editors/interface/interface_anim.c b/source/blender/editors/interface/interface_anim.c index 3101b3222c4..24c89029db5 100644 --- a/source/blender/editors/interface/interface_anim.c +++ b/source/blender/editors/interface/interface_anim.c @@ -120,7 +120,7 @@ void ui_but_anim_flag(uiBut *but, const AnimationEvalContext *anim_eval_context) } } -static uiBut *ui_but_anim_decorate_find_attached_button(uiButDecorator *but_decorate) +uiBut *ui_but_anim_decorate_find_attached_button(uiButDecorator *but_decorate) { uiBut *but_iter = NULL; diff --git a/source/blender/editors/interface/interface_intern.h b/source/blender/editors/interface/interface_intern.h index 3da66d45abd..8b19133fd6a 100644 --- a/source/blender/editors/interface/interface_intern.h +++ b/source/blender/editors/interface/interface_intern.h @@ -1071,6 +1071,7 @@ bool ui_but_anim_expression_get(uiBut *but, char *str, size_t maxlen); bool ui_but_anim_expression_set(uiBut *but, const char *str); bool ui_but_anim_expression_create(uiBut *but, const char *str); void ui_but_anim_autokey(struct bContext *C, uiBut *but, struct Scene *scene, float cfra); +uiBut *ui_but_anim_decorate_find_attached_button(struct uiButDecorator *but_decorate); void ui_but_anim_decorate_cb(struct bContext *C, void *arg_but, void *arg_dummy); void ui_but_anim_decorate_update_from_flag(uiButDecorator *but); diff --git a/source/blender/editors/interface/interface_templates.c b/source/blender/editors/interface/interface_templates.c index 67446ca681f..05683668322 100644 --- a/source/blender/editors/interface/interface_templates.c +++ b/source/blender/editors/interface/interface_templates.c @@ -3334,7 +3334,10 @@ static void colorband_buttons_layout(uiLayout *layout, row = uiLayoutRow(split, false); uiItemR(row, &ptr, "position", 0, IFACE_("Pos"), ICON_NONE); - bt = block->buttons.last; + bt = ui_but_anim_decorate_find_attached_button((uiButDecorator *)block->buttons.last); + if (!bt) { + bt = block->buttons.last; + } UI_but_func_set(bt, colorband_update_cb, bt, coba); row = uiLayoutRow(layout, false); @@ -3365,7 +3368,10 @@ static void colorband_buttons_layout(uiLayout *layout, row = uiLayoutRow(subsplit, false); uiItemR(row, &ptr, "position", UI_ITEM_R_SLIDER, IFACE_("Pos"), ICON_NONE); - bt = block->buttons.last; + bt = ui_but_anim_decorate_find_attached_button((uiButDecorator *)block->buttons.last); + if (!bt) { + bt = block->buttons.last; + } UI_but_func_set(bt, colorband_update_cb, bt, coba); row = uiLayoutRow(split, false); ``` This gets rid of the crash, works in both Properties Editor and Node Editor [albeit the position slider is now a bit jumpy for some reason -- looses the connection to the drag sometimes?] This might also have to be ifdef behind `UI_PROP_DECORATE`, but before spending more time, I would like to summon @JulianEisel to check if there is a better way to do this... Until then, I will step down to not block others looking at this.

Philipp Oeser commented

2021-09-13 10:59:22 +02:00

Added subscribers: @Raytirat, @Siddarth-Calidas, @EAW

Julian Eisel commented

2021-11-26 18:53:23 +01:00

It may be safer here to manually place the decorator button after getting the button via block->buttons.last, instead of letting uiItemR() do it. This can be done easily with uiItemDecoratorR() after disabling automatic decorators with uiLayoutSetPropDecorate()

It may be safer here to manually place the decorator button after getting the button via `block->buttons.last`, instead of letting `uiItemR()` do it. This can be done easily with `uiItemDecoratorR()` after disabling automatic decorators with `uiLayoutSetPropDecorate()`

Julian Eisel commented

2021-11-26 18:54:26 +01:00

Would be nice to have this included in corrective releases at least.

Sybren A. Stüvel commented

2022-03-07 15:08:48 +01:00

Added subscriber: @dr.sybren

Sybren A. Stüvel commented

2022-03-07 15:08:48 +01:00

In #85870#1116798, @CharlieJolly wrote:
Confirmed. I believe keyframing POS shouldn't be allowed as the data structure for the items is not ordered to allow this. However, here is the call stack from Windows debug build if it helps. Seems that there is no check for NULL property in the rna_property_update function.

Keyframing POS?

Update: ah, keyframing the "Pos" property. POS also is an abbreviation for 💩 ;-)

> In #85870#1116798, @CharlieJolly wrote: > Confirmed. I believe keyframing POS shouldn't be allowed as the data structure for the items is not ordered to allow this. However, here is the call stack from Windows debug build if it helps. Seems that there is no check for NULL property in the rna_property_update function. Keyframing POS? Update: ah, keyframing the "Pos" property. POS also is an abbreviation for 💩 ;-)

Philipp Oeser commented

2022-04-22 15:11:05 +02:00

Added subscriber: @maylog

jing commented

2022-05-14 09:30:44 +02:00

Added subscriber: @jinglepp

jing commented

2022-05-14 09:33:13 +02:00

Removed subscriber: @jinglepp

Philipp Oeser commented

2022-09-15 09:55:19 +02:00

Added subscribers: @Dobro, @HooglyBoogly, @Vyach, @Didier-Merette-Dufresne, @TheStegosaurus, @PratikPB2123, @WTF1TC, @filedescriptor

Philipp Oeser commented

2022-09-15 09:56:29 +02:00

In #86057#1417240, @HooglyBoogly wrote:
Since this is a crash in a not-so-niche situation, think it makes sense to raise the priority.

So will raise prio here as well

> In #86057#1417240, @HooglyBoogly wrote: > Since this is a crash in a not-so-niche situation, think it makes sense to raise the priority. So will raise prio here as well

Sybren A. Stüvel changed title from ~~ColorRamp Keyframes crash Blender.~~ to ColorRamp Keyframes crash Blender

2022-09-20 16:25:13 +02:00

John commented

2022-10-08 14:55:31 +02:00

Added subscriber: @dougcal

Pratik Borhade commented

2022-10-10 05:36:46 +02:00

Added subscribers: @Jake-B, @Oxicid

Pratik Borhade commented

2022-11-02 00:40:49 +01:00

Added subscriber: @meta_raketa

blender-admin commented

2022-11-08 12:14:31 +01:00

This issue was referenced by 0e1877b754

This issue was referenced by 0e1877b7542d9cbe5429577c4b0980a9c8c4590f

blender-admin commented

2022-11-08 12:14:31 +01:00

This issue was referenced by 19a13a8b8b

This issue was referenced by 19a13a8b8b80a458322e7895672658f453e72c4f

blender-admin commented

2022-11-08 12:14:31 +01:00

This issue was referenced by 82ba205646

This issue was referenced by 82ba2056462310b103ad16fba726340886e5b0b7

Julian Eisel commented

2022-11-08 12:18:41 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Julian Eisel closed this issue

2022-11-08 12:18:41 +01:00

Julian Eisel self-assigned this 2022-11-08 12:18:41 +01:00

Julian Eisel commented

2022-11-08 12:20:44 +01:00

Thanks @lichtwerk, your investigation was quite useful and saved me the time to investigate things myself. Code shouldn't assume uiItemR() only adds a single button, even regardless of decorators this can go wrong. So my fix entirely removes that assumption.

Thanks @lichtwerk, your investigation was quite useful and saved me the time to investigate things myself. Code shouldn't assume `uiItemR()` only adds a single button, even regardless of decorators this can go wrong. So my fix entirely removes that assumption.

Philipp Oeser commented

2022-11-08 13:18:30 +01:00

thx for the fix!

Christoph Lendenfeld referenced this issue

2023-02-10 11:56:22 +01:00

Blender LTS: Maintenance Task 3.3 #100749

Falk David referenced this issue

2023-02-10 13:03:22 +01:00

Blender LTS: Maintenance Task 2.93 #88449

Sign in to join this conversation.