#607 TEdgeSlip: Invalid layout during setup

[Vidar Hasfjord]

TEdgeSlip causes warning diagnostics to be generated in TRect constructors and/or TLayoutWindow::Layout due to denormal rectangles in the layout. See e.g. our Classes example.

This happens during slip insertion (TEdgeSlip::DockableInsert calling DockableMove, calling CompressGridLines, calling ReLayout, which finally calls TLayoutWindow::Layout) and window setup (TEdgeSlip::SetupWindow calling ReLayout, calling TLayoutWindow::Layout).

Note that the slip appears correct in the end, due to later Layout calls (from TDecoratedFrame::SetupWindow and TLayoutWindow::EvSize). However, it seems the layout logic assumes fully constructed windows, and hence fails to work as intended before and within setup.

Also, in addition to correcting and making the layout logic more robust, the many repeated and likely superfluous Layout calls from construction to final resize should be further looked into and streamlined.

[Ognyan Chernokozhev]

What the bug is

Ticket #607 ("TEdgeSlip: Invalid layout during setup") reports warnings about bad ("denormal") rectangles when an edge slip is laid out before or during the frame's setup.

Both paths in the ticket go through TEdgeSlip::ReLayout, which ends by calling the frame's Layout() without checking the frame's state (docking.cpp:1394):

• Slip insertion. Classes, RichEditor and WebView call THarbor::Insert before the frame window exists. The chain DockableInsert → DockableMove → CompressGridLines → ReLayout then runs Layout() while the frame's client size is still 0×0.
• Window setup. The slip's SetupWindow runs inside the frame's own setup, before TDecoratedFrame::SetupWindow has set the client size (decframe.cpp:112). So ReLayout(true) → Layout() sees the same zero size.

With a zero client size, Layout() sets the parent's right/bottom edge to -1 (layoutwi.cpp:300). Every decoration attached to those edges then gets right < left, which triggers the "Skipped invalid metrics" warning. The result still looks right in the end because TDecoratedFrame::SetupWindow and EvSize run Layout() again later.

These early Layout() calls aren't needed. The layout engine reads a slip's size from its stored window attributes (Attr), not its live window size (layoutwi.cpp:1243). ReLayout already updates those attributes and marks the frame's layout plan for rebuild, so the frame's own later Layout() picks up the new size.

The fix

In docking.cpp:1422, ReLayout still updates Attr and marks the plan for rebuild. It now only calls Layout() once the frame has finished setup, i.e. when its wfFullyCreated flag is set:

    if (df->IsFlagSet(wfFullyCreated))
      df->Layout();

This removes the early Layout() calls on both paths in the ticket. It also partly covers the ticket's second request to cut redundant Layout() calls between construction and the final resize. Docking after startup (dragging, inserting into a frame that already exists) works as before, because by then the flag is set.

Status and one risk

• Compiled: docking.cpp builds with MSBuild (owl, dtu|x64) with no errors.
• Not run: I haven't run Classes with diagnostics on to check that the warnings stop. The other behaviour claims above come from reading the code, not from running it.
• Possible regression: suppose an app overrides SetupWindow in a frame class, calls the base version, then inserts a new slip into the harbor in the same function. The frame isn't flagged as fully created yet, so ReLayout would skip Layout(), and the slip might not be placed until the next resize. None of the examples (Classes, RichEditor, WebView, FolderSize) do this; they all insert before the frame exists. If this case matters, one option is to also call Layout() once when the flag gets set.

[Ognyan Chernokozhev]

Fixed in trunk in [r8966]. The change is one line, so it will be safe to merge in 6.44 and 7, but the issues does not seem to be big enough to be worth it.