Color picker widget in 5.8 closes unnecessarily on property changes

Development Pipeline & Plugins
, ,
1

Epic changelist 52639755 (tagged “EditorUsability : LiveEdit”) added code in `FColorStructCustomization::CreateColorPicker()` that closes a color picker when other properties on the same Blueprint actor are changed. But it’s overly broad, and closes the picker in situations where the picker would have continued to work fine.

Specifically, what happens is: if another property on the same Blueprint actor receives an edit while a color picker is open, the picker is closed immediately to avoid the risk of leaving the picker outliving its property handle. That’s important for multi-user remote editing situations (e.g. UEFN Live Edit) where a committed property edit would replace the Blueprint actor, rebuilding the Details panel, and orphaning the picker so it would continue to be active but not connected to any property.

Note that this is a UX fix rather than a safety fix. If the property handle does become invalid, changing the color becomes a no-op: frustrating for the user, because to them it just looks like the picker has stopped working for no reason, but it doesn’t crash or corrupt anything.

The problem is that it also triggers on local property edits that don’t replace the Blueprint actor and where the color picker would have continued to work fine.

A minimal repro that demonstrates the problem is a Blueprint actor with an instance-editable color variable, a DynamicMeshComponent, and a construction script that just calls `GetDynamicMesh()->Reset()` on that component. (That’s in the attached project.) Using the color picker to modify the color variable on an instance of such an actor leads to:

AActor::PostEditChangeProperty

RerunConstructionScripts

UDynamicMesh::Reset

UDynamicMeshComponent::BroadcastMeshPropertyChangeEvent

That is a non-Interactive OnObjectPropertyChanged event for a different property on the same Blueprint actor, so it passes the tests in the new code, and the picker closes as soon as the color value is changed. But `RerunConstructionScripts` didn’t replace the actor, and the picker would have continued to work fine.

For us, that is a worse UX problem than the one the code is addressing, so we #ifdef it out. I’m wondering if a better fix would be to detect when the property handle actually does become invalid and close the picker reactively then instead of trying to do it in advance?

[Attachment Removed]

2

Steps to Reproduce
1) Open the attached project

2) Select the ColorPickerTest actor in the ColorPickerTestLevel

3) In the Details panel, click the Test Color value to open the color picker

4) Click one of the controls in the color picker to change the color

5) Observe that the picker closed

[Attachment Removed]

3

Thanks for reporting this! I’ve forwarded your feedback to the dev who submitted that CL. Either of us will respond once we have an update on whether and how we’ll address this.

[Attachment Removed]

4

Hey Jon, I have a small update:

  • I created UE-389761 so the status on your bug report can be publicly tracked.
  • The developer who submitted CL 52639755 is sorting his EPS access and I expect him to chime in later this week.
    [Attachment Removed]
5

Hello,

I hope you dont mind me chipping in but I agree with the fact that this is a high priority issue because we have also experienced these crashes. We can pick the CL with the fix and build ourselves but many teams won’t. If the whole 5.8 live contains this crash that will be very detrimental for many teams.

Thank you.

[Attachment Removed]

6

Hey everyone. Daniel and Jose, thanks for bringing the blueprint-related crash to our attention. Based on it, we escalated getting the color picker-related hotfixes into UE 5.8.x. Both issues, the picker closing prematurely and causing a crash when confirming a color value change on blueprints, I can confirm are addressed with //UE5/Main CL 57125337 a.ka. GitHub commit cf689c86dc2efeac93e9a898dc66d3f125a1971f.

UE 5.8.2 hotfix released a few days ago. The fix is not in UE 5.8.2, but it will roll out as part of 5.8.3 later (5.8.3 is in the works).

Thanks again for flagging this. Indeed, we wouldn’t want that crash to be part of the final 5.8.x version.

Edit: Also, apologies for my slow response time. In this case it didn’t impact the developer’s reaction time since he acted on your reports on the day of. I waited to respond until I had time to personally confirm the crash fix.

[Attachment Removed]

7

Rad, thank you!

[Attachment Removed]

8

Hello again, after talking to the dev who committed CL 52639755 that you called out, he went and fixed the color picking auto-closing. At least, it addresses the minimap repro you provided where previously the color picker auto-closes on construction script run. It will still close if you recompile the blueprint.

The fix CL is 56908205. The bug isn’t considered high enough priority to hotfix into UE 5.8.x, so the fix lives on the recently created //UE6/Main branch, but I believe you should be able to inspect the CL and manually backport if you want.

I’ve attached a video that the dev made of the new behavior. Does this address the issues you had?

[Attachment Removed]

9

The change that caused the extraneous color picker closes also caused another issue - if you have a blueprint light of any kind, place it in a level, and try to change the light color, the editor crashes.

I couldn’t find any EPS post about this issue, but it’s been reported on the public forums here: [UE5.8] Changing Light Color on a Blueprint subclass of a LightActor crashes the Editor

I would suggest you DO hotfix this for 5.8, because for people who don’t build their own editor, blueprinted lights are quite crashy at the moment.

Also, the underlying reason for the crash is that DestroyColorPicker() is not reentrant. The old code caused a stack overflow because it endlessly tried to destroy the color picker, then react to the picker being destroyed but updating some properties, and the mentioned code then tried to destroy the color picker in response to that again.

For reference, this is what the callstack looks like when that happens:

 	UnrealEditor-AppFramework-Win64-Debug.dll!DestroyColorPicker() Line 1602	C++
 	UnrealEditor-DetailCustomizations-Win64-Debug.dll!FColorStructCustomization::CreateColorPicker::__l9::<lambda_1>::operator()(UObject * InObject, FPropertyChangedEvent & InEvent) Line 329	C++
 	UnrealEditor-DetailCustomizations-Win64-Debug.dll!TBaseSPLambdaDelegateInstance<1,void __cdecl(UObject *,FPropertyChangedEvent &),FDefaultDelegateUserPolicy,`FColorStructCustomization::CreateColorPicker'::`9'::<lambda_1>>::ExecuteIfSafe(UObject * <Params_0>, FPropertyChangedEvent & <Params_1>) Line 376	C++
 	UnrealEditor-CoreUObject-Win64-Debug.dll!TMulticastDelegateBase<FDefaultDelegateUserPolicy>::Broadcast<IBaseDelegateInstance<void __cdecl(UObject *,FPropertyChangedEvent &),FDefaultDelegateUserPolicy>,UObject *,FPropertyChangedEvent &>(UObject * <Params_0>, FPropertyChangedEvent & <Params_1>) Line 306	C++
 	UnrealEditor-CoreUObject-Win64-Debug.dll!UObject::PostEditChangeProperty(FPropertyChangedEvent & PropertyChangedEvent) Line 558	C++
 	UnrealEditor-Engine-Win64-Debug.dll!AActor::PostEditChangeProperty(FPropertyChangedEvent & PropertyChangedEvent) Line 264	C++
 	UnrealEditor-CoreUObject-Win64-Debug.dll!UObject::PostEditChangeChainProperty(FPropertyChangedChainEvent & PropertyChangedEvent) Line 683	C++
 	UnrealEditor-PropertyEditor-Win64-Debug.dll!FPropertyNode::NotifyPostChange::__l14::<lambda_1>::operator()(UObject * Object) Line 3448	C++
 	UnrealEditor-PropertyEditor-Win64-Debug.dll!FPropertyNode::NotifyPostChange(FPropertyChangedEvent & InPropertyChangedEvent, FNotifyHook * InNotifyHook) Line 3453	C++
 	UnrealEditor-PropertyEditor-Win64-Debug.dll!FPropertyValueImpl::ImportText(const TArray<FObjectBaseAddress,TSizedDefaultAllocator<32>> & InObjects, const TArray<FString,TSizedDefaultAllocator<32>> & InValues, FPropertyNode * InPropertyNode, unsigned int Flags) Line 724	C++
 	UnrealEditor-PropertyEditor-Win64-Debug.dll!FPropertyValueImpl::ImportText(const FString & InValue, FPropertyNode * InPropertyNode, unsigned int Flags) Line 364	C++
 	UnrealEditor-PropertyEditor-Win64-Debug.dll!FPropertyValueImpl::SetValueAsString(const FString & InValue, unsigned int Flags) Line 1070	C++
 	UnrealEditor-PropertyEditor-Win64-Debug.dll!FPropertyHandleBase::SetValueFromFormattedString(const FString & InValue, unsigned int Flags) Line 2923	C++
 	UnrealEditor-DetailCustomizations-Win64-Debug.dll!FColorStructCustomization::OnColorPickerWindowClosed(const TSharedRef<SWindow,1> & Window) Line 442	C++
 	UnrealEditor-DetailCustomizations-Win64-Debug.dll!TBaseSPMethodDelegateInstance<0,FColorStructCustomization,1,void __cdecl(TSharedRef<SWindow,1> const &),FDefaultDelegateUserPolicy>::Execute(const TSharedRef<SWindow,1> & <Params_0>) Line 266	C++
 	UnrealEditor-AppFramework-Win64-Debug.dll!TDelegate<void __cdecl(TSharedRef<SWindow,1> const &),FDefaultDelegateUserPolicy>::Execute(const TSharedRef<SWindow,1> & <Params_0>) Line 628	C++
 	UnrealEditor-AppFramework-Win64-Debug.dll!SColorPicker::HandleParentWindowClosed(const TSharedRef<SWindow,1> & Window) Line 1411	C++
 	UnrealEditor-AppFramework-Win64-Debug.dll!TBaseSPMethodDelegateInstance<0,SColorPicker,1,void __cdecl(TSharedRef<SWindow,1> const &),FDefaultDelegateUserPolicy>::ExecuteIfSafe(const TSharedRef<SWindow,1> & <Params_0>) Line 283	C++
 	UnrealEditor-SlateCore-Win64-Debug.dll!TDelegate<void __cdecl(TSharedRef<SWindow,1> const &),FDefaultDelegateUserPolicy>::ExecuteIfBound(const TSharedRef<SWindow,1> & <Params_0>) Line 644	C++
 	UnrealEditor-SlateCore-Win64-Debug.dll!SWindow::NotifyWindowBeingDestroyed() Line 1410	C++
 	UnrealEditor-Slate-Win64-Debug.dll!FSlateApplication::PrivateDestroyWindow(const TSharedRef<SWindow,1> & DestroyedWindow) Line 7388	C++
 	UnrealEditor-Slate-Win64-Debug.dll!FSlateApplication::DestroyWindowsImmediately() Line 3286	C++
 	UnrealEditor-Slate-Win64-Debug.dll!FSlateApplication::RequestDestroyWindow(TSharedRef<SWindow,1> InWindowToDestroy) Line 2456	C++
 	UnrealEditor-SlateCore-Win64-Debug.dll!SWindow::RequestDestroyWindow() Line 1393	C++
 	UnrealEditor-AppFramework-Win64-Debug.dll!DestroyColorPicker() Line 1602	C++

Since that underlying issue is still there, I suggest you also add a reentrancy guard in DestroyColorPicker.

Ciao, Daniel!

[Attachment Removed]

10

Hi! Yep, that fix is working great for our use case. Thank you very much (and please pass my thanks on to the dev who submitted the fix).

[Attachment Removed]