Skip to content

Conversation

pconerly
Copy link
Contributor

@pconerly pconerly commented Apr 16, 2018

Hey @swistakm, this is my first pass on the breaking changes in 1.51. Migrating to 1.51 is needed for #52 and is probably just good to do in general. (I was able to install 1.50 and make rebuild with no compile errors at all, so I went on to 1.51)

There are plenty of non-breaking changes (such as ColorEditFlags being added, and SetColumnWidth being added) that I didn't do.

What do you do to test this? Does the cython compilation step ensure enough typesafety that we can be sure it's good? Should I make a "1.51 demo" app?

Here's the checklist I was working off on. Sorry that my commits are in a semi-random order:

Breaking Changes

  • Renamed IsItemHoveredRect() to IsItemRectHovered(). Kept inline redirection function (will obsolete).

  • Renamed IsMouseHoveringWindow() to IsWindowRectHovered() for consistency. Kept inline redirection function (will obsolete).

  • Renamed IsMouseHoveringAnyWindow() to IsAnyWindowHovered() for consistency. Kept inline redirection function (will obsolete).

  • Renamed ImGuiCol_Columns*** enums to ImGuiCol_Separator***. Kept redirection enums (will obsolete). (Not relevant, because not implemented)

  • Renamed ImGuiSetCond*** types and enums to ImGuiCond***. Kept redirection enums (will obsolete).

  • Renamed GetStyleColName() to GetStyleColorName() for consistency. Unlikely to be used by end-user!

  • Added PushStyleColor(ImGuiCol idx, ImU32 col) overload, which might cause an "ambiguous call" compilation error if you are using ImColor() with implicit cast. Cast to ImU32 or ImVec4 explicily to fix.

  • Marked the weird IMGUI_ONCE_UPON_A_FRAME helper macro as obsolete. Prefer using the more explicit ImGuiOnceUponAFrame.

  • Changed ColorEdit4(const char* label, float col[4], bool show_alpha = true) signature to ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flags = 0), where flags 0x01 is a safe no-op (hello dodgy backward compatibility!). The new ColorEdit4/ColorPicker4 functions have lots of available flags! Check and run the demo window, under "Color/Picker Widgets", to understand the various new options.

  • Changed signature of ColorButton(ImVec4 col, bool small_height = false, bool outline_border = true) to ColorButton(const char* desc_id, ImVec4 col, ImGuiColorEditFlags flags = 0, ImVec2 size = ImVec2(0,0)). This function was rarely used and was very dodgy (no explicit ID!).

    bool ColorButton( # ✓
    const char*, ImVec4 col,
    # note: optional
    ImGuiColorEditFlags flags, ImVec2 size
    ) except +

  • Changed BeginPopupContextWindow(bool also_over_items=true, const char* str_id=NULL, int mouse_button=1) signature to (const char* str_id=NULL, int mouse_button=1, bool also_over_items=true). This is perhaps the most aggressive change in this update, but note that the majority of users relied on default parameters completely, so this will affect only a fraction of users of this already rarely used function.

  • Removed IsPosHoveringAnyWindow(), which was partly broken and misleading. In the vast majority of cases, people using that function wanted to use io.WantCaptureMouse flag. Replaced with IM_ASSERT + comment redirecting user to io.WantCaptureMouse. (#1237)

  • Removed the old ValueColor() helpers, they are equivalent to calling Text(label) + SameLine() + ColorButton().

  • Removed ColorEditMode() and ImGuiColorEditMode type in favor of ImGuiColorEditFlags and parameters to the various Color*() functions. The SetColorEditOptions() function allows to initialize default but the user can still change them with right-click context menu. Commenting out your old call to ColorEditMode() may just be fine!

Other changes to migrate or stub out

  • Added flags to ColorEdit3(), ColorEdit4(). The color edit widget now has a context-menu and access to the color picker.
  • Added flags to ColorButton().
  • Added ColorPicker3(), ColorPicker4(). The API along with those of the updated ColorEdit4() was designed so you may use them in various situation and hopefully compose your own picker if required. There are a bunch of available flags, check the Demo window and comment for ImGuiColorEditFlags_. Some of the options it supports are: two color picker types (hue bar + sat/val rectangle, hue wheel + rotating sat/val triangle), display as u8 or float, lifting 0.0..1.0 constraints (currently rgba only), context menus, alpha bar, background checkerboard options, preview tooltip, basic revert. For simple use, calling the existing ColorEdit4() function as you did before will be enough, as you can now open the color picker from there.
  • Added SetColorEditOptions() to set default color options (e.g. if you want HSV over RGBA, float over u8, select a default picker mode etc. at startup time without a user intervention. Note that the user can still change options with the context menu unless disabled with ImGuiColorFlags_NoOptions or explicitly enforcing a display type/picker mode etc.).
  • Added user-facing IsPopupOpen() function.
  • Added GetColorU32(u32) variant that perform the style alpha multiply without a floating-point round trip, and helps makes code more consistent when using ImDrawList APIs.
  • Added PushStyleColor(ImGuiCol idx, ImU32 col) overload.
  • Added GetStyleColorVec4(ImGuiCol idx) which is equivalent to accessing ImGui::GetStyle().Colors[idx] (aka return the raw style color without alpha alteration).
    ImFontAtlas: Added GlyphRangesBuilder helper class, which makes it easier to build custom glyph ranges from your app/game localization data, or add into existing glyph ranges.
  • ImFontAtlas: Added TexGlyphPadding option.
  • ImFontAtlas: Made it possible to override size of AddFontDefault() (even if it isn't really recommended!).
  • ImDrawList: Added GetClipRectMin(), GetClipRectMax() helpers.
  • Columns: Added SetColumnWidth().

@pconerly pconerly changed the title Upgrade to imgui 1.51 Upgrade to imgui 1.51 [WIP] Apr 16, 2018
@swistakm
Copy link
Member

Hey, the amount of work you did is really impressive.

For the tests I have used scripts in custom .. visual-example:: section of docstrings. The project provides a REST markup extension and pytest plugin that in conjunction allow to reuse these scripts to provide both:

  • Autogenerated images in Sphinx documentation
  • Automated tests for the widgets.
    This approach is usually straightforward for functions that display some widgets. It is trickier for functions that deal with or depend in specific interactions (e.g. hovering, expanding on click). Still they can sometimes be designed in a way that provides both meaningful visual presentation and actually test the functionality (see example for set_tooltip()).

Regarding Cython, I think that the "compilation" should be enough to provide type safety for mapped functions. Still some functions must be tested with unit tests or visual examples to ensure that we ensure correctness of the intended Python API. For instance one of tests failed in following way on Python 3:

Visual example fail: C:\projects\pyimgui\imgui\core.pyx:2635: TypeError: expected bytes, str found
179
180>    imgui.begin("Example: color button")
181>    imgui.color_button('Red', 1, 0, 0, 1)
182>    ^^^
183>    imgui.color_button('Green', 0, 1, 0, 1)
184>    imgui.color_button('Blue', 0, 0, 1, 1)
185>    imgui.color_button('Violet', 1, 0, 1, 1)
186>    imgui.end()

It's because the function is now:

def color_button(
    desc_id,
    float r, float g, float b, a=1.,
    cimgui.ImGuiColorEditFlags flags = 0,
    float width= 0.,
    float height=0.,
):
    return cimgui.ColorButton(
        "%s" % _bytes(desc_id),
        _cast_args_ImVec4(r, g, b, a),
        flags,
        _cast_args_ImVec2(width, height),
    )

With above Cython will ensure that first argument to cimgui.ColorButton is a bytestring but not ensure you can pass desc_id to color_button as both Python 2 str and Python 3 str (these have different semantics). This is where doscring examples come helpful: they allow to document and test the intended library usage.

In above example it would be probably better to use following:

def color_button(
    str desc_id,
    float r, float g, float b, a=1.,
    cimgui.ImGuiColorEditFlags flags = 0,
    float width= 0.,
    float height=0.,
):
    return cimgui.ColorButton(
        _bytes(desc_id),
        _cast_args_ImVec4(r, g, b, a),
        flags,
        _cast_args_ImVec2(width, height),
    )

Note: the _bytes() and _from_bytes functions are made exactly to deal with Python 2/3 string compatibility.

@swistakm
Copy link
Member

Side note: I wonder how/if we should provide some backwards compatibility for these changes.

Some things like is_window_rect_hovered rename can be handled relatively easily with function alias and deprecation decorator. Other features like flags instead of keyword arguments will require bit more work and I wonder if it is really worth it.

@pconerly
Copy link
Contributor Author

You're welcome!

Oh! That's a cool way to do tests. I didn't realize that the visual docs work that way, I thought this repo was actually flying with no unit tests. I'll fix this tonight.

I'll also going to add checkboxes for myself to add function stubs or fully port all of the functions added in 1.50 and 1.51.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.9%) to 43.395% when pulling f316e34 on pconerly:upgrade-to-imgui-1.51 into 6312e23 on swistakm:master.

@swistakm swistakm mentioned this pull request May 16, 2018
@swistakm
Copy link
Member

Closed in favour of #73

@swistakm swistakm closed this May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants