Merge #6755

6755: Remove individual RGBA sliders from editor color picker, add shift+rightclick/leftclick to copy/paste color in editor r=Jupeyy a=Robyt3

Remove the individual RGBA sliders for editor color pickers and only show one button that opens the color picker popup instead.

Decrease size of layer and point popups that previously had color properties which need less space now.

Support shift-rightclicking color picker buttons to copy the color to the clipboard in RRGGBBAA hex format.

Support shift-leftclicking color picker buttons to paste a color from the clipboard in RGB, RGBA, RRGGBB or RRGGBBAA format with optional leading `#` or `$`.

Screenshots:
- Before:
![screenshot_2023-06-20_20-39-43](https://github.com/ddnet/ddnet/assets/23437060/4a8b230c-a66b-4e2a-9744-cb5f80f6a799)
- After:
![screenshot_2023-06-20_20-40-00](https://github.com/ddnet/ddnet/assets/23437060/517f90cf-06a5-4332-9eb8-6a87cbc91b32)
- After (color picker open):
![screenshot_2023-06-25_17-35-55](https://github.com/ddnet/ddnet/assets/23437060/30320b56-e4cb-4e93-bf7b-8cfc8d96620b)


Suggested by `@HiRavie` in https://github.com/ddnet/ddnet/pull/6743#issuecomment-1593886873, though it was easier and also looks better to me when the color picker button has exactly the same size as the other value selectors.

## Checklist

- [X] Tested the change ingame
- [X] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>

CMakeLists.txt

@@ -1864,6 +1864,7 @@ add_custom_command(OUTPUT "src/game/generated/wordlist.h"
 set_src(BASE GLOB_RECURSE src/base
   bezier.cpp
   bezier.h
+  color.cpp
   color.h
   detect.h
   dynamic.h

src/base/color.cpp

@@ -0,0 +1,48 @@
+#include "color.h"
+#include "system.h"
+
+template<typename T>
+std::optional<T> color_parse(const char *pStr)
+{
+	if(!str_isallnum_hex(pStr))
+		return {};
+
+	const unsigned Num = str_toulong_base(pStr, 16);
+
+	T Color;
+	switch(str_length(pStr))
+	{
+	case 3:
+		Color.x = (((Num >> 8) & 0x0F) + ((Num >> 4) & 0xF0)) / 255.0f;
+		Color.y = (((Num >> 4) & 0x0F) + ((Num >> 0) & 0xF0)) / 255.0f;
+		Color.z = (((Num >> 0) & 0x0F) + ((Num << 4) & 0xF0)) / 255.0f;
+		Color.a = 1.0f;
+		return Color;
+
+	case 4:
+		Color.x = (((Num >> 12) & 0x0F) + ((Num >> 8) & 0xF0)) / 255.0f;
+		Color.y = (((Num >> 8) & 0x0F) + ((Num >> 4) & 0xF0)) / 255.0f;
+		Color.z = (((Num >> 4) & 0x0F) + ((Num >> 0) & 0xF0)) / 255.0f;
+		Color.a = (((Num >> 0) & 0x0F) + ((Num << 4) & 0xF0)) / 255.0f;
+		return Color;
+
+	case 6:
+		Color.x = ((Num >> 16) & 0xFF) / 255.0f;
+		Color.y = ((Num >> 8) & 0xFF) / 255.0f;
+		Color.z = ((Num >> 0) & 0xFF) / 255.0f;
+		Color.a = 1.0f;
+		return Color;
+
+	case 8:
+		Color.x = ((Num >> 24) & 0xFF) / 255.0f;
+		Color.y = ((Num >> 16) & 0xFF) / 255.0f;
+		Color.z = ((Num >> 8) & 0xFF) / 255.0f;
+		Color.a = ((Num >> 0) & 0xFF) / 255.0f;
+		return Color;
+
+	default:
+		return {};
+	}
+}
+
+template std::optional<ColorRGBA> color_parse<ColorRGBA>(const char *);

src/base/color.h

@@ -5,6 +5,8 @@
 #include <base/math.h>
 #include <base/vmath.h>
 
+#include <optional>
+
 /*
 	Title: Color handling
 */
@@ -114,12 +116,40 @@ public:
 		return (Alpha ? ((unsigned)round_to_int(a * 255.0f) << 24) : 0) + ((unsigned)round_to_int(x * 255.0f) << 16) + ((unsigned)round_to_int(y * 255.0f) << 8) + (unsigned)round_to_int(z * 255.0f);
 	}
 
+	unsigned PackAlphaLast(bool Alpha = true) const
+	{
+		if(Alpha)
+			return ((unsigned)round_to_int(x * 255.0f) << 24) + ((unsigned)round_to_int(y * 255.0f) << 16) + ((unsigned)round_to_int(z * 255.0f) << 8) + (unsigned)round_to_int(a * 255.0f);
+		return ((unsigned)round_to_int(x * 255.0f) << 16) + ((unsigned)round_to_int(y * 255.0f) << 8) + (unsigned)round_to_int(z * 255.0f);
+	}
+
 	DerivedT WithAlpha(float alpha) const
 	{
 		DerivedT col(static_cast<const DerivedT &>(*this));
 		col.a = alpha;
 		return col;
 	}
+
+	template<typename UnpackT>
+	static UnpackT UnpackAlphaLast(unsigned Color, bool Alpha = true)
+	{
+		UnpackT Result;
+		if(Alpha)
+		{
+			Result.x = ((Color >> 24) & 0xFF) / 255.0f;
+			Result.y = ((Color >> 16) & 0xFF) / 255.0f;
+			Result.z = ((Color >> 8) & 0xFF) / 255.0f;
+			Result.a = ((Color >> 0) & 0xFF) / 255.0f;
+		}
+		else
+		{
+			Result.x = ((Color >> 16) & 0xFF) / 255.0f;
+			Result.y = ((Color >> 8) & 0xFF) / 255.0f;
+			Result.z = ((Color >> 0) & 0xFF) / 255.0f;
+			Result.a = 1.0f;
+		}
+		return Result;
+	}
 };
 
 class ColorHSLA : public color4_base<ColorHSLA>
@@ -262,4 +292,7 @@ T color_invert(const T &col)
 	return T(1.0f - col.x, 1.0f - col.y, 1.0f - col.z, 1.0f - col.a);
 }
 
+template<typename T>
+std::optional<T> color_parse(const char *pStr);
+
 #endif

src/base/system.cpp

@@ -3463,6 +3463,17 @@ int str_isallnum(const char *str)
 	return 1;
 }
 
+int str_isallnum_hex(const char *str)
+{
+	while(*str)
+	{
+		if(!(*str >= '0' && *str <= '9') && !(*str >= 'a' && *str <= 'f') && !(*str >= 'A' && *str <= 'F'))
+			return 0;
+		str++;
+	}
+	return 1;
+}
+
 int str_toint(const char *str)
 {
 	return str_toint_base(str, 10);

src/base/system.h

@@ -2219,7 +2219,11 @@ float str_tofloat(const char *str);
 int str_isspace(char c);
 
 char str_uppercase(char c);
+
 int str_isallnum(const char *str);
+
+int str_isallnum_hex(const char *str);
+
 unsigned str_quickhash(const char *str);
 
 enum

src/engine/shared/console.cpp

@@ -55,44 +55,7 @@ ColorHSLA CConsole::CResult::GetColor(unsigned Index, bool Light) const
 	}
 	else if(*pStr == '$') // Hex RGB/RGBA
 	{
-		ColorRGBA Rgba = ColorRGBA(0, 0, 0, 1);
+		return color_cast<ColorHSLA>(color_parse<ColorRGBA>(pStr + 1).value_or(ColorRGBA(0.0f, 0.0f, 0.0f, 1.0f)));
-		const int Len = str_length(pStr);
-		if(Len == 4)
-		{
-			const unsigned Num = str_toulong_base(pStr + 1, 16);
-			Rgba.r = (((Num >> 8) & 0x0F) + ((Num >> 4) & 0xF0)) / 255.0f;
-			Rgba.g = (((Num >> 4) & 0x0F) + ((Num >> 0) & 0xF0)) / 255.0f;
-			Rgba.b = (((Num >> 0) & 0x0F) + ((Num << 4) & 0xF0)) / 255.0f;
-		}
-		else if(Len == 5)
-		{
-			const unsigned Num = str_toulong_base(pStr + 1, 16);
-			Rgba.r = (((Num >> 12) & 0x0F) + ((Num >> 8) & 0xF0)) / 255.0f;
-			Rgba.g = (((Num >> 8) & 0x0F) + ((Num >> 4) & 0xF0)) / 255.0f;
-			Rgba.b = (((Num >> 4) & 0x0F) + ((Num >> 0) & 0xF0)) / 255.0f;
-			Rgba.a = (((Num >> 0) & 0x0F) + ((Num << 4) & 0xF0)) / 255.0f;
-		}
-		else if(Len == 7)
-		{
-			const unsigned Num = str_toulong_base(pStr + 1, 16);
-			Rgba.r = ((Num >> 16) & 0xFF) / 255.0f;
-			Rgba.g = ((Num >> 8) & 0xFF) / 255.0f;
-			Rgba.b = ((Num >> 0) & 0xFF) / 255.0f;
-		}
-		else if(Len == 9)
-		{
-			const unsigned Num = str_toulong_base(pStr + 1, 16);
-			Rgba.r = ((Num >> 24) & 0xFF) / 255.0f;
-			Rgba.g = ((Num >> 16) & 0xFF) / 255.0f;
-			Rgba.b = ((Num >> 8) & 0xFF) / 255.0f;
-			Rgba.a = ((Num >> 0) & 0xFF) / 255.0f;
-		}
-		else
-		{
-			return ColorHSLA(0, 0, 0);
-		}
-
-		return color_cast<ColorHSLA>(Rgba);
 	}
 	else if(!str_comp_nocase(pStr, "red"))
 		return ColorHSLA(0.0f / 6.0f, 1, .5f);

src/game/client/ui.cpp

@@ -1711,32 +1711,15 @@ CUI::EPopupMenuFunctionResult CUI::PopupColorPicker(void *pContext, CUIRect View
 
 	PickerColorHSV = ColorHSVA(H / 255.0f, S / 255.0f, V / 255.0f, A / 255.0f);
 
-	const auto RotateByteLeft = [pColorPicker](unsigned Num) {
-		if(pColorPicker->m_Alpha)
-		{
-			// ARGB -> RGBA (internal -> displayed)
-			return ((Num & 0xFF000000u) >> 24) | (Num << 8);
-		}
-		return Num;
-	};
-	const auto RotateByteRight = [pColorPicker](unsigned Num) {
-		if(pColorPicker->m_Alpha)
-		{
-			// RGBA -> ARGB (displayed -> internal)
-			return ((Num & 0xFFu) << 24) | (Num >> 8);
-		}
-		return Num;
-	};
-
 	SValueSelectorProperties Props;
 	Props.m_UseScroll = false;
 	Props.m_IsHex = true;
 	Props.m_HexPrefix = pColorPicker->m_Alpha ? 8 : 6;
-	const unsigned Hex = RotateByteLeft(color_cast<ColorRGBA>(PickerColorHSV).Pack(pColorPicker->m_Alpha));
+	const unsigned Hex = color_cast<ColorRGBA>(PickerColorHSV).PackAlphaLast(pColorPicker->m_Alpha);
 	const unsigned NewHex = pUI->DoValueSelector(&pColorPicker->m_aValueSelectorIds[4], &HexRect, "Hex:", Hex, 0, pColorPicker->m_Alpha ? 0xFFFFFFFFll : 0xFFFFFFll, Props);
 	if(Hex != NewHex)
 	{
-		PickerColorHSV = color_cast<ColorHSVA>(ColorRGBA(RotateByteRight(NewHex), pColorPicker->m_Alpha));
+		PickerColorHSV = color_cast<ColorHSVA>(ColorRGBA::UnpackAlphaLast<ColorRGBA>(NewHex, pColorPicker->m_Alpha));
 		if(!pColorPicker->m_Alpha)
 			PickerColorHSV.a = A / 255.0f;
 	}

src/game/editor/editor.cpp

@@ -1738,7 +1738,7 @@ void CEditor::DoQuadPoint(CQuad *pQuad, int QuadIndex, int V)
 					m_SelectedQuadIndex = FindSelectedQuadIndex(QuadIndex);
 
 					static SPopupMenuId s_PopupPointId;
-					UI()->DoPopupMenu(&s_PopupPointId, UI()->MouseX(), UI()->MouseY(), 120, 150, this, PopupPoint);
+					UI()->DoPopupMenu(&s_PopupPointId, UI()->MouseX(), UI()->MouseY(), 120, 75, this, PopupPoint);
 				}
 				UI()->SetActiveItem(nullptr);
 			}
@@ -3183,53 +3183,44 @@ int CEditor::DoProperties(CUIRect *pToolBox, CProperty *pProps, int *pIDs, int *
 		}
 		else if(pProps[i].m_Type == PROPTYPE_COLOR)
 		{
-			static const char *s_apTexts[4] = {"R", "G", "B", "A"};
+			const ColorRGBA ColorPick = ColorRGBA(
-			static const int s_aShift[] = {24, 16, 8, 0};
+				((pProps[i].m_Value >> 24) & 0xff) / 255.0f,
-			int NewColor = 0;
+				((pProps[i].m_Value >> 16) & 0xff) / 255.0f,
+				((pProps[i].m_Value >> 8) & 0xff) / 255.0f,
+				(pProps[i].m_Value & 0xff) / 255.0f);
 
-			// extra space
+			CUIRect ColorRect;
-			CUIRect ColorBox, ColorSlots;
+			Shifter.Draw(ColorRGBA(1.0f, 1.0f, 1.0f, 0.5f * UI()->ButtonColorMul(&pIDs[i])), IGraphics::CORNER_ALL, 5.0f);
-
+			Shifter.Margin(1.0f, &ColorRect);
-			pToolBox->HSplitTop(3.0f * 13.0f, &Slot, pToolBox);
+			ColorRect.Draw(ColorPick, IGraphics::CORNER_ALL, ColorRect.h / 2.0f);
-			Slot.VSplitMid(&ColorBox, &ColorSlots);
-			ColorBox.HSplitTop(8.0f, nullptr, &ColorBox);
-			ColorBox.VMargin(12.0f, &ColorBox);
-			ColorBox.HMargin((ColorBox.h - ColorBox.w) / 2.0f, &ColorBox);
-
-			for(int c = 0; c < 4; c++)
-			{
-				int v = (pProps[i].m_Value >> s_aShift[c]) & 0xff;
-				NewColor |= UiDoValueSelector(((char *)&pIDs[i]) + c, &Shifter, s_apTexts[c], v, 0, 255, 1, 1.0f, "Use left mouse button to drag and change the color value. Hold shift to be more precise. Rightclick to edit as text.") << s_aShift[c];
-
-				if(c != 3)
-				{
-					ColorSlots.HSplitTop(13.0f, &Shifter, &ColorSlots);
-					Shifter.HMargin(1.0f, &Shifter);
-				}
-			}
-
-			// hex
-			pToolBox->HSplitTop(13.0f, &Slot, pToolBox);
-			Slot.VSplitMid(nullptr, &Shifter);
-			Shifter.HMargin(1.0f, &Shifter);
-
-			int NewColorHex = pProps[i].m_Value & 0xff;
-			NewColorHex |= UiDoValueSelector(((char *)&pIDs[i] - 1), &Shifter, "", (pProps[i].m_Value >> 8) & 0xFFFFFF, 0, 0xFFFFFF, 1, 1.0f, "Use left mouse button to drag and change the color value. Hold shift to be more precise. Rightclick to edit as text.", false, true) << 8;
-
-			// color picker
-			ColorRGBA ColorPick = ColorRGBA(
-				((pProps[i].m_Value >> s_aShift[0]) & 0xff) / 255.0f,
-				((pProps[i].m_Value >> s_aShift[1]) & 0xff) / 255.0f,
-				((pProps[i].m_Value >> s_aShift[2]) & 0xff) / 255.0f,
-				1.0f);
-
-			static int s_ColorPicker;
-			if(UI()->HotItem() == &s_ColorPicker)
-				ColorBox.Margin(-2.0f, &ColorBox);
-			ColorBox.Draw(ColorPick, IGraphics::CORNER_ALL, 3.0f);
 
 			static CUI::SColorPickerPopupContext s_ColorPickerPopupContext;
-			if(DoButton_Editor_Common(&s_ColorPicker, nullptr, 0, &ColorBox, 0, "Click to show the color picker."))
+			const int ButtonResult = DoButton_Editor_Common(&pIDs[i], nullptr, 0, &Shifter, 0, "Click to show the color picker. Shift+rightclick to copy color to clipboard. Shift+leftclick to paste color from clipboard.");
+			if(Input()->ShiftIsPressed())
+			{
+				if(ButtonResult == 1)
+				{
+					const char *pClipboard = Input()->GetClipboardText();
+					if(*pClipboard == '#' || *pClipboard == '$') // ignore leading # (web color format) and $ (console color format)
+						++pClipboard;
+					if(str_isallnum_hex(pClipboard))
+					{
+						std::optional<ColorRGBA> ParsedColor = color_parse<ColorRGBA>(pClipboard);
+						if(ParsedColor)
+						{
+							*pNewVal = ParsedColor.value().PackAlphaLast();
+							Change = i;
+						}
+					}
+				}
+				else if(ButtonResult == 2)
+				{
+					char aClipboard[9];
+					str_format(aClipboard, sizeof(aClipboard), "%08X", ColorPick.PackAlphaLast());
+					Input()->SetClipboardText(aClipboard);
+				}
+			}
+			else if(ButtonResult > 0)
 			{
 				s_ColorPickerPopupContext.m_HsvaColor = color_cast<ColorHSVA>(ColorPick);
 				s_ColorPickerPopupContext.m_Alpha = true;
@@ -3238,18 +3229,12 @@ int CEditor::DoProperties(CUIRect *pToolBox, CProperty *pProps, int *pIDs, int *
 			else if(UI()->IsPopupOpen(&s_ColorPickerPopupContext))
 			{
 				ColorRGBA c = color_cast<ColorRGBA>(s_ColorPickerPopupContext.m_HsvaColor);
-				NewColor = ((int)(c.r * 255.0f) & 0xff) << 24 | ((int)(c.g * 255.0f) & 0xff) << 16 | ((int)(c.b * 255.0f) & 0xff) << 8 | (pProps[i].m_Value & 0xff);
+				const int NewColor = ((int)(c.r * 255.0f) & 0xff) << 24 | ((int)(c.g * 255.0f) & 0xff) << 16 | ((int)(c.b * 255.0f) & 0xff) << 8 | ((int)(c.a * 255.0f) & 0xff);
-			}
-
 				if(NewColor != pProps[i].m_Value)
 				{
 					*pNewVal = NewColor;
 					Change = i;
 				}
-			else if(NewColorHex != pProps[i].m_Value)
-			{
-				*pNewVal = NewColorHex;
-				Change = i;
 			}
 		}
 		else if(pProps[i].m_Type == PROPTYPE_IMAGE)
@@ -3759,7 +3744,7 @@ void CEditor::RenderLayers(CUIRect LayersBox)
 						else
 							s_LayerPopupContext.m_vpLayers.clear();
 
-						UI()->DoPopupMenu(&s_LayerPopupContext, UI()->MouseX(), UI()->MouseY(), 120, 320, &s_LayerPopupContext, PopupLayer);
+						UI()->DoPopupMenu(&s_LayerPopupContext, UI()->MouseX(), UI()->MouseY(), 120, 230, &s_LayerPopupContext, PopupLayer);
 					}
 
 					s_Operation = OP_NONE;