gui-cs / Terminal.Gui

Cross Platform Terminal UI toolkit for .NET
MIT License
9.63k stars 687 forks source link

Manipulating `Adornment` doesn't always cause `LayoutSubviews` #3702

Closed tig closed 1 month ago

tig commented 1 month ago

WindowsTerminal_8yd3k3DdVs

Oh. Yeah, that happens in v2_develop as well.

Do you think it's a bug on the Dim.Auto (DimAutoStyle.Content)?

We're missing SetLayoutNeeded or LayoutSubviews somewhere.

IIRC, I think I suspected it was someting to do with SubViewsNeedsLayout not getting set right. The logic of all of that is pretty complex.

Originally posted by @tig in https://github.com/gui-cs/Terminal.Gui/issues/3627#issuecomment-2316306055

BDisp commented 1 month ago

This unit test pass.

    [Fact]
    [AutoInitShutdown]
    public void SuperView_As_DimAutoStyle_Content_With_Two_SubViews ()
    {
        var supView = new View { Id = "supView", X = 5, Y = 5, Width = Auto (DimAutoStyle.Content), Height = Auto (DimAutoStyle.Content) };
        var subView1 = new View { Id = "subView1", Width = 10, Height = 10 };
        var subView2 = new View { Id = "subView2", Y = Pos.Bottom (subView1),  Width = 10, Height = 10 };
        supView.Add (subView1, subView2);
        var top = new Toplevel ();
        top.Add (supView);
        var rs = Application.Begin (top);

        Assert.Equal (new (10, 20), supView.Frame.Size);
        Assert.Equal (new (5, 5), supView.Frame.Location);
        Assert.Equal (new (10, 10), subView1.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Frame.Location);
        Assert.Equal (new (10, 10), subView2.Frame.Size);
        Assert.Equal (new (0, 10), subView2.Frame.Location);

        subView1.Height = 1;
        subView1.Width = 1;
        var firstIteration = false;
        Application.RunIteration (ref rs, ref firstIteration);
        Assert.Equal (new (10, 11), supView.Frame.Size);
        Assert.Equal (new (5, 5), supView.Frame.Location);
        Assert.Equal (new (1, 1), subView1.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Frame.Location);
        Assert.Equal (new (10, 10), subView2.Frame.Size);
        Assert.Equal (new (0, 1), subView2.Frame.Location);

        subView2.Height = 1;
        subView2.Width = 1;
        Application.RunIteration (ref rs, ref firstIteration);
        Assert.Equal (new (1, 2), supView.Frame.Size);
        Assert.Equal (new (5, 5), supView.Frame.Location);
        Assert.Equal (new (1, 1), subView1.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Frame.Location);
        Assert.Equal (new (1, 1), subView2.Frame.Size);
        Assert.Equal (new (0, 1), subView2.Frame.Location);

        top.Dispose ();
    }
BDisp commented 1 month ago

This unit tests also pass. So, I think the issue is the Border drawing on the super-view is done before the layout calculation and only on the next RunIteration it's drawn with the correct dimensions.

    [Fact]
    [AutoInitShutdown]
    public void SuperView_As_DimAutoStyle_Content_With_Two_SubViews_All_With_Borders ()
    {
        var supView = new View { Id = "supView", BorderStyle = LineStyle.Dashed, X = 5, Y = 5, Width = Auto (DimAutoStyle.Content), Height = Auto (DimAutoStyle.Content) };
        var subView1 = new View { Id = "subView1", BorderStyle = LineStyle.Dashed, Width = 10, Height = 10 };
        var subView2 = new View { Id = "subView2", BorderStyle = LineStyle.Dashed, Y = Pos.Bottom (subView1), Width = 10, Height = 10 };
        supView.Add (subView1, subView2);
        var top = new Toplevel ();
        top.Add (supView);
        var rs = Application.Begin (top);

        Assert.Equal (new (12, 22), supView.Frame.Size);
        Assert.Equal (new (5, 5), supView.Frame.Location);
        Assert.Equal (new (12, 22), supView.Border.Frame.Size);
        Assert.Equal (new (0, 0), supView.Border.Frame.Location);
        Assert.Equal (new (10, 10), subView1.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Frame.Location);
        Assert.Equal (new (10, 10), subView1.Border.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Border.Frame.Location);
        Assert.Equal (new (10, 10), subView2.Frame.Size);
        Assert.Equal (new (0, 10), subView2.Frame.Location);
        Assert.Equal (new (10, 10), subView2.Border.Frame.Size);
        Assert.Equal (new (0, 0), subView2.Border.Frame.Location);

        subView1.Height = 1;
        subView1.Width = 1;
        var firstIteration = false;
        Application.RunIteration (ref rs, ref firstIteration);
        Assert.Equal (new (12, 13), supView.Frame.Size);
        Assert.Equal (new (5, 5), supView.Frame.Location);
        Assert.Equal (new (12, 13), supView.Border.Frame.Size);
        Assert.Equal (new (0, 0), supView.Border.Frame.Location);
        Assert.Equal (new (1, 1), subView1.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Frame.Location);
        Assert.Equal (new (1, 1), subView1.Border.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Border.Frame.Location);
        Assert.Equal (new (10, 10), subView2.Frame.Size);
        Assert.Equal (new (0, 1), subView2.Frame.Location);
        Assert.Equal (new (10, 10), subView2.Border.Frame.Size);
        Assert.Equal (new (0, 0), subView2.Border.Frame.Location);
        subView2.Height = 1;
        subView2.Width = 1;
        Application.RunIteration (ref rs, ref firstIteration);
        Assert.Equal (new (3, 4), supView.Frame.Size);
        Assert.Equal (new (5, 5), supView.Frame.Location);
        Assert.Equal (new (3, 4), supView.Border.Frame.Size);
        Assert.Equal (new (0, 0), supView.Border.Frame.Location);
        Assert.Equal (new (1, 1), subView1.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Frame.Location);
        Assert.Equal (new (1, 1), subView1.Border.Frame.Size);
        Assert.Equal (new (0, 0), subView1.Border.Frame.Location);
        Assert.Equal (new (1, 1), subView2.Frame.Size);
        Assert.Equal (new (0, 1), subView2.Frame.Location);
        Assert.Equal (new (1, 1), subView2.Border.Frame.Size);
        Assert.Equal (new (0, 0), subView2.Border.Frame.Location);

        top.Dispose ();
    }
BDisp commented 1 month ago

You can see in this images that the AdornmentsEditor has the height of 19, not yet deducted the current height of the PaddingEditor which is 1.

image

image

tig commented 1 month ago

This unit tests also pass. So, I think the issue is the Border drawing on the super-view is done before the layout calculation and only on the next RunIteration it's drawn with the correct dimensions.

I suspect this is why I have bothered fixing it. I think I realized that once border is being drawn using subviews this will either get fixed by default or it'll have to be fixed.

BDisp commented 1 month ago

I suspect this is why I have bothered fixing it. I think I realized that once border is being drawn using subviews this will either get fixed by default or it'll have to be fixed.

For Dim.Auto we need to ensure that all subviews already been run the layout before calculation. Any Pos that reference a view that wasn't yet run the layout will give erroneous value to the superview, because the size is calculated based on Pos.X/Y + Dim.Width/Height.