imgui_club icon indicating copy to clipboard operation
imgui_club copied to clipboard

Memory Editor: doesn't seem to work with more than 24-bits of address space

Open ocornut opened this issue 3 years ago • 6 comments

From @simuuz https://github.com/ocornut/imgui/issues/3488#issuecomment-720126520

The reason for me making it from scratch is that the memory watcher extension for ImGui dies when feeding him with address spaces that are larger than 24 bit (IIRC, at least that's what has happened to me and some other emulator developers), and since the GBA has an address space that ranges from 28 to 32 bit, here I am :3

I answered

I haven't tried but I am assuming that there'll be an issue with floating point precision with large scroll amount. I don't know how you'd intend to solve it but whatever the solution (maybe implementing custom scroll bar + virtualizing position) it would be worth implementing it in the existing memory editor.. (let's resume this discussion in imgui_club issue tracker)

ocornut avatar Nov 03 '20 13:11 ocornut

By the way what it means @simuuz is I think you'll probably run into the same issue in your own memory watcher, so worth investigating a solution for the precision problem in large window (perhaps some fix to ImGuiListClipper could be done).

ocornut avatar Nov 03 '20 13:11 ocornut

@simuuz i think Omar is spot-on here. From wikipedia:

The IEEE 754 standard specifies a binary32 as having:
    Sign bit: 1 bit
    Exponent width: 8 bits
    Significand precision: 24 bits (23 explicitly stored)

I needed something similar in the past so i adapted original scrollbar code to work with 64bit integers. You may test using that:


// Vertical/Horizontal scrollbar
// The entire piece of code below is rather confusing because:
// - We handle absolute seeking (when first clicking outside the grab) and relative manipulation (afterward or when clicking inside the grab)
// - We store values as normalized ratio and in a form that allows the window content to change while we are holding on a scrollbar
// - We handle both horizontal and vertical scrollbars, which makes the terminology not ideal.
// Still, the code should probably be made simpler..
bool ScrollbarExI64(const ImRect& bb_frame, ImGuiID id, ImGuiAxis axis, int64_t* p_scroll_v, int64_t size_avail_v, int64_t size_contents_v, ImDrawCornerFlags rounding_corners)
{
    ImGuiContext& g = *GImGui;
    ImGuiWindow* window = g.CurrentWindow;
    if (window->SkipItems)
        return false;

    const float bb_frame_width = bb_frame.GetWidth();
    const float bb_frame_height = bb_frame.GetHeight();
    if (bb_frame_width <= 0.0f || bb_frame_height <= 0.0f)
        return false;

    // When we are too small, start hiding and disabling the grab (this reduce visual noise on very small window and facilitate using the resize grab)
    float alpha = 1.0f;
    if ((axis == ImGuiAxis_Y) && bb_frame_height < g.FontSize + g.Style.FramePadding.y * 2.0f)
        alpha = ImSaturate((bb_frame_height - g.FontSize) / (g.Style.FramePadding.y * 2.0f));
    if (alpha <= 0.0f)
        return false;

    const ImGuiStyle& style = g.Style;
    const bool allow_interaction = (alpha >= 1.0f);
    const bool horizontal = (axis == ImGuiAxis_X);

    ImRect bb = bb_frame;
    bb.Expand(ImVec2(-ImClamp(IM_ROUND(bb_frame_width - 2.0f), 0.0f, 3.0f), -ImClamp(IM_ROUND(bb_frame_height - 2.0f), 0.0f, 3.0f)));

    // V denote the main, longer axis of the scrollbar (= height for a vertical scrollbar)
    const float scrollbar_size_v = horizontal ? bb.GetWidth() : bb.GetHeight();

    // Calculate the height of our grabbable box. It generally represent the amount visible (vs the total scrollable amount)
    // But we maintain a minimum size in pixel to allow for the user to still aim inside.
    IM_ASSERT(ImMax<float>(size_contents_v, size_avail_v) > 0.0f); // Adding this assert to check if the ImMax(XXX,1.0f) is still needed. PLEASE CONTACT ME if this triggers.
    const int64_t win_size_v = ImMax(ImMax(size_contents_v, size_avail_v), 1L);
    const float grab_h_pixels = ImClamp(scrollbar_size_v * ((float)size_avail_v / (float)win_size_v), style.GrabMinSize, scrollbar_size_v);
    const float grab_h_norm = grab_h_pixels / scrollbar_size_v;

    // Handle input right away. None of the code of Begin() is relying on scrolling position before calling Scrollbar().
    bool held = false;
    bool hovered = false;
    ImGui::ButtonBehavior(bb, id, &hovered, &held, ImGuiButtonFlags_NoNavFocus);

    int64_t scroll_max = ImMax(1L, size_contents_v - 1/*size_avail_v*/);    // size_contents_v is number of items. this allows scrolling to see only last item.
    *p_scroll_v = ImClamp(*p_scroll_v, 0L, scroll_max);
    float scroll_ratio = ImSaturate((float)*p_scroll_v / (float)scroll_max);
    float grab_v_norm = scroll_ratio * (scrollbar_size_v - grab_h_pixels) / scrollbar_size_v;
    if (held && allow_interaction && grab_h_norm <= 1.0f)
    {
        float scrollbar_pos_v = horizontal ? bb.Min.x : bb.Min.y;
        float mouse_pos_v = horizontal ? g.IO.MousePos.x : g.IO.MousePos.y;

        // Click position in scrollbar normalized space (0.0f->1.0f)
        const float clicked_v_norm = ImSaturate((mouse_pos_v - scrollbar_pos_v) / scrollbar_size_v);
        ImGui::SetHoveredID(id);

        bool seek_absolute = false;
        if (g.ActiveIdIsJustActivated)
        {
            // On initial click calculate the distance between mouse and the center of the grab
            seek_absolute = (clicked_v_norm < grab_v_norm || clicked_v_norm > grab_v_norm + grab_h_norm);
            if (seek_absolute)
                g.ScrollbarClickDeltaToGrabCenter = 0.0f;
            else
                g.ScrollbarClickDeltaToGrabCenter = clicked_v_norm - grab_v_norm - grab_h_norm * 0.5f;
        }

        // Apply scroll
        // It is ok to modify Scroll here because we are being called in Begin() after the calculation of ContentSize and before setting up our starting position
        const float scroll_v_norm = ImSaturate((clicked_v_norm - g.ScrollbarClickDeltaToGrabCenter - grab_h_norm * 0.5f) / (1.0f - grab_h_norm));
        *p_scroll_v = IM_ROUND(scroll_v_norm * scroll_max);//(win_size_contents_v - win_size_v));

        // Update values for rendering
        scroll_ratio = ImSaturate((float)*p_scroll_v / (float)scroll_max);
        grab_v_norm = scroll_ratio * (scrollbar_size_v - grab_h_pixels) / scrollbar_size_v;

        // Update distance to grab now that we have seeked and saturated
        if (seek_absolute)
            g.ScrollbarClickDeltaToGrabCenter = clicked_v_norm - grab_v_norm - grab_h_norm * 0.5f;
    }

    // Render
    window->DrawList->AddRectFilled(bb_frame.Min, bb_frame.Max, ImGui::GetColorU32(ImGuiCol_ScrollbarBg), window->WindowRounding, rounding_corners);
    const ImU32 grab_col = ImGui::GetColorU32(held ? ImGuiCol_ScrollbarGrabActive : hovered ? ImGuiCol_ScrollbarGrabHovered : ImGuiCol_ScrollbarGrab, alpha);
    ImRect grab_rect;
    if (horizontal)
        grab_rect = ImRect(ImLerp(bb.Min.x, bb.Max.x, grab_v_norm), bb.Min.y, ImLerp(bb.Min.x, bb.Max.x, grab_v_norm) + grab_h_pixels, bb.Max.y);
    else
        grab_rect = ImRect(bb.Min.x, ImLerp(bb.Min.y, bb.Max.y, grab_v_norm), bb.Max.x, ImLerp(bb.Min.y, bb.Max.y, grab_v_norm) + grab_h_pixels);
    window->DrawList->AddRectFilled(grab_rect.Min, grab_rect.Max, grab_col, style.ScrollbarRounding);

    return held;
}

void ScrollbarI64(ImGuiAxis axis, int64_t* content_pos, int64_t content_size)
{
    ImGuiContext& g = *GImGui;
    ImGuiWindow* window = g.CurrentWindow;

    const ImGuiID id = ImGui::GetWindowScrollbarID(window, axis);
    ImGui::KeepAliveID(id);

    window->ScrollbarSizes[axis ^ 1] = g.Style.ScrollbarSize;

    // Calculate scrollbar bounding box
    const ImRect outer_rect = window->Rect();
    const ImRect inner_rect = window->InnerRect;
    const float border_size = window->WindowBorderSize;
    const float scrollbar_size = window->ScrollbarSizes[axis ^ 1];
    IM_ASSERT(scrollbar_size > 0.0f);
    const float other_scrollbar_size = window->ScrollbarSizes[axis];
    ImDrawCornerFlags rounding_corners = (other_scrollbar_size <= 0.0f) ? ImDrawCornerFlags_BotRight : 0;
    ImRect bb;
    if (axis == ImGuiAxis_X)
    {
        bb.Min = ImVec2(inner_rect.Min.x, ImMax(outer_rect.Min.y, outer_rect.Max.y - border_size - scrollbar_size));
        bb.Max = ImVec2(inner_rect.Max.x, outer_rect.Max.y);
        rounding_corners |= ImDrawCornerFlags_BotLeft;
    }
    else
    {
        bb.Min = ImVec2(ImMax(outer_rect.Min.x, outer_rect.Max.x - border_size - scrollbar_size), inner_rect.Min.y);
        bb.Max = ImVec2(outer_rect.Max.x, window->InnerRect.Max.y);
        rounding_corners |= ((window->Flags & ImGuiWindowFlags_NoTitleBar) && !(window->Flags & ImGuiWindowFlags_MenuBar)) ? ImDrawCornerFlags_TopRight : 0;
    }
    ScrollbarExI64(bb, id, axis, content_pos, inner_rect.Max[axis] - inner_rect.Min[axis], content_size, rounding_corners);
}

rokups avatar Nov 03 '20 14:11 rokups

Ah alright, thanks for the piece of code, really useful. Didn't know it was a problem with floating point precision. Sorry for the inconvenience

ghost avatar Nov 03 '20 15:11 ghost

is this fixed now?

mgood7123 avatar Aug 27 '21 07:08 mgood7123

is this fixed now?

Nope.

I needed something similar in the past so i adapted original scrollbar code to work with 64bit integers. You may test using that:

The problem isn't only a matter of Scrollbar() supporting larger range, but generally all positions (e..g CursorPos) are stored as float. However if we assume use case where for a given large window the clipper is the ONLY used thing (aka no other widget submitted outside of clipper use) it may be possible to work out a simplified scheme toward fixing this. It would requires at minimum three things:

  • It would require user feeding item height to clipper (because clipper can't reliably calculate item height from first submitted item)
  • it would require clipper to do some computation as double
  • it would require scrolling to be stored as double or int

This may be feasible and fix some use cases (not all).

ocornut avatar Nov 22 '21 15:11 ocornut

I have pushed variety of tweaks/fixes to improve this from multiple angles. It's a bit fragile but basically in the situation where a very large range window always use the clipper, things will tend to work much better into higher range. I am not considering this a full fix.

With those changes, e.g. in the Tables>Advanced demo

  • One million items now works without an issue
  • Ten millions items works with a small offset but not juggling or jumping around.

In memory editor a 30-bit range with 16 columns works more or less, scrolling on nav/up doesn't work. Because it depends on font size it mean we can't really guarantee a 30-bit range on very large fonts, but this was tested with a 28 px high fonts (will is a decent size for 4K screen). Above that would lead to other issues should as overflow leading to scrollbar disappearing (https://github.com/ocornut/imgui/issues/3609)

https://github.com/ocornut/imgui/commit/a76bc52 Clipper: store initial precision loss and apply delta in clipper.

https://github.com/ocornut/imgui/commit/027a7ba Clipper: use line size instead of cursor comparison when ranges are large.

https://github.com/ocornut/imgui/commit/6e141a9 Internals: made ScrollbarEx() use ImS64 to facilitate use with larger ranges (not fully tested) This is mostly to allow people reusing that code for custom scrollbars.

(+ unrelated fixed a small bug while doing this 926addb and 20e040c)

PS: None of those change requires a change to memory_editor they are changes in core dear imgui, but good to update memory editor to catch up with recent changes too.

ocornut avatar Dec 06 '21 16:12 ocornut