ENB: VR panes and the NestedSplitter plugin

58 views
Skip to first unread message

Edward K. Ream

unread,
May 10, 2024, 5:58:50 AM5/10/24
to leo-editor

This Engineering Notebook post discusses issues with the nested_splitter (NS) plugin uncovered two days ago while working on #3892: Add VR commands to cover/uncover the body pane. I've been considering what to do ever since.

tl;dr: See the Summary.

Background

PR #3893 is a thorough revision of the VR plugin. This PR leaves the VR3 plugin unchanged.

Late-stage testing revealed that Leo's caches were not restoring the ratio between the body and VR panes. A lengthy investigation showed that the caching code was not to blame.  Instead, the culprit is the LeoQtFrame.ratio property, aka c.frame.ratio.

The PR adds the VR pane in a way that breaks the c.frame.ratio property. After several days of noodling, I suspect this property is not well-defined!

Remedies

First, I'll replace the ratio properties with explicit calls to two new methods, say c.frame.compute_ratio and c.frame.compute_secondary_ratio. There is no need to obscure where the computations happen!

Second, I'll attempt to fix the computations. This work will likely involve changes to the nested_splitter plugin. For example, the primary and secondary splitters should have well-defined names. Computing the ratios should depend on the names of various splitters, not their containment relationships. Why? Because the NS plugin can change those relationships!

What I won't do

I don't want to change the PR to accommodate the NS plugin. That would have the tail wag the dog. Instead, c.frame.compute_ratio and c.frame.compute_secondary_ratio should produce correct results in all situations.

Summary

Leo's Qt gui defines two (hard-to-find!!) and buggy properties: c.frame.ratio and c.frame.secondary_ratio. I suspect these properties are not well-defined!

For clarity, Leo's Qt code will use two new methods instead: c.frame.compute_ratio and c.frame.compute_secondary_ratio. These methods may use enhancements to the NestedSplitter class. All aspects of this work are experimental.

The existing properties must remain for compatibility. However, each property will print a deprecation warning the first time scripts call them.

All your questions and comments are welcome.

Edward

Thomas Passin

unread,
May 10, 2024, 9:47:46 AM5/10/24
to leo-editor
Going by last night's devel rev, VR was close to working as expected when I used the toggle command, at least with the particular layout I was using when I tried it.  The branch for this PR, though, was a mess so I'm looking forward to your new approach.  Hopefully I will be able to adapt it to VR3 too, since this part of the code started out the same.

Edward K. Ream

unread,
May 10, 2024, 1:13:54 PM5/10/24
to leo-editor
On Friday, May 10, 2024 at 8:47:46 AM UTC-5 Thomas wrote:
Going by last night's devel rev, VR was close to working as expected when I used the toggle command, at least with the particular layout I was using when I tried it.

That's good to know.
 
The branch for this PR, though, was a mess so I'm looking forward to your new approach.

I'm not sure what you mean by a mess. PR #3906 cleans up a lot of cruft.

In any case, there will still be work to do after the intermediate PR #3906 gets merged into devel and PR #3892. Let's take things one step at a time and test each step thoroughly.

Edward

Thomas Passin

unread,
May 10, 2024, 2:18:49 PM5/10/24
to leo-editor
On Friday, May 10, 2024 at 1:13:54 PM UTC-4 Edward K. Ream wrote:
On Friday, May 10, 2024 at 8:47:46 AM UTC-5 Thomas wrote:
Going by last night's devel rev, VR was close to working as expected when I used the toggle command, at least with the particular layout I was using when I tried it.

That's good to know.

Turns out it was only for one particular layout.   When I tried other outlines that used different layouts VR didn't open right over the body pane.  I've set up a shortcut, ALT-F10, to toggle VR on and off so it's easy for me to try out.

I originally thought of replacing the body pane with a QStackedWidget or QTabbedWidget that would contain the body editor.  The VR/VR3 widget would go into one of the other slots.  That's how I do it with Freewin, and opening VR3 and my bookmark manager in tabs in the Log frame.  But I got a little lost, with all the nested frames, where to make that change and still make sure all the editor gets initialized correctly, the signals get hooked up right, hand have and the method signature and return types stay the same .  This splitter approach is probably better once we figure out the geometry.
 
The branch for this PR, though, was a mess so I'm looking forward to your new approach.

I'm not sure what you mean by a mess. PR #3906 cleans up a lot of cruft.

When I toggled VR's visibility, a new blank pane opened up outside Leo's main window, and the new pane never got populated.
 
In any case, there will still be work to do after the intermediate PR #3906 gets merged into devel and PR #3892. Let's take things one step at a time and test each step thoroughly.

The tricky part seems to be how to get the layout we want given that the starting layouts can be very different, even as to the number of panes as well as their arrangement.  The VR pane needs to move over the body and and adopt its size without changing the position of any of the existing splitter bars.
 
Edward

Edward K. Ream

unread,
May 10, 2024, 6:53:40 PM5/10/24
to leo-editor
On Friday, May 10, 2024 at 1:18:49 PM UTC-5 Thomas wrote:

>>>Going by last night's devel rev, VR was close to working as expected when I used the toggle command, at least with the particular layout I was using when I tried it.

> Turns out it was only for one particular layout.   When I tried other outlines that used different layouts VR didn't open right over the body pane.  I've set up a shortcut, ALT-F10, to toggle VR on and off so it's easy for me to try out.

I've done a lot of work this afternoon:

- Merged PR #3906 (remove ratio properties) into devel and into PR #3893 (VR pane, still in progress).

- PR #3893 radically simplifies FreeLayoutController.get_main_splitter. It now always returns the NestedSplitter whose name is "main_splitter".
Imo, if this splitter isn't the main splitter then no other splitter is! How else is Leo to know what the main splitter is???

The only item remaining for PR #3893 involves rendering rST in a timely way.

Summary

The shenanigans with the body/outline ratio are resolved to my satisfaction.

Thomas, if the VR3 plugin rearranges panes, then VR3 must manage the new layout.

Edward
Reply all
Reply to author
Forward
0 new messages