Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trying to scroll in the same frame #6349

Closed
voidware opened this issue Apr 18, 2023 · 14 comments
Closed

Trying to scroll in the same frame #6349

voidware opened this issue Apr 18, 2023 · 14 comments

Comments

@voidware
Copy link

version: latest master.

As text gets added, it won't scroll in the same frame, only 1 later. This means i get a horrible flicker as it renders the text in the wrong place, then scrolls up and renders again.

To make things exciting all this text appears in a single ImgGui item which i add like this:

   ImVec2 pos = ImGui::GetCursorScreenPos();
   ImVec2 size(r._w, r._h);
   ImVec2 bot(pos.x + size.x, pos.y + size.y);

   ImRect bb(pos, bot);
   ImGui::ItemSize(size); // , 0.0f);
   ImGui::ItemAdd(bb, 0));

The text itself is added directly to the window drawlist.

Plan A

I was hoping to use SetNextWindowScroll. So it appears this either doesn't apply immediately or must be before the entire window and not just a child region (where the item is to go). So that didn't work.

Plan B

This nearly worked;

Because i have everything in one item. I can calculate the size of this item and the size of the client area it is to be placed before ItemAdd using GetContentRegionAvail

This means i can calculate how much it needs to scroll. then i adjust by placing the item higher (just like some hacker would!)

   ImVec2 pos = ImGui::GetCursorScreenPos();

   pos.y -= scrolly;
   
   ImVec2 size(r._w, r._h);
   ImVec2 bot(pos.x + size.x, pos.y + size.y);

   ImRect bb(pos, bot);
   ImGui::ItemSize(size); // , 0.0f);
   ImGui::ItemAdd(bb, 0));

Works nicely. Except the scrollbar appears to have no idea what I'm doing. Instead showing space to scroll below instead of above.

Is there a way to fix the scrollbar? Or is there a better way altogether (i actually hope so).

thanks for any help.

@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

I was hoping to use SetNextWindowScroll. So it appears this either doesn't apply immediately or must be before the entire window and not just a child region (where the item is to go). So that didn't work.

You should go with Plan A. You can call SetNextWindowScroll() before BeginChild() as well.
I guess your issue is that in Begin/BeginChild the scroll value gets clamped to the previous maximum scroll value.

As you are adjusted both scrolling and contents together you should probably use SetNextWindowContentSize(0.0f, expected_content_height) + SetNextWindowScroll() together, where the content size would be your text size if that's the only thing you have in your window.

If your text is really large you might want to consider an optimization AFTER it works, of tracking how much new text height is added and then adding that to previous window->ContentSize.y and using that in SetNextWindowContentSize().

@voidware
Copy link
Author

Thanks for your reply.

I tried Plan A, but can't get it to work. Scrolling still seems to need two frames. Also SetNextWindowContentSize causes scrolling to never happen.

Here's what's going on:

I calculate in advance the size of my child window (to hold the text object) and the client space, based on some hackery with padding.

        // main text box
        int f;

        if (isMobile) f = ImGuiWindowFlags_NoScrollbar;
        else f = ImGuiWindowFlags_AlwaysVerticalScrollbar;

        // calculate the client region ourselves based on the window size
        ImVec2 cr1 = textBoxSz;
        ImGuiStyle& style = ImGui::GetStyle();
        cr1.y -= style.WindowPadding.y*2;
        cr1.x -= style.WindowPadding.x*2;
        if (!isMobile) cr1.x -= style.ScrollbarSize; // no scrollbar on mobile
        
        sctx._mainText.setViewSize(cr1);

Then i layout my content internally and calculate any scroll in Y, which is the height of my "page" minus the height of the client area (when >0).

        // first pass. if we needed to layout, then something has
        // changed and we need to scroll to end
        bool changed = sctx._mainText.renderFirst();
        int scrolly = sctx._mainText.calculateScroll();

So if changed and scrolly, i want to scroll.

Then decide to scroll either once or twice depending on how many scrolls i set.

        static int scrolls = 0;

        if (changed) scrolls = 1; // or 2 ?
        
        if (scrolls)
        {
            --scrolls;
            
            if (scrolly)
            {
                ImVec2 s(0, scrolly);
                ImGui::SetNextWindowScroll(s);
            }
        }

        ImGui::BeginChild("Main", textBoxSz, true, f);

When scrolls=1 it doesn't properly scroll. But when scrolls=2 it will scroll a second frame. This works but i get my text flicker as before.

So if SetNextWindowScroll works immediately, why does a second frame scroll change anything, given nothing else has changed?

Or is there something horrible, I'm missing? (most likely)

Thanks for helping.

@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

I can’t see any call to SetNextWindowContentSize() in your sample. Please try to build a minimal repro to exhibit the problem if any.

@voidware
Copy link
Author

Hi,

Adding SetNextWindowContentSize makes no difference because i already know the content size ImGui will choose beforehand.

I've discovered that instead of manually doing a 2-frame loop, you can add another call to SetScrollY. Although Imgui sometimes takes two frames to catch up.

Like this:

        // first pass. if we needed to layout, then something has
        // changed and we need to scroll to end
        bool changed = sctx._mainText.renderFirst();
        int scrolly = sctx._mainText.calculateScroll();
        
        if (changed)
        {
            LOG1("my scroll:", scrolly);

            ImGui::SetNextWindowContentSize(ImVec2(0, cr1.y));
            ImGui::SetNextWindowScroll(ImVec2(0,scrolly));
        }

        ImGui::BeginChild("Main", textBoxSz, true, f);

        if (changed) ImGui::SetScrollY(scrolly);

        ImGuiWindow* window = ImGui::GetCurrentWindowRead();            
        LOG1("window scroll:", window->Scroll.y);

So you'd expect "window scroll" to report the same as "my scroll". But what you get is:

my scroll:260
window scroll:179 
window scroll:260

So without issuing any new scroll commands, or moving the UI, there are two "window scroll" values reported on subsequent frame renders, and it takes Imgui another frame to "magically" catch up with "my scroll".

@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

Adding SetNextWindowContentSize makes no difference because i already know the content size ImGui will choose beforehand.

But you need to tell it so scrolling doesn't get clamped on Begin().

You are not providing a repro here as I don't know what's the value you are passing to SetNextWindowContentSize().

And actual content submitted needs to at least match or be more (not be less) than SetNextWindowContentSize() value, otherwise scrolling will be clamped again on frame N+1.

@voidware
Copy link
Author

Ok solved.

If Imgui gets it wrong for one frame, but fixes it automatically later. I just have to apply any correction to the true scroll value by adjusting my content position for that frame. Thus;

            ImVec2 pos = ImGui::GetCursorScreenPos();
            ImVec2 size(r._w, r._h);
            ImVec2 bot(pos.x + size.x, pos.y + size.y);
            
            if (_scrolly)  // non-zero only when content changed
            {
                ImGuiWindow* window = ImGui::GetCurrentWindowRead();            
                int imscroll = window->Scroll.y;

                // fixing displacement?
                int ds = _scrolly - imscroll;

                // adjust for any imgui scroll error
                pos.y -= ds;
            }

            ImRect bb(pos, bot);
            ImGui::ItemSize(size); // , 0.0f);
            ok = ImGui::ItemAdd(bb, 0);

ha!

@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

Ok solved.

You went back for Plan B which is not ideal. I am 99% certain Plan A should work but don't seem to trying it so far or at least not providing evidence that it doesn't work.

@voidware
Copy link
Author

Ok. let me investigate further.

@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

int frame_count = ImGui::GetFrameCount();
ImGui::SetNextWindowSize(ImVec2(100, 100));
ImGui::SetNextWindowScroll(ImVec2(0.0f, 300.0f));
ImGui::Begin("Test #6349");

if (frame_count < 10)
    ImGui::Dummy(ImVec2(200, 200));
if (frame_count >= 10)
    ImGui::Dummy(ImVec2(200, 500)); // Grow content after frame 10

ImGuiContext& g = *GImGui;
ImGuiWindow* window = g.CurrentWindow;
IMGUI_DEBUG_LOG("Inside window: Scroll=%.3f/%.3f, ContentSize=%.3f\n", window->Scroll.y, window->ScrollMax.y, window->ContentSize.y);

ImGui::End();
[00001] Inside window: Scroll=0.000/0.000, ContentSize=0.000
[00002] Inside window: Scroll=135.000/135.000, ContentSize=200.000 <-- Scroll clamped
[00003] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00004] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00005] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00006] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00007] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00008] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00009] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00010] Inside window: Scroll=135.000/135.000, ContentSize=200.000 <-- submitted Dummy(500,500) but scroll is late
[00011] Inside window: Scroll=300.000/435.000, ContentSize=500.000 <-- GOOD
[00012] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00013] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00014] Inside window: Scroll=300.000/435.000, ContentSize=500.000

Now if I add BEFORE Begin();

if (frame_count >= 10)
    ImGui::SetNextWindowContentSize(ImVec2(0, 500.0f));
ImGui::Begin(...);
[00001] Inside window: Scroll=0.000/0.000, ContentSize=0.000
[00002] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00003] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00004] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00005] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00006] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00007] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00008] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00009] Inside window: Scroll=135.000/135.000, ContentSize=200.000
[00010] Inside window: Scroll=300.000/435.000, ContentSize=500.000 <--- GOOD
[00011] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00012] Inside window: Scroll=300.000/435.000, ContentSize=500.000

It works.

EDIT for completeness, if I always call SetNextWindowContentSize(), then the scroll is right on 1st frame as well:

//if (frame_count >= 10)
    ImGui::SetNextWindowContentSize(ImVec2(0, 500.0f));
ImGui::Begin(...);
[00001] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00002] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00003] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00004] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00005] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00006] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00007] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00008] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00009] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00010] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00011] Inside window: Scroll=300.000/435.000, ContentSize=500.000
[00012] Inside window: Scroll=300.000/435.000, ContentSize=500.000

@voidware
Copy link
Author

Hi.

Thanks for that. You're right. I need SetNextWindowContentSize as well. Sorry, I was mistaken in thinking it didn't make any difference because i was passing in the client region and not my page region. Which is content it will need to be.

I don't need any of my previous hacks (neither planB).

So, to summarise:

I calculate the child window visible client area, based on the size i will give to BeginChild;

        // calculate the client region ourselves based on the window size
        ImVec2 cr1 = textBoxSz;
        ImGuiStyle& style = ImGui::GetStyle();
        cr1.y -= style.WindowPadding.y*2;
        cr1.x -= style.WindowPadding.x*2;
        if (!isMobile) cr1.x -= style.ScrollbarSize; // no scrollbar on mobile

Then I perform my internal layout and figure out if the layout has changed and whether there is any excess scroll over the region above.

        sctx._mainText.setViewSize(cr1);

        // first pass. if we needed to layout, then something has
        // changed and we need to scroll to end
        bool changed = sctx._mainText.renderFirst();
        int scrolly = sctx._mainText.calculateScroll();

Then important, i pass in the height of my layout region and my required scroll excess. Then BeginChild.

        if (changed)
        {
            ImGui::SetNextWindowContentSize(ImVec2(0, cr1.y + scrolly));
            ImGui::SetNextWindowScroll(ImVec2(0,scrolly));
        }

        ImGui::BeginChild("Main", textBoxSz, true, f);

And that is it!

Thanks very much for helping. Plan B was a hack. The trick was getting the correct height to SetNextWindowContentSize.

Cheers!

@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

I don't understand the complexity of your code above.
You are supposed to pass the SIZE of the content you intend to submit after BeginChild(). If you submit 1000 worth of height, pass 1000 to SetNextWindowContentSize() and then your scrollbar will be correct and scroll not clamped to previous-frame limit. You shouldn't need to calculate nor care about anything to do with client region size. The first block of code should not exist.

I can't understand nor guess what's going on in the other functions that are in your code, but this sounds abnormally complicated.

@voidware
Copy link
Author

The first block of code calculates the child window visible area and is needed for my text layout. It would be nice to have a function that calculates, given a BeginChild size what the client area will normally be. So i don't have to second guess the padding, scrollbar etc.

I need the view area width for text layout and the height for placement of sensible picture sizes.

 sctx._mainText.setViewSize(cr1);  // set visible area size
 bool changed = sctx._mainText.renderFirst(); // perform layout

Above renderFirst doesn't do any actual rendering but figures out all the object placements within the visible region width.

calculateScroll works out the height scroll excess.

    int calculateScroll() const
    {
        // get the layout region
        Rect r = _tbuild.extent();

        // get the size of the view
        ImVec2 vz = viewSize();

        // any scroll will be how much the layout overflows the view
        int dy = r.bottom() - vz.y;

        // if no overflow, then no scroll
        if (dy < 0) dy = 0;

        return dy;
    }

I recover the page height using cr.y + scrolly, which is the same _tbuild.extent()._h if i called that instead.
cdfd
After the scroll code, it performs the actual render after BeginChild

        ImGui::BeginChild("Main", textBoxSz, true, f);

        // second pass, the actual render
        sctx._mainText.renderSecond();

Here's a pic that might make things clearer.

witch3a

@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

The first block of code calculates the child window visible area and is needed for my text layout. It would be nice to have a function that calculates, given a BeginChild size what the client area will normally be. So i don't have to second guess the padding, scrollbar etc.

It's tricky to calculate it ahead of Begin() in a trustable and generic manner because those elements (e.g. Scrollbar, Menu Bar) state may change based on other parameters.

Since a refactor a few months ago there are 4 fields in ImGuiWindow: float DecoOuterSizeX1, DecoOuterSizeY1, DecoOuterSizeX2, DecoOuterSizeY2 which we use. You can use that if you trust that the decoration aren't going to change (I guess worst case if they do it'll be rare and for one frame). ImGuiWindowFlags_AlwaysVerticalScrollbar may be useful to minimize those changes.

Also note window->ScrollbarSize.x/.y which is a subset of DecoOuterXXX fields (the later are including the ScrollbarSizes).

I need the view area width for text layout and the height for placement of sensible picture sizes.

Right, that makes sense in this context.

I would guess once you performed this layout you may be able to obtain a content size and then the SetNextWindowContentSize() call would be fed height_obtained_from_layout instead of cr.y + scroll_y but I guess that's just your internal sauce, and indeed this is what you suggest with "using cr.y + scrolly, which is the same _tbuild.extent()._h if i called that instead.".

Nice looking richtext layout btw !

@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

Begin() does:

window->InnerRect.Min.x = window->Pos.x + window->DecoOuterSizeX1;
window->InnerRect.Min.y = window->Pos.y + window->DecoOuterSizeY1;
window->InnerRect.Max.x = window->Pos.x + window->Size.x - window->DecoOuterSizeX2;
window->InnerRect.Max.y = window->Pos.y + window->Size.y - window->DecoOuterSizeY2;

So you may as well use window->InnerRect or perform a similar calculation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants