ColorEdit: Fix multiple issues. (#4014)

* Change g.ColorEditLastColor type to ImU32 and store RGB color value.
  - Fixes inability to change hue when saturation is 0. (#4014)
  - Fixes edgecases where lossy color conversion prevent restoration of hue/saturation.
  - Fixes hue value jitter when modifying color using SV square.
* Fix hue resetting to 0 when it is set to 255 by explicitly restoring hue if it is 0 and previous value was 1.
* Further reduce hue jitter by restoring hue when color is modified using SV square.
This commit is contained in:
Rokas Kupstys 2021-04-08 14:37:03 +03:00 committed by ocornut
parent 15132217a3
commit 30546bc0e7
4 changed files with 42 additions and 28 deletions

View File

@ -63,6 +63,10 @@ Other Changes:
The only situation where that change would make a meaningful difference is TreePush((const char*)NULL) The only situation where that change would make a meaningful difference is TreePush((const char*)NULL)
(_explicitely_ casting a null pointer to const char*), which is unlikely and will now crash. (_explicitely_ casting a null pointer to const char*), which is unlikely and will now crash.
You may replace it with anything else. You may replace it with anything else.
- ColorEdit4: Fixed not being able to change hue when saturation is 0. (#4014) [@rokups]
- ColorEdit4: Fixed hue resetting to 0 when it is set to 255. [@rokups]
- ColorEdit4: Fixed hue value jitter when source color is stored as RGB in 32-bit integer and perform
RGB<>HSV round trips every frames. [@rokups]
- Menus: Fixed vertical alignments of MenuItem() calls within a menu bar. (broken in 1.84). (#4538) - Menus: Fixed vertical alignments of MenuItem() calls within a menu bar. (broken in 1.84). (#4538)
- Menus: Adjust closing logic to accomodate for varying font size and dpi. - Menus: Adjust closing logic to accomodate for varying font size and dpi.
- Menus: Fixed crash when navigating left inside a child window inside a sub-menu. (#4510). - Menus: Fixed crash when navigating left inside a child window inside a sub-menu. (#4510).

View File

@ -64,7 +64,7 @@ Index of this file:
// Version // Version
// (Integer encoded as XYYZZ for use in #if preprocessor conditionals. Work in progress versions typically starts at XYY99 then bounce up to XYY00, XYY01 etc. when release tagging happens) // (Integer encoded as XYYZZ for use in #if preprocessor conditionals. Work in progress versions typically starts at XYY99 then bounce up to XYY00, XYY01 etc. when release tagging happens)
#define IMGUI_VERSION "1.85 WIP" #define IMGUI_VERSION "1.85 WIP"
#define IMGUI_VERSION_NUM 18416 #define IMGUI_VERSION_NUM 18417
#define IMGUI_CHECKVERSION() ImGui::DebugCheckVersionAndDataLayout(IMGUI_VERSION, sizeof(ImGuiIO), sizeof(ImGuiStyle), sizeof(ImVec2), sizeof(ImVec4), sizeof(ImDrawVert), sizeof(ImDrawIdx)) #define IMGUI_CHECKVERSION() ImGui::DebugCheckVersionAndDataLayout(IMGUI_VERSION, sizeof(ImGuiIO), sizeof(ImGuiStyle), sizeof(ImVec2), sizeof(ImVec4), sizeof(ImDrawVert), sizeof(ImDrawIdx))
#define IMGUI_HAS_TABLE #define IMGUI_HAS_TABLE

View File

@ -1606,9 +1606,9 @@ struct ImGuiContext
ImFont InputTextPasswordFont; ImFont InputTextPasswordFont;
ImGuiID TempInputId; // Temporary text input when CTRL+clicking on a slider, etc. ImGuiID TempInputId; // Temporary text input when CTRL+clicking on a slider, etc.
ImGuiColorEditFlags ColorEditOptions; // Store user options for color edit widgets ImGuiColorEditFlags ColorEditOptions; // Store user options for color edit widgets
float ColorEditLastHue; // Backup of last Hue associated to LastColor[3], so we can restore Hue in lossy RGB<>HSV round trips float ColorEditLastHue; // Backup of last Hue associated to LastColor, so we can restore Hue in lossy RGB<>HSV round trips
float ColorEditLastSat; // Backup of last Saturation associated to LastColor[3], so we can restore Saturation in lossy RGB<>HSV round trips float ColorEditLastSat; // Backup of last Saturation associated to LastColor, so we can restore Saturation in lossy RGB<>HSV round trips
float ColorEditLastColor[3]; ImU32 ColorEditLastColor; // RGB value with alpha set to 0.
ImVec4 ColorPickerRef; // Initial/reference color at the time of opening the color picker. ImVec4 ColorPickerRef; // Initial/reference color at the time of opening the color picker.
ImGuiComboPreviewData ComboPreviewData; ImGuiComboPreviewData ComboPreviewData;
float SliderCurrentAccum; // Accumulated slider delta when using navigation controls. float SliderCurrentAccum; // Accumulated slider delta when using navigation controls.
@ -1777,7 +1777,7 @@ struct ImGuiContext
TempInputId = 0; TempInputId = 0;
ColorEditOptions = ImGuiColorEditFlags_DefaultOptions_; ColorEditOptions = ImGuiColorEditFlags_DefaultOptions_;
ColorEditLastHue = ColorEditLastSat = 0.0f; ColorEditLastHue = ColorEditLastSat = 0.0f;
ColorEditLastColor[0] = ColorEditLastColor[1] = ColorEditLastColor[2] = FLT_MAX; ColorEditLastColor = 0;
SliderCurrentAccum = 0.0f; SliderCurrentAccum = 0.0f;
SliderCurrentAccumDirty = false; SliderCurrentAccumDirty = false;
DragCurrentAccumDirty = false; DragCurrentAccumDirty = false;

View File

@ -4781,6 +4781,30 @@ bool ImGui::ColorEdit3(const char* label, float col[3], ImGuiColorEditFlags flag
return ColorEdit4(label, col, flags | ImGuiColorEditFlags_NoAlpha); return ColorEdit4(label, col, flags | ImGuiColorEditFlags_NoAlpha);
} }
// ColorEdit supports RGB and HSV inputs. In case of RGB input resulting color may have undefined hue and/or saturation.
// Since widget displays both RGB and HSV values we must preserve hue and saturation to prevent these values resetting.
static void ColorEditRestoreHS(const float* col, float* H, float* S, float* V)
{
// This check is optional. Suppose we have two color widgets side by side, both widgets display different colors, but both colors have hue and/or saturation undefined.
// With color check: hue/saturation is preserved in one widget. Editing color in one widget would reset hue/saturation in another one.
// Without color check: common hue/saturation would be displayed in all widgets that have hue/saturation undefined.
// g.ColorEditLastColor is stored as ImU32 RGB value: this essentially gives us color equality check with reduced precision.
// Tiny external color changes would not be detected and this check would still pass. This is OK, since we only restore hue/saturation _only_ if they are undefined,
// therefore this change flipping hue/saturation from undefined to a very tiny value would still be represented in color picker.
ImGuiContext& g = *GImGui;
if (g.ColorEditLastColor != ImGui::ColorConvertFloat4ToU32(ImVec4(col[0], col[1], col[2], 0)))
return;
// When S == 0, H is undefined.
// When H == 1 it wraps around to 0.
if (*S == 0.0f || (*H == 0.0f && g.ColorEditLastHue == 1))
*H = g.ColorEditLastHue;
// When V == 0, S is undefined.
if (*V == 0.0f)
*S = g.ColorEditLastSat;
}
// Edit colors components (each component in 0.0f..1.0f range). // Edit colors components (each component in 0.0f..1.0f range).
// See enum ImGuiColorEditFlags_ for available options. e.g. Only access 3 floats if ImGuiColorEditFlags_NoAlpha flag is set. // See enum ImGuiColorEditFlags_ for available options. e.g. Only access 3 floats if ImGuiColorEditFlags_NoAlpha flag is set.
// With typical options: Left-click on color square to open color picker. Right-click to open option menu. CTRL-Click over input fields to edit them and TAB to go to next item. // With typical options: Left-click on color square to open color picker. Right-click to open option menu. CTRL-Click over input fields to edit them and TAB to go to next item.
@ -4836,13 +4860,7 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag
{ {
// Hue is lost when converting from greyscale rgb (saturation=0). Restore it. // Hue is lost when converting from greyscale rgb (saturation=0). Restore it.
ColorConvertRGBtoHSV(f[0], f[1], f[2], f[0], f[1], f[2]); ColorConvertRGBtoHSV(f[0], f[1], f[2], f[0], f[1], f[2]);
if (memcmp(g.ColorEditLastColor, col, sizeof(float) * 3) == 0) ColorEditRestoreHS(col, &f[0], &f[1], &f[2]);
{
if (f[1] == 0)
f[0] = g.ColorEditLastHue;
if (f[2] == 0)
f[1] = g.ColorEditLastSat;
}
} }
int i[4] = { IM_F32_TO_INT8_UNBOUND(f[0]), IM_F32_TO_INT8_UNBOUND(f[1]), IM_F32_TO_INT8_UNBOUND(f[2]), IM_F32_TO_INT8_UNBOUND(f[3]) }; int i[4] = { IM_F32_TO_INT8_UNBOUND(f[0]), IM_F32_TO_INT8_UNBOUND(f[1]), IM_F32_TO_INT8_UNBOUND(f[2]), IM_F32_TO_INT8_UNBOUND(f[3]) };
@ -4977,7 +4995,7 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag
g.ColorEditLastHue = f[0]; g.ColorEditLastHue = f[0];
g.ColorEditLastSat = f[1]; g.ColorEditLastSat = f[1];
ColorConvertHSVtoRGB(f[0], f[1], f[2], f[0], f[1], f[2]); ColorConvertHSVtoRGB(f[0], f[1], f[2], f[0], f[1], f[2]);
memcpy(g.ColorEditLastColor, f, sizeof(float) * 3); g.ColorEditLastColor = ColorConvertFloat4ToU32(ImVec4(f[0], f[1], f[2], 0));
} }
if ((flags & ImGuiColorEditFlags_DisplayRGB) && (flags & ImGuiColorEditFlags_InputHSV)) if ((flags & ImGuiColorEditFlags_DisplayRGB) && (flags & ImGuiColorEditFlags_InputHSV))
ColorConvertRGBtoHSV(f[0], f[1], f[2], f[0], f[1], f[2]); ColorConvertRGBtoHSV(f[0], f[1], f[2], f[0], f[1], f[2]);
@ -5112,13 +5130,7 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
{ {
// Hue is lost when converting from greyscale rgb (saturation=0). Restore it. // Hue is lost when converting from greyscale rgb (saturation=0). Restore it.
ColorConvertRGBtoHSV(R, G, B, H, S, V); ColorConvertRGBtoHSV(R, G, B, H, S, V);
if (memcmp(g.ColorEditLastColor, col, sizeof(float) * 3) == 0) ColorEditRestoreHS(col, &H, &S, &V);
{
if (S == 0)
H = g.ColorEditLastHue;
if (V == 0)
S = g.ColorEditLastSat;
}
} }
else if (flags & ImGuiColorEditFlags_InputHSV) else if (flags & ImGuiColorEditFlags_InputHSV)
{ {
@ -5171,6 +5183,10 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
{ {
S = ImSaturate((io.MousePos.x - picker_pos.x) / (sv_picker_size - 1)); S = ImSaturate((io.MousePos.x - picker_pos.x) / (sv_picker_size - 1));
V = 1.0f - ImSaturate((io.MousePos.y - picker_pos.y) / (sv_picker_size - 1)); V = 1.0f - ImSaturate((io.MousePos.y - picker_pos.y) / (sv_picker_size - 1));
// Greatly reduces hue jitter and reset to 0 when hue == 255 and color is rapidly modified using SV square.
if (g.ColorEditLastColor == ColorConvertFloat4ToU32(ImVec4(col[0], col[1], col[2], 0)))
H = g.ColorEditLastHue;
value_changed = value_changed_sv = true; value_changed = value_changed_sv = true;
} }
if (!(flags & ImGuiColorEditFlags_NoOptions)) if (!(flags & ImGuiColorEditFlags_NoOptions))
@ -5247,7 +5263,7 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
ColorConvertHSVtoRGB(H >= 1.0f ? H - 10 * 1e-6f : H, S > 0.0f ? S : 10 * 1e-6f, V > 0.0f ? V : 1e-6f, col[0], col[1], col[2]); ColorConvertHSVtoRGB(H >= 1.0f ? H - 10 * 1e-6f : H, S > 0.0f ? S : 10 * 1e-6f, V > 0.0f ? V : 1e-6f, col[0], col[1], col[2]);
g.ColorEditLastHue = H; g.ColorEditLastHue = H;
g.ColorEditLastSat = S; g.ColorEditLastSat = S;
memcpy(g.ColorEditLastColor, col, sizeof(float) * 3); g.ColorEditLastColor = ColorConvertFloat4ToU32(ImVec4(col[0], col[1], col[2], 0));
} }
else if (flags & ImGuiColorEditFlags_InputHSV) else if (flags & ImGuiColorEditFlags_InputHSV)
{ {
@ -5301,13 +5317,7 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
G = col[1]; G = col[1];
B = col[2]; B = col[2];
ColorConvertRGBtoHSV(R, G, B, H, S, V); ColorConvertRGBtoHSV(R, G, B, H, S, V);
if (memcmp(g.ColorEditLastColor, col, sizeof(float) * 3) == 0) // Fix local Hue as display below will use it immediately. ColorEditRestoreHS(col, &H, &S, &V); // Fix local Hue as display below will use it immediately.
{
if (S == 0)
H = g.ColorEditLastHue;
if (V == 0)
S = g.ColorEditLastSat;
}
} }
else if (flags & ImGuiColorEditFlags_InputHSV) else if (flags & ImGuiColorEditFlags_InputHSV)
{ {