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

Modal window flag request #402

Open
dougbinks opened this issue Nov 9, 2015 · 8 comments
Open

Modal window flag request #402

dougbinks opened this issue Nov 9, 2015 · 8 comments

Comments

@dougbinks
Copy link
Contributor

Currently the window flags specify that Modal is an internal flag only able to be set by a popup using BeginPopupModal and thus using OpenPopup().

Since OpenPopup() requires both the stack and the str_id/name of the popup re-using a function which draws the popup becomes a bit more of a pain to manage than it should be (since you need to repeat code or manage state of the popup), or call OpenPopup() every frame.

A usable modal window flag would solve this problem for me.

This is likely related to issue #331 and perhaps comments from @Flix01 in issue #88 about needing to call OpenPopup() every frame.

@ocornut
Copy link
Owner

ocornut commented Nov 9, 2015

I'm not sure I understand the issue nor the request very precisely.
Are you trying to use the Modal flag on a window that isn't a popup?
Or are you trying to use popups in general while straying off the current OpenPopup/BeginBegin scheme because it has issues?

It is is the later I would be more inclined to discuss those issues in more details and find a solution. I'm pretty sure there are issues with the current scheme as I myself as a user had to sometimes juggle with the id stack and where I am user popups. I haven't yet taken the problem apart to figure out a correct solution and more than happy to hear in detail about your use case.

FYI the reason I had to initially make the opened/close state of popups stored within ImGui is that it needs to be able to close them without user intervention/code and therefore it needs access to those booleans. Calling OpenPopup() very frame is definitively incorrect and probably leading to other subtle problems.

@ocornut
Copy link
Owner

ocornut commented Nov 9, 2015

Note that even thou ImGuiWindowFlags_Modal is described as "private, don't use!" it is pretty much public and you could probably devise an improved popup patterns using a copy of the BeginPopupModal() code as a base, however I'm very much interesting in hearing about your suggestion and findings on how to make popup usage better.

@dougbinks
Copy link
Contributor Author

Apologies for not being clear.

I would like to be able to create a modal window without using the BeginPopupModal() method.

I did try using the ImGuiWindowFlags_Modal flag, which didn't work. I will take a look into the popup code to see what else is needed to replicate this functionality.

@ocornut
Copy link
Owner

ocornut commented Nov 9, 2015

Then my comment above stands - even though it is fine to bypass that mechanism I would like to know why exactly and how we can fix/improve it, rather than people just bypassing it.
(Right now my intuition is that the popup may need a re-overhaul but I haven't had time to sit and think about it so much, so every extra info is good to hear)

@dougbinks
Copy link
Contributor Author

My problem is due to not quite using the imgui approach for displaying windows, but instead I have windows which add themselves to a list to be rendered. With the exception of modal dialogues this works really well using your ImGui (many thanks :) ).

So the render function for a modal popup currently looks something like this:

void Render()
{
   static bool open = false;
    if( !open ) { ImGui::OpenPopup("Modal"); }
    if (ImGui::BeginPopupModal("Modal", NULL, ImGuiWindowFlags_AlwaysAutoResize))
    {
        open = true;
        ImGui::Text("Would you like to save changes?\n");
        if( ImGui::Button("OK") )
        {
            SomeFunctionWhichRemovesWindowFromRenderList();
        }
        ImGui::EndPopup();
    }
    else
    {
        open = false;
    }
}

Note that this function only gets called if I want the dialogue to be shown. The dialogue gets added to the list outside the render loop, and potentially by another thread so the OpenPopup() can't easily be called there.

Does this help enough in understanding the problem? I'm happy using the above approach so this isn't an urgent or essential feature request.

@ocornut
Copy link
Owner

ocornut commented Nov 9, 2015

So what's the problem with using the code above? (Sorry if I am missing something obvious)

@dougbinks
Copy link
Contributor Author

There are two problems that I can see, neither of which are major:

  1. It's not obvious you shouldn't just call ImGui::OpenPopup() every frame. A comment could fix this.
  2. The code doesn't reflect your standard window code. Moving from popup to non-modal window requires more than just a simple flag change.

ocornut added a commit that referenced this issue Nov 15, 2015
@ocornut
Copy link
Owner

ocornut commented Nov 15, 2015

  • Both 1) and 2) are connected. The behavior of popups are that ImGui can close them without your intervention. Whereas you would usually carry a bool, ImGui has to carry it for you. OpenPopup() essentially is a glorified version of setting this bool to true. If you set it to true every frame it won't break per se but then you're blocking ImGui from closing the popup and that pretty much defeat the intended behavior.
  • The other difference is that the unique identifier those function build are based on the current ID stack. It looks like this is sometimes problematic, but it is also often a desirable behavior if you consider that popups are contextual menu to a given location/item.

So

  • On the first point, I'm not sure what to do but I also feel something could be clarified. Ideally it would follow a pattern closer to Begin/End where you provide the bool but I gave up with making that possible when I was initially working on popup menus. Note that the early popup API worked like that (see Menus API work #126 from May 7) and I had to change it. I don't remember the exact reasons. I don't rule out investigating that again but realistically with the time I have on ImGui now it isn't super likely.
  • The second point, one current issue with the popup API is that it only allows you to create popups whose ID are based on the ID stack and not allow for global identifier like the regular Begin() does. I am not sure how to correct that yet but it would be good to allow for both possibilities, that's fairly likely to happen and probably easy.
  • Popup currently imply a few other minor features: a change of style by default (border), default positioning following the mouse at the moment of opening the popup but clamped within screen. Those features could possibility be made available aside from the popup flag.

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