ocornut/imgui

Fork 0

mirror of https://github.com/ocornut/imgui.git synced 2026-11-14 19:04:44 +00:00

CalcListClipping, should it start with the first partially visible row? Or scroll in increments of row height? #716

New issue

Closed

opened 2016-06-26 22:26:30 +00:00 by MauganRa · 8 comments

MauganRa commented

2016-06-26 22:26:30 +00:00

(Migrated from github.com)

While working with a large table I noticed that while scrolling the start of the first visible row looked like it was padding + height of clipped text that wouldn't be fully visible.

Is there a way to make the scrollbar scroll in increments of row height?

The padding becomes consistent if I change line 2990 to:
start = ImClamp(start - 1, 0, items_count);
From:
start = ImClamp(start, 0, items_count);

But then the first row is not always fully visible.

Thanks!
Andrew

While working with a large table I noticed that while scrolling the start of the first visible row looked like it was padding + height of clipped text that wouldn't be fully visible. Is there a way to make the scrollbar scroll in increments of row height? The padding becomes consistent if I change line 2990 to: start = ImClamp(start - 1, 0, items_count); From: start = ImClamp(start, 0, items_count); But then the first row is not always fully visible. Thanks! Andrew

ocornut commented

2016-06-27 05:44:01 +00:00

(Migrated from github.com)

Could you provide demo/repro code please?

MauganRa commented

2016-06-28 01:37:51 +00:00

(Migrated from github.com)

Added a demo in a repo here:
https://github.com/MauganRa/ImGuiTableTest

Right now it's set to render the partial line.

Added a demo in a repo here: https://github.com/MauganRa/ImGuiTableTest Right now it's set to render the partial line.

ocornut commented

2016-06-28 07:34:14 +00:00

(Migrated from github.com)

Thanks, I will look at it later this week. Generally it is ten times easier (and faster to fix) if:

you post a screenshot so we can see what you are talking about at a glance.
you can reduce a bug report to a repro of a few dozen lines that I can paste anywhere. I will end up having to create that myself either way.

Thanks, I will look at it later this week. Generally it is ten times easier (and faster to fix) if: - you post a screenshot so we can see what you are talking about at a glance. - you can reduce a bug report to a repro of a few dozen lines that I can paste anywhere. I will end up having to create that myself either way.

MauganRa commented

2016-06-28 14:53:52 +00:00

(Migrated from github.com)

Okay, I'll create a gist/snippet to reproduce what I'm seeing.

Here is a screenshot:

Thanks!

Okay, I'll create a gist/snippet to reproduce what I'm seeing. Here is a screenshot: ![clipping](https://cloud.githubusercontent.com/assets/5143463/16420049/7a9dfe22-3d15-11e6-904b-fe81a742844f.png) Thanks!

MauganRa commented

2016-06-28 21:41:51 +00:00

(Migrated from github.com)

Code to reproduce the issue:

#include <ctime>
#include <cstdlib>
#include <vector>
#include <string>
#include <cmath>
#include <sstream>
#include <iomanip>
static void DrawTestWindow()
{
#define NUM_TEST_ROWS 10000
   static bool show_treeivew = true;
   static long start_time = 0;
   static std::vector<std::string> IDs;

   if (start_time == 0)
   {
      start_time = GetTickCount();
      srand(time(0));

      for (int i = 0; i < NUM_TEST_ROWS; i++)
      {
         std::stringstream ss;
         ss << "0x" << std::hex << (rand() % 2000000);
         IDs.push_back(ss.str());
      }
   }

   long current_time = GetTickCount();

   ImGui::Begin("Messages", &show_treeivew, ImVec2(800, 800));

   ImGui::Columns(3, "mycolumns");
   ImGui::Separator();
   ImGui::Text("Time"); ImGui::NextColumn();
   ImGui::Text("Message ID"); ImGui::NextColumn();
   ImGui::Text("Test Row"); ImGui::NextColumn();

   ImGui::Separator();

   static int selected = -1;

   ImGuiListClipper clipper(NUM_TEST_ROWS, ImGui::GetFontSize());

   while (clipper.Step())
   {
      if (clipper.StepNo > 0)  // we only want to render after step 0
      {
         for (int i = clipper.DisplayStart; i < clipper.DisplayEnd; i++)
         {
            char label[64];
            unsigned char can_data[8] = { rand() % 255, rand() % 255, rand() % 255, rand() % 255, rand() % 255, rand() % 255, rand() % 255, rand() % 255 };
            double trace_time = (current_time - start_time) / 1000.0;

            sprintf(label, "%0.4f", trace_time);
            if (ImGui::Selectable(label, selected == i, ImGuiSelectableFlags_SpanAllColumns))
            {
               selected = i;
            }
            ImGui::SetItemAllowOverlap();
            ImGui::NextColumn();

            if (ImGui::TreeNode(IDs[i].c_str()))
            {
               for (int j = 0; j < 8; j++)
               {
                  ImGui::Text("Test Signal Name %d:\t%2x", j, can_data[j]);
               }
               ImGui::TreePop();
            }
            ImGui::NextColumn();

            ImGui::Text("Test Text"); ImGui::NextColumn();
         }
      }
   }
   ImGui::End();
}

Screenshot:

Code to reproduce the issue: ``` #include <ctime> #include <cstdlib> #include <vector> #include <string> #include <cmath> #include <sstream> #include <iomanip> static void DrawTestWindow() { #define NUM_TEST_ROWS 10000 static bool show_treeivew = true; static long start_time = 0; static std::vector<std::string> IDs; if (start_time == 0) { start_time = GetTickCount(); srand(time(0)); for (int i = 0; i < NUM_TEST_ROWS; i++) { std::stringstream ss; ss << "0x" << std::hex << (rand() % 2000000); IDs.push_back(ss.str()); } } long current_time = GetTickCount(); ImGui::Begin("Messages", &show_treeivew, ImVec2(800, 800)); ImGui::Columns(3, "mycolumns"); ImGui::Separator(); ImGui::Text("Time"); ImGui::NextColumn(); ImGui::Text("Message ID"); ImGui::NextColumn(); ImGui::Text("Test Row"); ImGui::NextColumn(); ImGui::Separator(); static int selected = -1; ImGuiListClipper clipper(NUM_TEST_ROWS, ImGui::GetFontSize()); while (clipper.Step()) { if (clipper.StepNo > 0) // we only want to render after step 0 { for (int i = clipper.DisplayStart; i < clipper.DisplayEnd; i++) { char label[64]; unsigned char can_data[8] = { rand() % 255, rand() % 255, rand() % 255, rand() % 255, rand() % 255, rand() % 255, rand() % 255, rand() % 255 }; double trace_time = (current_time - start_time) / 1000.0; sprintf(label, "%0.4f", trace_time); if (ImGui::Selectable(label, selected == i, ImGuiSelectableFlags_SpanAllColumns)) { selected = i; } ImGui::SetItemAllowOverlap(); ImGui::NextColumn(); if (ImGui::TreeNode(IDs[i].c_str())) { for (int j = 0; j < 8; j++) { ImGui::Text("Test Signal Name %d:\t%2x", j, can_data[j]); } ImGui::TreePop(); } ImGui::NextColumn(); ImGui::Text("Test Text"); ImGui::NextColumn(); } } } ImGui::End(); } ``` Screenshot: ![clippingissue](https://cloud.githubusercontent.com/assets/5143463/16433267/f844b0d0-3d4e-11e6-8ad9-bbb5142387df.png)

ocornut commented

2016-06-29 07:38:36 +00:00

(Migrated from github.com)

Hi,

Thanks! Below the same test code with less unnecessary information. Please take the time to trim your code when submitting future issues because it usually both helps you and me in understanding a problem. In many instances people find a solution just by clarifying their repro. Otherwise it helps me seeing through it and eventually all of those bugs will make it into a test suite, so the simpler the better.

ImGui::Begin("Bug #716");

ImGui::Columns(3);
ImGuiListClipper clipper(100, ImGui::GetFontSize());
while (clipper.Step())
{
    for (int i = clipper.DisplayStart; i < clipper.DisplayEnd; i++)
    {
        ImGui::Text("time %.2f", (float)i);
        ImGui::NextColumn();
        ImGui::Text("0x%d", i);
        ImGui::NextColumn();
        ImGui::Text("Test Text"); 
        ImGui::NextColumn();
    }
}

ImGui::End();

But before-hand: The clipper helper only works with elements of same height, so by definition adding a tree-node here would break that assumption and make scrolling weird.
You could either a) maintain an ordered list of open tree-node to do an optimal fast-forward/seek the same way ImGuiListClipper does. Aka given your list of opened tree-node figure out where to seek in view. This is a little more complex that using ImGuiListClipper as is but pretty much requirement if you want to deal with enormous list with items of varying sizes.
Or you could b) not have a tree node here.. Instead use a popup window, context menu, tooltip etc. anything overlayed that wouldn't alter the list layout. It seems like a more sensible solution in this sort of design because you are unlikely to want to see the details of multiple items. Anytime I wanted to do (a) it was cumbersome and (b) ended up making more some.
Now on to the problem you reported :)
I was about to say that your line if (clipper.StepNo > 0) // we only want to render after step 0 is absolutely incorrect and illegal and would break the clipper in some circumstances. Upon digging I am finding there is a problem with the clipper introduced in #661 and which is noticeable when using column which explains what you tried to add that code. Looking into it now.

Hi, Thanks! Below the same test code with less unnecessary information. Please take the time to trim your code when submitting future issues because it usually both helps you and me in understanding a problem. In many instances people find a solution just by clarifying their repro. Otherwise it helps me seeing through it and eventually all of those bugs will make it into a test suite, so the simpler the better. ``` ImGui::Begin("Bug #716"); ImGui::Columns(3); ImGuiListClipper clipper(100, ImGui::GetFontSize()); while (clipper.Step()) { for (int i = clipper.DisplayStart; i < clipper.DisplayEnd; i++) { ImGui::Text("time %.2f", (float)i); ImGui::NextColumn(); ImGui::Text("0x%d", i); ImGui::NextColumn(); ImGui::Text("Test Text"); ImGui::NextColumn(); } } ImGui::End(); ``` 1. But before-hand: The clipper helper only works with elements of same height, so by definition adding a tree-node here would break that assumption and make scrolling weird. You could either a) maintain an ordered list of open tree-node to do an optimal fast-forward/seek the same way `ImGuiListClipper` does. Aka given your list of opened tree-node figure out where to seek in view. This is a little more complex that using `ImGuiListClipper` as is but pretty much requirement if you want to deal with enormous list with items of varying sizes. Or you could b) not have a tree node here.. Instead use a popup window, context menu, tooltip etc. anything overlayed that wouldn't alter the list layout. It seems like a more sensible solution in this sort of design because you are unlikely to want to see the details of multiple items. Anytime I wanted to do (a) it was cumbersome and (b) ended up making more some. 2. Now on to the problem you reported :) I was about to say that your line `if (clipper.StepNo > 0) // we only want to render after step 0` is absolutely incorrect and illegal and would break the clipper in some circumstances. Upon digging I am finding there is a problem with the clipper introduced in #661 and which is noticeable when using column which explains what you tried to add that code. Looking into it now.

ocornut commented

2016-06-29 08:34:18 +00:00

(Migrated from github.com)

The issues should be fixed now.

There's still a minor problem in the sense that ImGuiListClipper needs to adjust for columns - and it works now - but equivalent user-code wouldn't naturally know to do that. So it is less obvious how to make your own clipper from scratch if you had to do so. I'm taking note of it for further columns work #125 and may need to clarify some internal details about seeking.

The issues should be fixed now. There's still a minor problem in the sense that ImGuiListClipper needs to adjust for columns - and it works now - but equivalent user-code wouldn't naturally know to do that. So it is less obvious how to make your own clipper from scratch if you had to do so. I'm taking note of it for further columns work #125 and may need to clarify some internal details about seeking.

MauganRa commented

2016-06-29 23:03:15 +00:00

(Migrated from github.com)

Wow, thanks! I hadn't noticed the issue with the open nodes when scrolling yet.

I've gotten better support on here than with almost any paid tools I've used, signed up for Patron!

Wow, thanks! I hadn't noticed the issue with the open nodes when scrolling yet. I've gotten better support on here than with almost any paid tools I've used, signed up for Patron!