Dim.Percent(50)+1 does not work as expected

[tig]

All those items should have black background across the entire Width.

[BDisp]

Yes that is caused by the View container that needs to made private to avoids to be cleared on the View.Redraw, like the ScrollView.

[tig]

Is this a regression then? Caused by what change?

[BDisp]

Was the #2094. The views that has a View as container must be changed to a nested private class inheriting a View. Otherwise a simple View can't be cleared.

[tznind]

The views that has a View as container must be changed to a nested private class inheriting a View

Do you mean views like Window that have a ContentView within them?

Otherwise a simple View can't be cleared.

I don't understand... is this a side effect of calling base.Redraw(...) from a subclass? Now that base class clears itself does that not play nice with ColorScheme or draw order something?

[BDisp]

Do you mean views like Window that have a ContentView within them?

No, in this case the ContentView is already a private nested class that inherit from View and there is no need to change anything.

I don't understand... is this a side effect of calling base.Redraw(...) from a subclass? Now that base class clears itself does that not >play nice with ColorScheme or draw order something?

Yes it's. If the view doesn't overridden the Redraw method the View.Redraw will clear it with the superview ColorScheme if it's null. If it's focused will be cleared with the superview Focus color. If a View that not inherit and that not overridden the View.Redraw, the only way to be cleared is on the View.Redraw. If there is no intention to be cleared then it must be a nested private class or must override the Redraw method and return without redraw.
But I'm open for suggestions.

[tznind]

Ok looking at the code in SearchCollectionNavigatorTester and the screenshot, I don't think this has anything to do with the View redraw changes.

The TreeView is configured to only take up 50% of the width of it's column (line ~SearchCollectionNavigatorTester:155) and is rendering correctly into that area except for the multi width characters where it is overspilling its bounds:

Width = Dim.Percent (50) - 1,

If you change that line to use Dim.Fill(0) then it looks as expected

But we do still need to address that overspill with long characters. I will have a look into that later, probably somewhere it is padding right based on char count not console char widths.

[BDisp]

Is this a regression then? Caused by what change?

@tig I also have this fix #2124 that avoids clearing twice if TextFormatter.Text isn't empty or null.

[tznind]

Actually looking into this it looks like there might be a bug with Dim.Percent(x)-1, I would expect it to fill 50% space like before but leave a 1 pixel margin.

[BDisp]

@tig the ustring.ConsoleWidth property has a bug in the NStack repo. It must be different on calculating the console width which must return 0 if it's less than 0. Only Rune.ColumnWidth must return the right value if it's less than zero. I already submitted a PR gui-cs/NStack#84 to fix that.

[BDisp]

Actually looking into this it looks like there might be a bug with Dim.Percent(x)-1, I would expect it to fill 50% space like before but leave a 1 pixel margin.

Don't forget that the Dim.Percent has a flag to calculate 50% of the remaining space or 50% of the superview space. The default is the last. Can you write a unit test to prove this, please.
Instead you write -1 in the offset write only 1 to see the effect, please.

[BDisp]

Yes that is caused by the View container that needs to made private to avoids to be cleared on the View.Redraw, like the ScrollView.

Only the color is resulting from the related fix, but the width difference is really caused by wrong wide characters calculation.
Edit:
Maybe this wasn't caught before because the background were never printed, but only the superview.

[tznind]

Actually looking into this it looks like there might be a bug with Dim.Percent(x)-1, I would expect it to fill 50% space like before but leave a 1 pixel margin.

Don't forget that the Dim.Percent has a flag to calculate 50% of the remaining space or 50% of the superview space. The default is the last. Can you write a unit test to prove this, please. Instead you write -1 in the offset write only 1 to see the effect, please.

This is with the width set as Dim.Percent(50)+1. I will try to write a unit test to check Frame later but haven't got time right now. Based on your statement it sounds like the offset bit is changing to PosCombine but flipping the Percent to be 'remaining space' instead of 'all space (will have to write test to confirm though as you say).

[tznind]

Here is a unit test that demonstrates the bug with Dim.Percent(50)+1

[Theory, AutoInitShutdown]
[InlineData (0,true)]
[InlineData (0, false)]
[InlineData (50,true)]
[InlineData (50, false)]

public void DimPercentPlusOne(int startingDistance,bool testHorizontal)
{
	var container = new View {
		Width = 100,
		Height = 100,
	};

	var label = new Label {
		X = testHorizontal ?  startingDistance : 0,
		Y = testHorizontal ? 0 : startingDistance,
		Width = testHorizontal ? Dim.Percent (50) + 1 : 1,
		Height = testHorizontal ? 1 : Dim.Percent (50) + 1,
	};

	Application.Top.Add (container);
	container.Add (label);
	Application.Top.LayoutSubviews ();


	Assert.Equal (100, container.Frame.Width);
	Assert.Equal (100, container.Frame.Height);

	if (testHorizontal) {
		Assert.Equal (51, label.Frame.Width);
		Assert.Equal (1, label.Frame.Height);
	}
	else {
		Assert.Equal (1, label.Frame.Width);
		Assert.Equal (51, label.Frame.Height);
	}
}

There are 4 test cases in there. Only 2 fails and that is when the position is > 0

DimPercentPlusOne(startingDistance: 50, testHorizontal: False) Failed	UnitTests	52 ms
DimPercentPlusOne(startingDistance: 50, testHorizontal: True) Failed	UnitTests	5 ms		

The failure case matches exactly what we see in the original post. When a control is positioned anywhere other than 0 and the Width is a DimCombine combining Factor(50) and a constant then the space is miscalculated as 50% of remaining distance +1 instead of 50% of total distance +1

It fails with:

Message: 
Assert.Equal() Failure
Expected: 51
Actual:   26

  Stack Trace: 
ViewTests.DimPercentPlusOne(Int32 startingX, Boolean testHorizontal) line 4095

[tznind]

I think the bug is in this code here:

Terminal.Gui/Terminal.Gui/Core/View.cs

Lines 2187 to 2195 in 237682d

	case Dim.DimFactor factor when !factor.IsFromRemaining ():
	actH = height.Anchor (hostFrame.Height);
	actH = AutoSize && s.Height > actH ? s.Height : actH;
	break;
	default:
	actH = Math.Max (height.Anchor (hostFrame.Height - actY), 0);
	actH = AutoSize && s.Height > actH ? s.Height : actH;
	break;
	}

When there is a DimFactor then the case statement looks at whether it !factor.IsFromRemaining(). If so then Anchor is called with the full hostFrame.Height. But if it is a DimCombine then you get into the default condition of the switch which has no check and enters Anchor with hostFrame.Height - actY (which is 50 in the failing case).

Need to be quite careful about the fix because DimCombine can be recursively e.g. you could have:

(Dim.Percent(25) + 1) + (Dim.Percent(25)-1)

Which would result in

| DimCombine
|__DimCombine
|   |_DimFactor
|   |_DimAbsolute
|_DimCombine
|   |_DimFactor
|   |_DimAbsolute