Skip to content

Commit

Permalink
Fix #2722 - ColorPicker / ColorEdit Widget Wrong Conversion from HSV …
Browse files Browse the repository at this point in the history
…to RGB

There already was a fix for hue wrapping around. It depended on `g.ActiveId != 0 && !g.ActiveIdAllowOverlap` condition for detecting changes through `DragInt`/`DragFloat` and it no longer worked. Fix is simply a `bool ColorFixHueWrap` in `ImGuiContext` which is set whenever `DragInt`/`DragFloat` has modified color values, thus 100% reliable now.
  • Loading branch information
rokups committed Aug 30, 2019
1 parent 3f99890 commit 7fc71db
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 7 deletions.
1 change: 1 addition & 0 deletions imgui_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,7 @@ struct ImGuiContext
ImGuiID TempInputTextId; // Temporary text input when CTRL+clicking on a slider, etc.
ImGuiColorEditFlags ColorEditOptions; // Store user options for color edit widgets
ImVec4 ColorPickerRef;
bool ColorFixHueWrap;
bool DragCurrentAccumDirty;
float DragCurrentAccum; // Accumulator for dragging modification. Always high-precision, not rounded by end-user precision settings
float DragSpeedDefaultRatio; // If speed == 0.0f, uses (max-min) * DragSpeedDefaultRatio
Expand Down
14 changes: 7 additions & 7 deletions imgui_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4242,6 +4242,9 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag
}
if (!(flags & ImGuiColorEditFlags_NoOptions))
OpenPopupOnItemClick("context");
// Differentiating between using DragInt vs. using the InputText or DropTarget. For the later we don't want
// to run the hue-wrap canceling code. If you are well versed in HSV picker please provide your input! (See #2050)
g.ColorFixHueWrap |= value_changed;
}
}
else if ((flags & ImGuiColorEditFlags_DisplayHex) != 0 && (flags & ImGuiColorEditFlags_NoInputs) == 0)
Expand Down Expand Up @@ -4629,20 +4632,16 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
}

// R,G,B and H,S,V slider color editor
bool value_changed_fix_hue_wrap = false;
if ((flags & ImGuiColorEditFlags_NoInputs) == 0)
{
PushItemWidth((alpha_bar ? bar1_pos_x : bar0_pos_x) + bars_width - picker_pos.x);
ImGuiColorEditFlags sub_flags_to_forward = ImGuiColorEditFlags__DataTypeMask | ImGuiColorEditFlags__InputMask | ImGuiColorEditFlags_HDR | ImGuiColorEditFlags_NoAlpha | ImGuiColorEditFlags_NoOptions | ImGuiColorEditFlags_NoSmallPreview | ImGuiColorEditFlags_AlphaPreview | ImGuiColorEditFlags_AlphaPreviewHalf;
ImGuiColorEditFlags sub_flags = (flags & sub_flags_to_forward) | ImGuiColorEditFlags_NoPicker;
if (flags & ImGuiColorEditFlags_DisplayRGB || (flags & ImGuiColorEditFlags__DisplayMask) == 0)
{
if (ColorEdit4("##rgb", col, sub_flags | ImGuiColorEditFlags_DisplayRGB))
{
// FIXME: Hackily differenciating using the DragInt (ActiveId != 0 && !ActiveIdAllowOverlap) vs. using the InputText or DropTarget.
// For the later we don't want to run the hue-wrap canceling code. If you are well versed in HSV picker please provide your input! (See #2050)
value_changed_fix_hue_wrap = (g.ActiveId != 0 && !g.ActiveIdAllowOverlap);
value_changed = true;
}
}
if (flags & ImGuiColorEditFlags_DisplayHSV || (flags & ImGuiColorEditFlags__DisplayMask) == 0)
value_changed |= ColorEdit4("##hsv", col, sub_flags | ImGuiColorEditFlags_DisplayHSV);
if (flags & ImGuiColorEditFlags_DisplayHex || (flags & ImGuiColorEditFlags__DisplayMask) == 0)
Expand All @@ -4651,8 +4650,9 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl
}

// Try to cancel hue wrap (after ColorEdit4 call), if any
if (value_changed_fix_hue_wrap && (flags & ImGuiColorEditFlags_InputRGB))
if ((flags & ImGuiColorEditFlags_InputRGB) && g.ColorFixHueWrap)
{
g.ColorFixHueWrap = false;
float new_H, new_S, new_V;
ColorConvertRGBtoHSV(col[0], col[1], col[2], new_H, new_S, new_V);
if (new_H <= 0 && H > 0)
Expand Down

0 comments on commit 7fc71db

Please sign in to comment.