ENB: Saving restoring layouts

[Edward K. Ream]

For the last several days I have been studying the FreeLayoutController and NestedSplitter classes, with an eye to understanding how those classes collaborate to save and restore layouts.  All the following work is background to #1686. It's not an easy project.

Today I attempted to simplify ns.get_layout and ns.get_saveable_layout. Here are the original versions, sans docstrings:

def get_layout(self, _saveable=False):
    ans = {
        'orientation': self.orientation(),
        'content': []
    }
    if not _saveable:
        ans['splitter'] = self
    ans['sizes'] = self.sizes()
    for i in range(self.count()):
        w = self.widget(i)
        if isinstance(w, NestedSplitter):
            ans['content'].append(w.get_layout(_saveable=_saveable))
        else:
            if _saveable:
                ans['content'].append(getattr(w, '_ns_id', 'UNKNOWN'))
            else:
                ans['content'].append(w)
    return ans

def get_saveable_layout(self):
    return self.get_layout(_saveable=True)

Vitalije's trick of removing kwargs simplifies ns.get_layout as follows:

def get_layout(self):
    """
    Return a dict describing the layout.

    Usually you would call ns.top().get_layout()
    """
    def content(w):
        return w.get_layout() if isinstance(w, NestedSplitter) else w
       
    def widgets():
        """Yield an ordered list of the contained widgets."""
        return (self.widget(i) for i in range(self.count()))
   
    return {
        'content': [content(w) for w in widgets()],
        'orientation': self.orientation(),
        'sizes': self.sizes(),
        'splitter': self,
    }

My first thought was to wonder why ns.get_saveable_layout isn't exactly the same. The answer is that the dict returned by ns.get_saveable_layout will eventually be picked. But Qt widgets can not be pickled.  Instead, here is the new version of ns.get_saveable_layout:

def get_saveable_layout(self):
    """
    Return a dict describing the layout.

    Usually you would call ns.top().get_layout()
    """
    def content(w):
        return w.get_saveable_layout() if isinstance(w, NestedSplitter) else getattr(w, '_ns_id', 'UNKNOWN')
       
    def widgets():
        """Yield an ordered list of the contained widgets."""
        return (self.widget(i) for i in range(self.count()))
   
    return {
        'content': [content(w) for w in widgets()],
        'orientation': self.orientation(),
        'sizes': self.sizes(),
    }

The two methods are similar, but ns.get_layout returns a dict containing Qt widgets (and includes a 'splitter' key) while ns.get_saveable_layout substitutes id strings for the actual widgets.

Summary

I believe the new code is equivalent to the old. However, it's not clear that changing the two methods is worthwhile.

Anyway, I now understand what is going on in much more detail.

I will eventually make at least one change. ns.layout_to_text is buggy and should be replaced by g.printObj.

Edward

[tbp1...@gmail.com]

When a pane is opened as a toplevel window by using the Open Window context menu, that pane is not a part of the ordinary nested splitter layout, so far as I can tell.  How can this pane be accessed?

[tbp1...@gmail.com]

This question is related to (or part of) the question of how you can tell that your pane has been opened as  top-level window or as one of the panes of a nested splitter?

[Edward K. Ream]

On Tue, Sep 29, 2020 at 10:23 PM tbp1...@gmail.com <tbp1...@gmail.com> wrote:

When a pane is opened as a toplevel window by using the Open Window context menu, that pane is not a part of the ordinary nested splitter layout, so far as I can tell.  How can this pane be accessed?

When I execute one of these commands, the pane becomes a floating window.  However, it does not become a floating window when Leo is reloaded.

The relevant code is dense and not easily understood or summarized. I'm not sure how much more effort I can justify in this area. I'll spend some more time on it, but probably not a lot more.

Let me summarize what I think I know at present:

1. Terry's code has no machinery to represent floating windows, as is shown in the two methods I discussed yesterday, namely ns.get_layout and ns.get_saveabke layout. It might be relatively straightforward to add a 'floating' key to the python dicts returned from those methods.

2. Aside from floating windows, Terry's code already does a good job of automatically saving/restoring layouts. In addition, and perhaps confusingly, Terry's code also allows us to save/restore named layouts. Imo, all this works fairly well.

3. On startup, flc.loadLayouts creates free-layout-load-{layout_name} commands that will load the named layouts. So (modulo floating panes) there already is an easy way to load any layout you want, and you can bind any key to the commands using @button or @command nodes, and possibly even in myLeoSettings.leo.

Summary

Imo, Terry's code is pretty good as it is. It automatically saves/restores layouts in Leo's caches. You can see the state of the cache using --trace=cache. You can change the relevant parts of the cache by saving or deleting named layouts.

At present, Terry's code does not remember whether panes are floated. It might be possible to add that capability, but imo that is not a high priority.

The question of whether VR3 should be global or local turns out to be way tricky. I have refactored flc.loadLayouts in order to get a grip on this question, but everything is still murky. I would like to do an ENB on this subject. Maybe today or tomorrow.

Edward

[tbp1...@gmail.com]

On Wednesday, September 30, 2020 at 11:57:38 AM UTC-4 Edward K. Ream wrote:

On Tue, Sep 29, 2020 at 10:23 PM tbp1...@gmail.com <tbp1...@gmail.com> wrote:

When a pane is opened as a toplevel window by using the Open Window context menu, that pane is not a part of the ordinary nested splitter layout, so far as I can tell.  How can this pane be accessed?

When I execute one of these commands, the pane becomes a floating window.  However, it does not become a floating window when Leo is reloaded.

That's what I see, too.

The relevant code is dense and not easily understood or summarized. I'm not sure how much more effort I can justify in this area. I'll spend some more time on it, but probably not a lot more.

Let me summarize what I think I know at present:

1. Terry's code has no machinery to represent floating windows, as is shown in the two methods I discussed yesterday, namely ns.get_layout and ns.get_saveabke layout. It might be relatively straightforward to add a 'floating' key to the python dicts returned from those methods.

I haven't been particularly interested in floating windows, until I started getting some with VR3 without intending to.  If the main Leo window obscures part of the floating window, I find that annoying because I have to constantly bring it to the front.  But I suppose that some might want it to appear in a second monitor - which I don't have - so one way or another the floating window case probably ought to behave itself.

 ...

> 3. On startup, flc.loadLayouts creates free-layout-load-{layout_name} commands that will load the named layouts. So (modulo floating

> panes) there already is an easy way to load any layout you want, and you can bind any key to the commands using @button or

> @command nodes, and possibly even in myLeoSettings.leo.

Yes, I saw that the load_layout code calls your pane's ns_provide(), which returns your pane's widget if it isn't there already.  And I can see that a floating top level window isn't included in this process.  Like you, I think this machinery works pretty well.  But somewhere there must be some reference to the floating window, and I haven't tracked it down yet.

> Summary

...

> At present, Terry's code does not remember whether panes are floated. It might be possible to add that capability, but imo that is not a

> high priority.

Agreed.

> The question of whether VR3 should be global or local turns out to be way tricky. I have refactored flc.loadLayouts in order to get a grip

> on this question, but everything is still murky. I would like to do an ENB on this subject. Maybe today or tomorrow.

I see that there is a way to make the pane it a singleton, but that might get tricky if VR3 wants to change the type of widget (I.e., to a plain text widget).  I don't really have a problem with a separate widget per outline (except possibly for a floating window).  But I have started to wonder if the current VRx code could be leaking widgets.  It seems that they can be created frequently without anyone being able to notice.  When ns_provide() runs, a new widget is created and returned.  I *think* that when this gets assigned it replaces the previous assignment (I mean in Terry's code).  If this is correct, then the previous widget ought to be available for garbage collection, but I don't feel confident about that yet.

[Edward K. Ream]

> On Tue, Sep 29, 2020 at 10:23 PM tbp1...@gmail.com <tbp1...@gmail.com> wrote:

> I haven't been particularly interested in floating windows, until I started getting some with VR3 without intending to.  If the main Leo window obscures part of the floating window, I find that annoying because I have to constantly bring it to the front.  But I suppose that some might want it to appear in a second monitor - which I don't have - so one way or another the floating window case probably ought to behave itself.

>> At present, Terry's code does not remember whether panes are floated. It might be possible to add that capability, but imo that is not a high priority.

> Agreed.

Good. I am going to put this project on the back burner for now. I just can't seem to get too excited about it. Note that the "layouts" branch contains the recent code. I probably will merge layouts into devel, but there is no rush.

> I have started to wonder if the current VRx code could be leaking widgets.  It seems that they can be created frequently without anyone being able to notice.  When ns_provide() runs, a new widget is created and returned.  I *think* that when this gets assigned it replaces the previous assignment (I mean in Terry's code).  If this is correct, then the previous widget ought to be available for garbage collection, but I don't feel confident about that yet.

Let me look into this. Because of undo, very little that is referenced via any c.ivars ever gets gc'd. Hmm, I wonder whether Leo's reference chains ever reference more than one vr widget.

Let's take a look. The module-level (global) controllers dict contains a reference to a ViewRenderedController3 (a QWidget) for every Commander. That is, keys are Commanders, values are QWidget's. In effect, every Commander has its own singleton ViewRenderedController3 widget. That seems like it should prevent horrible leaks.

At present, there can be multiple calls to vr3.create_base_text_widget. I'm not sure whether this might "over-allocate" widgets, but even if so, the extra widgets are likely to be gc'd.

Edward

[tbp1...@gmail.com]

By adding print statements to the constructors, I see that only one VR3 plugin instance is getting created per outline.  I also see that a new VR3 controller widget (ViewRenderedController3) is getting created every time the vr3-show command is run. So the question is, what happens to the previous instance?  Leaked or GCed?  I suppose the Viewrendered plugin works the same way, since the VR3 code was borrowed pretty much as is from it.

Of course, it would be better if the same widget is used when it is hidden and then shown again.  I'm going to look into that.

On Friday, October 2, 2020 at 12:14:47 PM UTC-4 Edward K. Ream wrote:

...

[Edward K. Ream]

On Fri, Oct 2, 2020 at 11:57 AM tbp1...@gmail.com <tbp1...@gmail.com> wrote:

By adding print statements to the constructors, I see that only one VR3 plugin instance is getting created per outline.  I also see that a new VR3 controller widget (ViewRenderedController3) is getting created every time the vr3-show command is run.

That's not how I read the code.  See vr3.getVr3. It's vr3.ensure_text_widget that allocates the widget.

 

So the question is, what happens to the previous instance?  Leaked or GCed?

Imo you are overly worried about the gc. Unless VR3 is creating separate links to allocated widgets, it hardly matters how many widgets get allocated. Only widgets with references will stick around. The gc will handle the rest.

Edward

[tbp1...@gmail.com]

There are two widgets involved here.  One is the actual displayed widget, such as a QWebView.  That's the one checked by vr3.ensure_text_widget(). The other one is the ViewRenderedController3 itself, which is not a display widget in itself. The latter is the one I was surprised to see is getting re-created every time I toggle the VR3 panel off and then on again.  I hadn't expected that, although I suppose I should have.

I don't favor creating and dropping resources if not needed, but it doesn't happen very often for these VRx panes. So I suppose it's not that bad.  I just want to make sure if possible that they are GCed, and not trapped in limbo by some unexpected reference.  For instance, it looks like the layout that shows the widget has a reference to it (I think).  When that layout is asked for again, is it copied with the copy getting a reference to the new widget, or is its reference simply replaced by the new?  If the former, we might have a leak if the old layout still hangs around somewhere.

[Edward K. Ream]

On Sat, Oct 3, 2020 at 7:41 AM tbp1...@gmail.com <tbp1...@gmail.com> wrote:

There are two widgets involved here.  One is the actual displayed widget, such as a QWebView.  That's the one checked by vr3.ensure_text_widget(). The other one is the ViewRenderedController3 itself, which is not a display widget in itself. The latter is the one I was surprised to see is getting re-created every time I toggle the VR3 panel off and then on again.  I hadn't expected that, although I suppose I should have.

This is likely a bug in the VR3 plugin.  I did a cff on ViewRenderedController3 and found that vr3.setup does not save the new controller in the global "controllers" dict. This bug does not exist in the VR plugin.

Edward

[Thomas Passin]

OK, I'll take a look at that.

[Thomas Passin]

I presume you are referring to the code in the command "vr":

@g.command('vr')
def viewrendered(event):
    """Open render view for commander"""
    global controllers, layouts
    if g.app.gui.guiName() != 'qt':
        return None
    c = event.get('c')
    if not c:
        return None
    h = c.hash()
    vr = controllers.get(h)
    if not vr:
        controllers[h] = vr = ViewRenderedController(c)

I had slightly modified this code.  But when I restored it to your version, I still found that a new ViewRenderedController3 object gets instantiated each time the rendering panel gets hidden and then shown.  What's more, I find that the same happens with the current no-docks version of the VR plugin, too.

I think this is inevitable.  When VR/VR3 gets shown again, its new layout calls ns_provide() to get the widget to populate the panel.  That call instantiates a new VR/VR3 widget.  The other day, I tried caching the widget outside the layout and controllers, and returning the cached object when ns_provide() was called.  But I got errors from pyqt saying that the c/c++ wrapper had already been torn down.

[Edward K. Ream]

On Mon, Oct 5, 2020 at 1:12 PM Thomas Passin <tbp1...@gmail.com> wrote:

I presume you are referring to the code in the command "vr":

@g.command('vr')
def viewrendered(event):
    """Open render view for commander"""
    global controllers, layouts
    if g.app.gui.guiName() != 'qt':
        return None
    c = event.get('c')
    if not c:
        return None
    h = c.hash()
    vr = controllers.get(h)
    if not vr:
        controllers[h] = vr = ViewRenderedController(c)

I had slightly modified this code.  But when I restored it to your version, I still found that a new ViewRenderedController3 object gets instantiated each time the rendering panel gets hidden and then shown.  What's more, I find that the same happens with the current no-docks version of the VR plugin, too.

Ok. Let's agree not to worry about this for now. We both have more important things to do.

Edward

[Thomas Passin]

On Monday, October 5, 2020 at 3:30:12 PM UTC-4, Edward K. Ream wrote:

...

I had slightly modified this code.  But when I restored it to your version, I still found that a new ViewRenderedController3 object gets instantiated each time the rendering panel gets hidden and then shown.  What's more, I find that the same happens with the current no-docks version of the VR plugin, too.

Ok. Let's agree not to worry about this for now. We both have more important things to do.

Good by me.  That's about where I was anyway.  For future reference, though, I've learned that when Python deletes a pyqt object, the underlying QT c/c++ object doesn't necessarily get deleted;  it's possible that only the python wrapper gets deleted.  The reverse can happen, too.  At some point, maybe I will understand this well enough to make useful suggestions.