Skip to content

Commit

Permalink
ColorEdit, ColorPicker: Fixed hue/saturation preservation logic from …
Browse files Browse the repository at this point in the history 
…interfering with the displayed value (but not stored value) of others widgets instances. (ocornut#6155)

Amend 30546bc, accb026, 38d22bc
  • Loading branch information
@ocornut
ocornut committed Feb 14, 2023
1 parent 092b682 commit fa0852f
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 27 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ All changes:
(Not adding Super+Delete to delete to up to end-of-line on OSX, as OSX doesn't have it)
- InputText: On OSX, inhibit usage of Alt key to toggle menu when active (used for work skip).
- Menus: Fixed layout of MenuItem()/BeginMenu() when label contains a '\n'. (#6116) [@imkcy9]
- ColorEdit, ColorPicker: Fixed hue/saturation preservation logic from interfering with
the displayed value (but not stored value) of others widgets instances. (#6155)
- PlotHistogram, PlotLines: Passing negative sizes honor alignment like other widgets.
- Combo: Allow SetNextWindowSize() to alter combo popup size. (#6130)
- Fonts: Assert that in each GlyphRanges[] pairs first is <= second.
Expand Down
13 changes: 8 additions & 5 deletions imgui_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1945,9 +1945,11 @@ struct ImGuiContext
ImFont InputTextPasswordFont;
ImGuiID TempInputId; // Temporary text input when CTRL+clicking on a slider, etc.
ImGuiColorEditFlags ColorEditOptions; // Store user options for color edit widgets
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, so we can restore Saturation in lossy RGB<>HSV round trips
ImU32 ColorEditLastColor; // RGB value with alpha set to 0.
ImGuiID ColorEditCurrentID; // Set temporarily while inside of the parent-most ColorEdit4/ColorPicker4 (because they call each others).
ImGuiID ColorEditSavedID; // ID we are saving/restoring HS for
float ColorEditSavedHue; // Backup of last Hue associated to LastColor, so we can restore Hue in lossy RGB<>HSV round trips
float ColorEditSavedSat; // Backup of last Saturation associated to LastColor, so we can restore Saturation in lossy RGB<>HSV round trips
ImU32 ColorEditSavedColor; // RGB value with alpha set to 0.
ImVec4 ColorPickerRef; // Initial/reference color at the time of opening the color picker.
ImGuiComboPreviewData ComboPreviewData;
float SliderGrabClickOffset;
Expand Down Expand Up @@ -2133,8 +2135,9 @@ struct ImGuiContext

TempInputId = 0;
ColorEditOptions = ImGuiColorEditFlags_DefaultOptions_;
ColorEditLastHue = ColorEditLastSat = 0.0f;
ColorEditLastColor = 0;
ColorEditCurrentID = ColorEditSavedID = 0;
ColorEditSavedHue = ColorEditSavedSat = 0.0f;
ColorEditSavedColor = 0;
SliderGrabClickOffset = 0.0f;
SliderCurrentAccum = 0.0f;
SliderCurrentAccumDirty = false;
Expand Down
57 changes: 35 additions & 22 deletions imgui_widgets.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4978,28 +4978,32 @@ bool ImGui::ColorEdit3(const char* label, float col[3], ImGuiColorEditFlags flag
return ColorEdit4(label, col, flags | ImGuiColorEditFlags_NoAlpha);
}

static void ColorEditRestoreH(const float* col, float* H)
{
ImGuiContext& g = *GImGui;
IM_ASSERT(g.ColorEditCurrentID != 0);
if (g.ColorEditSavedID != g.ColorEditCurrentID || g.ColorEditSavedColor != ImGui::ColorConvertFloat4ToU32(ImVec4(col[0], col[1], col[2], 0)))
return;
*H = g.ColorEditSavedHue;
}

// 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)))
IM_ASSERT(g.ColorEditCurrentID != 0);
if (g.ColorEditSavedID != g.ColorEditCurrentID || g.ColorEditSavedColor != 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;
if (*S == 0.0f || (*H == 0.0f && g.ColorEditSavedHue == 1))
*H = g.ColorEditSavedHue;

// When V == 0, S is undefined.
if (*V == 0.0f)
*S = g.ColorEditLastSat;
*S = g.ColorEditSavedSat;
}

// Edit colors components (each component in 0.0f..1.0f range).
Expand All @@ -5022,6 +5026,9 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag

BeginGroup();
PushID(label);
const bool set_current_color_edit_id = (g.ColorEditCurrentID == 0);
if (set_current_color_edit_id)
g.ColorEditCurrentID = window->IDStack.back();

// If we're not showing any slider there's no point in doing any HSV conversions
const ImGuiColorEditFlags flags_untouched = flags;
Expand Down Expand Up @@ -5055,7 +5062,7 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag
ColorConvertHSVtoRGB(f[0], f[1], f[2], f[0], f[1], f[2]);
else if ((flags & ImGuiColorEditFlags_InputRGB) && (flags & ImGuiColorEditFlags_DisplayHSV))
{
// Hue is lost when converting from greyscale rgb (saturation=0). Restore it.
// Hue is lost when converting from grayscale rgb (saturation=0). Restore it.
ColorConvertRGBtoHSV(f[0], f[1], f[2], f[0], f[1], f[2]);
ColorEditRestoreHS(col, &f[0], &f[1], &f[2]);
}
Expand Down Expand Up @@ -5194,10 +5201,11 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag
f[n] = i[n] / 255.0f;
if ((flags & ImGuiColorEditFlags_DisplayHSV) && (flags & ImGuiColorEditFlags_InputRGB))
{
g.ColorEditLastHue = f[0];
g.ColorEditLastSat = f[1];
g.ColorEditSavedHue = f[0];
g.ColorEditSavedSat = f[1];
ColorConvertHSVtoRGB(f[0], f[1], f[2], f[0], f[1], f[2]);
g.ColorEditLastColor = ColorConvertFloat4ToU32(ImVec4(f[0], f[1], f[2], 0));
g.ColorEditSavedID = g.ColorEditCurrentID;
g.ColorEditSavedColor = ColorConvertFloat4ToU32(ImVec4(f[0], f[1], f[2], 0));
}
if ((flags & ImGuiColorEditFlags_DisplayRGB) && (flags & ImGuiColorEditFlags_InputHSV))
ColorConvertRGBtoHSV(f[0], f[1], f[2], f[0], f[1], f[2]);
Expand All @@ -5209,6 +5217,8 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag
col[3] = f[3];
}

if (set_current_color_edit_id)
g.ColorEditCurrentID = 0;
PopID();
EndGroup();

Expand Down Expand Up @@ -5282,6 +5292,9 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
g.NextItemData.ClearFlags();

PushID(label);
const bool set_current_color_edit_id = (g.ColorEditCurrentID == 0);
if (set_current_color_edit_id)
g.ColorEditCurrentID = window->IDStack.back();
BeginGroup();

if (!(flags & ImGuiColorEditFlags_NoSidePreview))
Expand Down Expand Up @@ -5330,7 +5343,7 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
float R = col[0], G = col[1], B = col[2];
if (flags & ImGuiColorEditFlags_InputRGB)
{
// Hue is lost when converting from greyscale rgb (saturation=0). Restore it.
// Hue is lost when converting from grayscale rgb (saturation=0). Restore it.
ColorConvertRGBtoHSV(R, G, B, H, S, V);
ColorEditRestoreHS(col, &H, &S, &V);
}
Expand Down Expand Up @@ -5385,10 +5398,7 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
{
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));

// 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;
ColorEditRestoreH(col, &H); // Greatly reduces hue jitter and reset to 0 when hue == 255 and color is rapidly modified using SV square.
value_changed = value_changed_sv = true;
}
if (!(flags & ImGuiColorEditFlags_NoOptions))
Expand Down Expand Up @@ -5463,9 +5473,10 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
if (flags & ImGuiColorEditFlags_InputRGB)
{
ColorConvertHSVtoRGB(H, S, V, col[0], col[1], col[2]);
g.ColorEditLastHue = H;
g.ColorEditLastSat = S;
g.ColorEditLastColor = ColorConvertFloat4ToU32(ImVec4(col[0], col[1], col[2], 0));
g.ColorEditSavedHue = H;
g.ColorEditSavedSat = S;
g.ColorEditSavedID = g.ColorEditCurrentID;
g.ColorEditSavedColor = ColorConvertFloat4ToU32(ImVec4(col[0], col[1], col[2], 0));
}
else if (flags & ImGuiColorEditFlags_InputHSV)
{
Expand Down Expand Up @@ -5629,6 +5640,8 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
if (value_changed && g.LastItemData.ID != 0) // In case of ID collision, the second EndGroup() won't catch g.ActiveId
MarkItemEdited(g.LastItemData.ID);

if (set_current_color_edit_id)
g.ColorEditCurrentID = 0;
PopID();

return value_changed;
Expand Down

0 comments on commit fa0852f

Please sign in to comment.