Dock info in wxAuiManager should be publicly accessible

[craftyjon]

Description

wxAuiManager stores state in two arrays, m_panes and m_docks. There are public APIs to get access to m_panes but none for m_docks. This prevents users from being able to save/restore all parts of an AUI state without using Save/LoadPerspective, which we don't want to do for various reasons.

I haven't yet formed an opinion on what APIs should be exposed, but basically we need to be able to get and set the wxAuiDockInfo for a given pane.

Platform and version information

• wxWidgets version you use: 3.2.1
• wxWidgets port you use: all
• OS and its version: all

[vadz]

We probably could provide an iterator over wxAuiDockInfo, i.e. allow writing

for (const wxAuiDockInfo& dockInfo : manager.GetDocks()) {
   ...
}

or something like that.

But it would also be nice to have some higher level API for (de)serializing layout information, perhaps we should discuss adding this instead?

[craftyjon]

Such an iterator would be fine if we can also stick the information back in (i.e. when we are restoring a saved configuration) somehow.

But it would also be nice to have some higher level API for (de)serializing layout information, perhaps we should discuss adding this instead?

Perhaps. In general my thinking is that the client may want control over serialization. Speaking for KiCad, we just want access to the wx data structures (not serialized) because we have serialization ourselves, so we'd be faced with (/ are faced with, because the only way to do it today is with the save/loadperspective things) converting one serialization format to another.

[vadz]

I don't think it's a good idea to provide direct write access to m_docks, this would significantly reduce the possibility of changing the implementation later.

By "higher level API" I mean something that would allow you to plug it in your existing serialization format, i.e. it would provide a bridge/adapter that you would plug into -- but would also have some default implementation that could be used by the applications not needing any custom code.

If you could please summarize how does your existing serialization code work, it would be nice. In particular, I wonder how do you identify individual wxAuiDockInfo objects, as they don't have any names (should we perhaps add them?), do you do it by their index?

[craftyjon]

It doesn't work, at present. We have no way of saving and restoring floating panes, and we'd like to add it. The only thing I can think of right now is to extract data from the text format used by SavePerspective.

Perhaps it would be better to get rid of m_docks and just make wxAuiDockInfo be a child of wxAuiPaneInfo?

[vadz]

It doesn't work, at present. We have no way of saving and restoring floating panes, and we'd like to add it.

But you wrote that you already used your own serialization format. So we could reformulate the question as: how would you like it to work with your format? I was thinking of providing functions to save all panes/docks/whatever else and allowing to provide an object implementing these functions in any way it wants.

The only thing I can think of right now is to extract data from the text format used by SavePerspective.

I was thinking about replacing the format with something more structured (probably XML, as much as I dislike it, just because we already use it in several places). Would this help?

Perhaps it would be better to get rid of m_docks and just make wxAuiDockInfo be a child of wxAuiPaneInfo?

I don't think there is one-to-one relationship between docs and panes, is there?

[craftyjon]

So we could reformulate the question as: how would you like it to work with your format? I was thinking of providing functions to save all panes/docks/whatever else and allowing to provide an object implementing these functions in any way it wants.

I would like a public class/struct that contains all relevant data to restore all AUI state for a particular pane.

Then I would create a wrapper for this wx struct that serializes it to JSON for me, and do something like this on program exit:

for( paneName : listOfPanes )
{
   data = MyWrapper::Serialize( auiManager->GetPane( paneName ) );
   persistToDisk( data );
}

And startup is the reverse:

for( paneName : listOfPanes )
{
    data = loadFromDisk( paneName );
    window = createPaneWindow( paneName );
    wxAuiPaneInfo paneInfo = MyWrapper::Deserialize( data );
    auiManager->AddPane( window, paneInfo );
}

// At this point I expect all panes to have their state restored, whether they are docked or floating
auiManager->Update();

I don't think there is one-to-one relationship between docs and panes, is there?

To be honest I haven't studied the wxAuiManager code enough to know. But I am trying to persist state one-to-one per pane.

I was thinking about replacing the format with something more structured (probably XML, as much as I dislike it, just because we already use it in several places). Would this help?

I guess it would be better than parsing the existing bespoke format, but I would rather not have to write a parser and formatter for this. We can already save/restore much of the pane info through access to the wxAuiPaneInfo object via GetPane and AddPane, just not all of it.

[vadz]

I would like a public class/struct that contains all relevant data to restore all AUI state for a particular pane.

This is simple enough, but you (and everybody else) also need to save and restore all the rest of the state, i.e. docks and, importantly, anything else that gets added in the future. E.g. I once I finally implement #23986 you'd also need to save this state and so on. Hence it would be nice to have something easy to extend.

Then I would create a wrapper for this wx struct that serializes it to JSON for me, and do something like this on program exit:

for( paneName : listOfPanes )
{
   data = MyWrapper::Serialize( auiManager->GetPane( paneName ) );
   persistToDisk( data );
}

I was thinking about something like this instead:

class MyAUISerializer : public wxAuiSerializer {
public:
    void Save(const wxAuiPaneInfo& pane) override {
        data = MyWrapper::Serialize(pane);
        persistToDisk( data );
    }

    wxWindow* Restore(wxAuiPaneInfo& pane) override {
        data = loadFromDisk( pane.name );
        pane = MyWrapper::Deserialize( data );
        return createPaneWindow( pane.name );
    }
};

MyAUISerializer serializer;
auiManager->Serialize(serializer);

I don't think there is one-to-one relationship between docs and panes, is there?

To be honest I haven't studied the wxAuiManager code enough to know. But I am trying to persist state one-to-one per pane.

I could be really missing something here but I don't think you can work only with the panes. Right now you also have the docks, but you may well need more things in the future.

[craftyjon]

Hence it would be nice to have something easy to extend.

I agree about that!

I was thinking about something like this instead:

I don't see much practical difference between my pseudocode and your mockup, they both seem like they would work for me. Both rely on all required info being available via a public member of wxAuiPaneInfo

One thing that I'm not sure about with your proposal is that you directly return a window rather than just its paneinfo in Restore. This would work for my trivial example case, when creating the window and restoring its state happens at the same time at app startup, but would not work if we wanted to add a feature where you could save and restore layouts of panels at runtime (where we would not want the actual window objects deleted and re-created, potentially.

I could be really missing something here but I don't think you can work only with the panes. Right now you also have the docks, but you may well need more things in the future.

This is fine. As long as the docks, and whatever is added in the future, have publicly-accessible setters/getters for state, we can adapt to future changes.

[vadz]

One thing that I'm not sure about with your proposal is that you directly return a window

Yes, this was just a quick draft and is probably not the best way to do it. Thinking aloud here:

The way LoadPerspective() works right now is that it does nothing unless it finds an existing pane with the same name as the one being restored and so the only way to use it is to create all the possible windows before calling it. This is, of course, not ideal as you might prefer not to create any windows that are not needed, instead of creating and hiding them.

OTOH the idea of reusing already existing windows is probably good. So I think we should keep this logic and add another function to wxAuiSerializer which would be called if there is no existing pane with this window.

I.e. restore logic would be like this:

void wxAuiManager::RestoreState(wxAuiSerializer& serializer) {
    for ( auto newPane : serializer.RestorePanes() ) {
        auto& existingPane = GetPane(newPane.name);
        if ( existingPane.IsOk() ) {
            // Update the existing pane.
            existingPane.SafeSet(newPane);
        } else {
            // Add a new one.
            if ( auto w = serializer.CreateWindow(newPane.name) ) {
                AddPane(w, newPane);
            }
        }
    }

    ... deal with docks and all the rest ...
}

AFAICS this should allow to accommodate all use cases, what do you think?

[rcane]

It would be nice if an improved restoration of AUI state as proposed here could take into account things like:

• resolution changes between application starts
• displays not being there anymore in a multi-monitor setup
• changed GUI language

The current AUI code does not deal with those things at all. LoadPerspective() can put a floating frame on a non-existing monitor making it impossible to reach. Toolbars vanish because the screen resolution (or DPI scaling) was changed so that they are put outside the screen. The GUI language was changed but the captions of the panes still have the previous language version because SavePerspective() just saves the caption string currently set (instead of the translatable one).

@vadz already proposed a "higher level API". I think this should entail decoupling the internal pane representation from the information we potentially save on disk. The panes and their layout should be defined in a resolution-independent way. AddPane() would then map that to actual coordinates internally. This would make it much easier to define a stable format and gradually improve AUI.

[vadz]

Resolution changes is relatively simple, as we'd just have to use DIPs instead of pixels but this could be confusing because all the rest of wx API uses (logical) pixels, so it would need to be very clearly documented.

Monitor change could be handled in the same way it is done in wxTLWGeometryGeneric::ApplyTo(), see this code https://github.com/wxWidgets/wxWidgets/blob/f37401dde3f359860a08236fe8d6414b7c93ee88/include/wx/private/tlwgeom.h#L127-L140

Changed language: not sure what could we possibly do about this as we don't have the untranslated strings anywhere. It would be up to the serializer to use the pane names to get their titles, I guess.

[craftyjon]

KiCad already handles much of this outside wxWidgets, so we're used to needing to extend wx to get that kind of functionality. Regarding titles, those shouldn't be included in persistence data in my opinion. Pane names are an untranslated internal identifier, and the title should be set using the translation system elsewhere.

[vadz]

KiCad already handles much of this outside wxWidgets, so we're used to needing to extend wx to get that kind of functionality.

Yes, but if we could do it inside wx, it would help both KiCad (which could get rid of this code and profit of any further enhancements to it in wx itself -- knowing that if you want to enhance it yourself, you can always just do it there too) and all the other applications, so it would be nice if wx could take care of this, notably avoid placing the windows outside of the currently visible area as I hate it when applications do this, as a user (every cloud has a silver lining: at least by preventing the applications from positioning their windows at all, Wayland has solved this problem, although this approach could be also described by the other idiom, about babies and bathwater...).

[vadz]

So I've implemented my draft in #24235, any comments are welcome.

@craftyjon Please let me know what do you think and if you see anything missing from KiCad point of view. TIA!