Fixes #3169. `Bounds` -> `Viewport`: Content Scrolling in `View` #3323

tig · 2024-03-14T15:35:06Z

This is a big, far-reaching PR that addresses some fundamental issues in v1 that I originally laid out here:

https://github.com/gui-cs/Terminal.Gui/blob/v2_develop/docfx/docs/View.md

The Conceptual Documentation for Layout and Drawing has been improved to reflect the new design. Start there.

Things I'm proud of:

The new "Viewport" model is intuitive and easy to explain. You'll note the updated API docs are consistent and use plain-English that relate to the actual API terms like "Viewport", "ContentSize", etc...
My vision for Adornment is coming to fruition. It took a bunch of work but Adornments can now have subviews and it all works (and has unit tests to back it up). Note that in the new "Virtual Content" Scenario the settings UI for Viewports is actually a set of subviews w/in Padding.
Porting v1 code is easy. For example in this PR it took me about 20 minutes to get ListView using Viewport.Y instead of the old ListView._top. It took another 20 min to remove un-needed code (e.g. ListView.ScrollUp/Down/Left/Right were replaced with ListView.ScrollHoriztonal/ScrollVertical.
Things are more testable. The previous scroll functionality in ScrollView depended on unit tests that compared Draw output. Now the core functionality is tested with real unit tests.

Things I'm not proud of:

I feel like I frustrated @bidisp as he was trying to solve similar issues by trying to massage the old ScrollView code to work well with subviews of Adornments etc...
We're still not done with Adornments. Specifically Focus-related things do not work.
I'm worried I effed up how I rebased v2_develop into this and it's going to make other's have merge hell after this gets merged.

Fixes

Major progress against Finish Adornments (master Issue) #2994
Partial fix for Make OnMouseXXX follow the correct pattern #3284 / Refactor MouseEvent and MouseEventEventArgs to simplify #3029 (MouseEvent is done, MouseClick remains)
Fixes Adornments: Enable SubViews in Margin, Border, and Padding #2563
Fixes Create new ScrollBar based on a new Scroll and remove ScrollBarView/ScrollView #2489
Fixes EnsureVisibleBounds is complex & confusing #2407
Fixes Rename View.Bounds to View.Viewport and add View.ContentSize to enable built-in content scrolling #3169
Fixes WindowsDriver in TrueColor mode: Cursor flickers in bottom right corner #3364
Fixes SetRelativeLayout doesn't handled with positive/negative location offset. #3362
Fixes View drawing APIs should restrict drawing to Viewport #3302https://github.com/gui-cs/Terminal.Gui/pull/3323/commits
The layout engine has been simplified. Specfiically SetRelativeLayout.
Many new unit tests and old unit tests refactored.

Proposed Changes/Todos

Try different Bounds names. E.g. Viewport
Fix existing Draw/Layout code that isn't quite correct w.r.t. honoring Viewport (e.g. View.Move)
Enable virtual content area (Viewport.Location can be non-zero).
Add a new Scenario and unit tests to ensure complex scenarios with Adornments etc... work.
Revamp Charmap to use built-in capabilty; add a temporary 'fake' scrollbar impl based on the updated Button that supports continuous button presses.
Implement the equivalent of KeepContentInViewport -> ViewportSettings.AllowXGreaterThanContentWidth
Refactor ListView to use Viewport.Location and ContentSize instead of _top and _left

Changes that can be done after this is merged to finish the new system

Replace ScrollBarView with a new view, ScrollBar that is a composotion of 2 Button objects and another new view called Scroll
Implement the equivalent of AutoShowScrollbars
Update other built-in Views that support scrollling to take advantage of the new stuff

BDisp · 2024-03-14T15:38:27Z

Why Viewport and not ContentArea?

dodexahedron · 2024-03-14T22:48:39Z

I think both are perfectly valid.

ViewPort and ContentArea are both terms used in various UI frameworks.

For what it's worth, however, WPF uses the term Bounds for this property on controls.

BDisp · 2024-03-14T22:55:21Z

I also think the Bounds is also a perfect for this property. But I don't why we discuss what the name is to use and suddenly the others opinions doesn't matter.

tig · 2024-03-15T00:54:23Z

I also think the Bounds is also a perfect for this property. But I don't why we discuss what the name is to use and suddenly the others opinions doesn't matter.

I meant to mark this PR as draft.

I'm experimenting.

I have said for a long time that

Bounds is a bad name. It lacks any descriptiveness.
It is stupid that Bounds is a Rectangle and not a Size given it's current use.

I have also argued View needs 3 pieces of data to enable built-in scrolling:

Something that describes the size of the visible area (a Size).
Something that describes the size of the content (virtual content) (a Size)
Something that describes where the top-left of the visible area is within the content (a Point).

One idea is to combine 1) and 3) into a single Rectangle. This could be named Bounds, ContentArea, or ViewPort.

Another idea is to have separate properties for 1) and 3) (a Point and a Size).

The size of the content needs to be a Size and could be named ContentSize, VirtualContentSize, etc...

In this PR, I decided to run an experiment of what it would be like to keep 1) and 3) a single Rectangle property.

I researched other UI frameworks and saw the term Viewport was used and that its definition in those frameworks seemed to fit what we're looking for perfectly.

I wanted to like ContentArea, but I struggle with because the word "Area" means SIZE, not SIZE & LOCATION (at least to me).

So, this PR is an experiment. Nothing's decided and renaming Viewport to ContentArea or something else will not be hard.

Next step for me here is to add public Size ContentSize and get scrolling working with just the keyboard.

Note, there IS another part of all this that needs to be figured out: What do we do about Clip. As you know, I am a fan of us making clipping far more sophisticated....an irregular region;

Adds "Clip Region" clipping to ConsoleDriver #2606

Clearly, clipping only applies to Viewport (aka Bounds). IMO, by default, when View.OnDrawContent is called, the Clip should be set to Viewport and attempts do draw outside of the Viewport should not do anything.

I have continued to ignore this because the one disadvantage of Clip is it hides poorly written drawing code and code that does things like Clear too frequently. But that will come later...

…o v2_3269_Bounds-ContentArea

BDisp · 2024-03-18T14:53:44Z

I meant to mark this PR as draft.

I'm experimenting.

I have said for a long time that

Bounds is a bad name. It lacks any descriptiveness.

It is stupid that Bounds is a Rectangle and not a Size given it's current use.

System.Windows.Forms use it, see https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.control.bounds?view=windowsdesktop-8.0.

It's really a Rectangle that until now was always using the Location.Empty but in the #3254 is now possible to use negative location on the his getter property, by using the UseContentOffset and the ContentOffset properties.
Note that it's possible scrolling with or without scroll bars. See the ScrollBars and the Scrooling Without ScrollBars scenarios.

I have also argued View needs 3 pieces of data to enable built-in scrolling:

Something that describes the size of the visible area (a Size).

That is what the GetVisibleContentArea method does to using by the ClipToBounds method and on others views, that use scrolling.
Note that for the TextFormatter?.Draw we need to pass the real location and size, which the location is negative or not and the size is incremented by the negative location or not. Otherwise, it won't redraw as expected considering the Alignment, VerticalAlignment, etc...

Something that describes the size of the content (virtual content) (a Size)

ContentSize is the right property to use.

Something that describes where the top-left of the visible area is within the content (a Point).

If you are meaning the visible area of the content ia always Point.Empty.

One idea is to combine 1) and 3) into a single Rectangle. This could be named Bounds, ContentArea, or ViewPort.

On my PR I use the GetVisibleContentArea method for that, but it can be a read-only property too, as VisibleContentArea. We need to decide if the ContentArea is all the Rectangle with negative locations and incremented size, or the visible area used for clipping the region where the contents can be re-draw.

Another idea is to have separate properties for 1) and 3) (a Point and a Size).

The size of the content needs to be a Size and could be named ContentSize, VirtualContentSize, etc...

My PR use ContentSize for Size and ContentOffset for Point. Note that the ContentSize is a fixed value until the user want to change it. What it will change the Width and Height of the Rectangle is the ContentOffset. e.g. A view has a Width=10, Height=10, ContentSize.Width=20, ContentSize.Height=20, ContentOffset.X=-4, ContentOffset.Y=-6. Considering the ContentArea as the real and not the visible region it will return ContentArea(X=-4, Y=-6, Width=16, Height=14).
Note that Adornments will never use the ContentOffset neither the ContentSize, obviously.

In this PR, I decided to run an experiment of what it would be like to keep 1) and 3) a single Rectangle property.

should be a Rectangle that is combined with the ContentOffset for Location and the Frame.Size, Adornment.Thickness and the ContentOffset for the Size.

I researched other UI frameworks and saw the term Viewport was used and that its definition in those frameworks seemed to fit what we're looking for perfectly.

The name is irrelevant and must be you to decide because you are the mentor of this project.

I wanted to like ContentArea, but I struggle with because the word "Area" means SIZE, not SIZE & LOCATION (at least to me).

Any area is something with a size, no matter is rectangle, square, etc..., but with a location too. But decide by your head and whatever you decide is fine to me, only hopping you understood all my explanations so far.

So, this PR is an experiment. Nothing's decided and renaming Viewport to ContentArea or something else will not be hard.

Next step for me here is to add public Size ContentSize and get scrolling working with just the keyboard.

Leverage what I did so far because it will no diverge so much.

Note, there IS another part of all this that needs to be figured out: What do we do about Clip. As you know, I am a fan of us making clipping far more sophisticated....an irregular region;

Well that's a real challenger, may be passing an array of rectangles?

Adds "Clip Region" clipping to ConsoleDriver #2606

Clearly, clipping only applies to Viewport (aka Bounds). IMO, by default, when View.OnDrawContent is called, the Clip should be set to Viewport and attempts do draw outside of the Viewport should not do anything.

That's also my opinion.

I have continued to ignore this because the one disadvantage of Clip is it hides poorly written drawing code and code that does things like Clear too frequently. But that will come later...

Sometimes the use of the Clear too frequently is because the View is forcing only using the Normal or Focus colors. If a view want to use a custom color other than them, have to override the View.OnDrawContent to do that.

tig · 2024-03-18T16:32:51Z

I meant to mark this PR as draft.
I'm experimenting.
I have said for a long time that

Bounds is a bad name. It lacks any descriptiveness.

It is stupid that Bounds is a Rectangle and not a Size given it's current use.

System.Windows.Forms use it, see https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.control.bounds?view=windowsdesktop-8.0.

No, Window Forms does NOT use Bounds for the same thing. In Windows Forms Bounds is equivalent to our Frame:

Gets or sets the size and location of the control including its nonclient elements, in pixels, relative to the parent control.

This is partially why I have long not liked the name.

It's really a Rectangle that until now was always using the Location.Empty but in the #3254 is now possible to use negative location on the his getter property, by using the UseContentOffset and the ContentOffset properties. Note that it's possible scrolling with or without scroll bars. See the ScrollBars and the Scrooling Without ScrollBars scenarios.

FWIW, I started this PR because you told me you wanted me to, here:

#3295 (comment)

I'm approaching this with as fresh a perspective as I can, asking myself the most fundamental questions and trying to ignore legacy thinking. I find that when I do that, it helps my brain get clarity.

I have also argued View needs 3 pieces of data to enable built-in scrolling:

Something that describes the size of the visible area (a Size).

That is what the GetVisibleContentArea method does to using by the ClipToBounds method and on others views, that use scrolling. Note that for the TextFormatter?.Draw we need to pass the real location and size, which the location is negative or not and the size is incremented by the negative location or not. Otherwise, it won't redraw as expected considering the Alignment, VerticalAlignment, etc...

I'm in the middle of thinking this through right now (by actually changing the code and API docs to see the impact). If you look at the current code in this branch you'll see this API doc paragraph:

The <see cref="Viewport"/> Location and Size indicate what part of the View's virtual content area, defined by <see cref="ContentSize"/>, is visible and should be drawn. The coordinates taken by <see cref="Move"/> and <see cref="AddRune"/> are relative to <see cref="Viewport"/>, thus if <c>ViewPort.Location.Y</c> is <c>5</c> the 5th row of the content should be drawn using <c>MoveTo (x, 5)</c>.

Terminal.Gui/Terminal.Gui/View/ViewDrawing.cs

Lines 372 to 402 in 1549d0d

    
           /// <summary> 
        
           ///     Draws the view's content, including Subviews. 
        
           /// </summary> 
        
           /// <remarks> 
        
           ///     <para> 
        
           ///         The <paramref name="viewport"/> parameter is provided as a convenience; it has the same values as the 
        
           ///         <see cref="Viewport"/> property. 
        
           ///     </para> 
        
           ///     <para> 
        
           ///         The <see cref="Viewport"/> Location and Size indicate what part of the View's virtual content area, defined 
        
           ///         by <see cref="ContentSize"/>, is visible and should be drawn. The coordinates taken by <see cref="Move"/> and 
        
           ///         <see cref="AddRune"/> are relative to <see cref="Viewport"/>, thus if <c>ViewPort.Location.Y</c> is <c>5</c> 
        
           ///         the 5th row of the content should be drawn using <c>MoveTo (x, 5)</c>. 
        
           ///     </para> 
        
           ///     <para> 
        
           ///         If <see cref="ContentSize"/> is larger than <c>ViewPort.Size</c> drawing code should use <see cref="Viewport"/> 
        
           ///         to constrain drawing for better performance. 
        
           ///     </para> 
        
           ///     <para> 
        
           ///         The <see cref="Clip"/> may define smaller area than <see cref="Viewport"/>; complex drawing code can be more 
        
           ///         efficient by using <see cref="Clip"/> to constrain drawing for better performance. 
        
           ///     </para> 
        
           ///     <para> 
        
           ///         Overrides should loop through the subviews and call <see cref="Draw"/>. 
        
           ///     </para> 
        
           /// </remarks> 
        
           /// <param name="viewport"> 
        
           ///     The rectangle describing the currently visible viewport into the <see cref="View"/>; has the same value as 
        
           ///     <see cref="Viewport"/>. 
        
           /// </param> 
        
           public virtual void OnDrawContent (Rectangle viewport)

Otherwise, it won't redraw as expected considering the Alignment, VerticalAlignment, etc...

I don't understand this point. What information would be missing in how I've defined things above that would prevent alignment from working?

Something that describes the size of the content (virtual content) (a Size)

ContentSize is the right property to use.

Good. That's my current thinking too.

Something that describes where the top-left of the visible area is within the content (a Point).

If you are meaning the visible area of the content ia always Point.Empty.

One idea is to combine 1) and 3) into a single Rectangle. This could be named Bounds, ContentArea, or ViewPort.

On my PR I use the GetVisibleContentArea method for that, but it can be a read-only property too, as VisibleContentArea. We need to decide if the ContentArea is all the Rectangle with negative locations and incremented size, or the visible area used for clipping the region where the contents can be re-draw.

Another idea is to have separate properties for 1) and 3) (a Point and a Size).
The size of the content needs to be a Size and could be named ContentSize, VirtualContentSize, etc...

My PR use ContentSize for Size and ContentOffset for Point. Note that the ContentSize is a fixed value until the user want to change it. What it will change the Width and Height of the Rectangle is the ContentOffset. e.g. A view has a Width=10, Height=10, ContentSize.Width=20, ContentSize.Height=20, ContentOffset.X=-4, ContentOffset.Y=-6. Considering the ContentArea as the real and not the visible region it will return ContentArea(X=-4, Y=-6, Width=16, Height=14). Note that Adornments will never use the ContentOffset neither the ContentSize, obviously.

I think we're aligned here. The only difference is the "offset from the top-left (0,0) coordinate of the content to where the visiible area is" in my mind is Viewport.Location and the offset is provided in positive values. In the OnDrawContent API docs I linked to above "if ViewPort.Location.Y is 5 the 5th row of the content should be drawn using MoveTo (x, 5)."

In this PR, I decided to run an experiment of what it would be like to keep 1) and 3) a single Rectangle property.

should be a Rectangle that is combined with the ContentOffset for Location and the Frame.Size, Adornment.Thickness and the ContentOffset for the Size.

I researched other UI frameworks and saw the term Viewport was used and that its definition in those frameworks seemed to fit what we're looking for perfectly.

The name is irrelevant and must be you to decide because you are the mentor of this project.

I wanted to like ContentArea, but I struggle with because the word "Area" means SIZE, not SIZE & LOCATION (at least to me).

Any area is something with a size, no matter is rectangle, square, etc..., but with a location too. But decide by your head and whatever you decide is fine to me, only hopping you understood all my explanations so far.

So, this PR is an experiment. Nothing's decided and renaming Viewport to ContentArea or something else will not be hard.
Next step for me here is to add public Size ContentSize and get scrolling working with just the keyboard.

Leverage what I did so far because it will no diverge so much.

I'm trying to do that!

Note, there IS another part of all this that needs to be figured out: What do we do about Clip. As you know, I am a fan of us making clipping far more sophisticated....an irregular region;

Well that's a real challenger, may be passing an array of rectangle

Adds "Clip Region" clipping to ConsoleDriver #2606

I just looked at #2606 again and am now confused. I SWEAR I implemented a really cool structure there that represnted an irregular region composed of rectangles called Region. I must have not pushed a commit ... which means whatever brilliant work I did (ha!) is now lost. I had modeled it after the old Windows User API https://learn.microsoft.com/en-us/windows/win32/gdi/regions.

Anyway, such a complex API may not be needed... a rectangle is probably fine. But if we're serious about great performance over constrained connections like SSH, it may be worth it.

Clearly, clipping only applies to Viewport (aka Bounds). IMO, by default, when View.OnDrawContent is called, the Clip should be set to Viewport and attempts do draw outside of the Viewport should not do anything.

That's also my opinion.

I have continued to ignore this because the one disadvantage of Clip is it hides poorly written drawing code and code that does things like Clear too frequently. But that will come later...

Sometimes the use of the Clear too frequently is because the View is forcing only using the Normal or Focus colors. If a view want to use a custom color other than them, have to override the View.OnDrawContent to do that.

Yep. Another reason we need to revisit the Color API (see #457).

tig · 2024-03-18T16:36:56Z

Oops. Just noticed a typo:

I think we're aligned here. The only difference is the "offset from the top-left (0,0) coordinate of the content to where the visiible area is" in my mind is Viewport.Location and the offset is provided in positive values. In the OnDrawContent API docs I linked to above "if ViewPort.Location.Y is 5 the 5th row of the content should be drawn using MoveTo (x, 5)."

This should read "...is 5 the 6th row of the content...".

BDisp · 2024-03-18T17:22:44Z

No, Window Forms does NOT use Bounds for the same thing. In Windows Forms Bounds is equivalent to our Frame:

Understood.

This is partially why I have long not liked the name.

Choose one that you like it.

FWIW, I started this PR because you told me you wanted me to, here:

#3295 (comment)

I'm approaching this with as fresh a perspective as I can, asking myself the most fundamental questions and trying to ignore legacy thinking. I find that when I do that, it helps my brain get clarity.

I confirm, but I also expect that you target more or less the same result as I achieve. Remember that I implemented the scroll bars on various views that are working great and I hope you don't break any of them. Otherwise this doesn't make sense. I expect you do it better than I did but with more features and improvements and not the other way.

I'm in the middle of thinking this through right now (by actually changing the code and API docs to see the impact). If you look at the current code in this branch you'll see this API doc paragraph:

The <see cref="Viewport"/> Location and Size indicate what part of the View's virtual content area, defined by <see cref="ContentSize"/>, is visible and should be drawn. The coordinates taken by <see cref="Move"/> and <see cref="AddRune"/> are relative to <see cref="Viewport"/>, thus if <c>ViewPort.Location.Y</c> is <c>5</c> the 5th row of the content should be drawn using <c>MoveTo (x, 5)</c>.

I doubt that will work well with positive offset. I think that is the contrary of the scrolling behavior. With positive offsets it look like you're pushing to the right and the bottom. But surely you know what you are doing.

I don't understand this point. What information would be missing in how I've defined things above that would prevent alignment from working?

Here I only was to note that for drawing we send the real content area, the one that use offset, for redraw contents to the view and not the visible area that is used to clip a region. I didn't mention that there is some problem with alignments at all.

Good. That's my current thinking too.

Great.

I think we're aligned here. The only difference is the "offset from the top-left (0,0) coordinate of the content to where the visiible area is" in my mind is Viewport.Location and the offset is provided in positive values. In the OnDrawContent API docs I linked to above "if ViewPort.Location.Y is 5 the 5th row of the content should be drawn using MoveTo (x, 5)."

This is make me confusing. If we are at (0,0) then we are at the start of the content, right?
If the ContentSize is bigger than the View.Width/Height we want to see the contents beyond the right and beyond the bottom, right?
For that we need to push to left and top, right?
Well tell me what values you have for them? Positive or negative?
For me they are negative and is with that way they are working well on my PR.

I'm trying to do that!

I know and you'll do it well.

I just looked at #2606 again and am now confused. I SWEAR I implemented a really cool structure there that represnted an irregular region composed of rectangles called Region. I must have not pushed a commit ... which means whatever brilliant work I did (ha!) is now lost. I had modeled it after the old Windows User API https://learn.microsoft.com/en-us/windows/win32/gdi/regions.

Anyway, such a complex API may not be needed... a rectangle is probably fine. But if we're serious about great performance over constrained connections like SSH, it may be worth it.

That is very interesting and I'm curious to see how they work with irregular areas.

Yep. Another reason we need to revisit the Color API (see #457).

For sure. It may be more configurable to use a view preference than force only the Normal and Focus, beside I understand the reason for that.

BDisp · 2024-03-18T17:31:45Z

Oops. Just noticed a typo:

I think we're aligned here. The only difference is the "offset from the top-left (0,0) coordinate of the content to where the visiible area is" in my mind is Viewport.Location and the offset is provided in positive values. In the OnDrawContent API docs I linked to above "if ViewPort.Location.Y is 5 the 5th row of the content should be drawn using MoveTo (x, 5)."

This should read "...is 5 the 6th row of the content...".

I understand because it start from 0. This is an example about why it should be negative. You are using as positive but in the reality it should be -5 because the first visible row will be the 0th. See how it work: -5, -4, -3, -2, -1, 0. The 0th will be in the reality the 6th row but ofseted.

tig · 2024-03-20T14:50:52Z

I doubt that will work well with positive offset. I think that is the contrary of the scrolling behavior. With positive offsets it look like you're pushing to the right and the bottom. But surely you know what you are doing.

I think it will, but I want to make sure I understand your point. This mental model makes sense to me:

The View's content area has its origin (0, 0) at the top-left.
The View's content area is defined by ContentSize (a Size).
The Viewport is a rectangular "portal" into the View's content area with a location and a size.
Viewport.Location is the location where the top-left corner of the viewport resides within the content area.
Viewport.Size is the size of the viewport (the number of columns and rows visible to the user).
Ergo, Viewport.Location will always be >= 0, 0.

Let's compare this to ListView (assume no scrollbars):

ListView's content area has it's origin at (0, 0) at the top-left.
ListView's content area size is defined by the number of items in Source
If ListView.SelectedItem = 0 then the 1st row of it's content (ListView.Source[0]) is selected.
If the user uses the mouse wheel to scroll the ListView down one, the first visible item will now be ListView.Source[1].
Thus, the location of the first visible item is 1, not -1.

I struggle to articulate this so simply if Viewport.Location has negative values. How would you articulate it?

tig · 2024-03-20T15:00:34Z

Here I only was to note that for drawing we send the real content area, the one that use offset, for redraw contents to the view and not the visible area that is used to clip a region. I didn't mention that there is some problem with alignments at all.

Yes, you did:

That is what the GetVisibleContentArea method does to using by the ClipToBounds method and on others views, that use scrolling. Note that for the TextFormatter?.Draw we need to pass the real location and size, which the location is negative or not and the size is incremented by the negative location or not. Otherwise, it won't redraw as expected considering the Alignment, VerticalAlignment, etc...

I am trying to understand what you meant by "Otherwise, it won't redraw as expected considering the Alignment, VerticalAlignment, etc..."

BDisp · 2024-03-20T15:24:51Z

I think it will, but I want to make sure I understand your point. This mental model makes sense to me:

The View's content area has its origin (0, 0) at the top-left.

Correct.

The View's content area is defined by ContentSize (a Size).

No. It's defined by the view bounds size incremented by the negative location of the ContentOffset.

The Viewport is a rectangular "portal" into the View's content area with a location and a size.

If you meaning Viewport as the visible content area is always (0,0). If you meaning Viewport as the virtual area then the location is the negative values from the ContentOffset and the size is the bounds size incremented by the negative location.
Remember that ContentSize is always the total size of columns and rows, which mean that not all the content on the right and bottom sides are not yet visible.

Viewport.Location is the location where the top-left corner of the viewport resides within the content area.

Same answer as above.

Viewport.Size is the size of the viewport (the number of columns and rows visible to the user).

Same answer as above.

Ergo, Viewport.Location will always be >= 0, 0.

Same answer as above. If it is only the visible area used by the clipping area the location is always (0,0), otherwise is the negative values of the ContentOffset.

Let's compare this to ListView (assume no scrollbars):

You can't compare with ListView which already implemented the left and the top offset and in this case I use a UseContentOffset as false. I only grab their values to assign to the ContentOffset as negative values. If scrollbars are enabled then it use the values from the ContentOffset to set the Position property and vice versa.

ListView's content area has it's origin at (0, 0) at the top-left.

ListView's content area size is defined by the number of items in Source

If ListView.SelectedItem = 0 then the 1st row of it's content (ListView.Source[0]) is selected.

If the user uses the mouse wheel to scroll the ListView down one, the first visible item will now be ListView.Source[1].

Thus, the location of the first visible item is 1, not -1.

I didn't answer all of them because I'm not change the behavior how they offset left and top, like TextView, TableView, TreeView, etc... and I advise you not to change it. If you use the same code as they use you will struggle with the re-draw and with my solution I don't have to worry about that. The TextFormatter will draw correctly. Any derived View class can use the ContentOffset and the ContentSize properties to only redraw the contents from the positive values up to the available visible area.

I'm only using in views that the user set the UseContentOffset as true, with or without enable scollbars.

I struggle to articulate this so simply if Viewport.Location has negative values. How would you articulate it?

I think you haven't seen my PR properly yet. Otherwise you will understand how it works.

BDisp · 2024-04-15T19:29:58Z

I almost dare to say that the cause of the flickering is not related to the View drawing but to the way the Windows API processes its output to the screen. In WindowsDriver, WriteConsoleOutput and WriteConsole. In NetDriver the Console.Write. If there were problems with View drawing, it would also cause the same symptom in CursesDriver. Unless it is the value of the color attribute that is not being passed correctly to the output, since CursesDriver does not use ASCII escape sequences for color.

BDisp · 2024-04-15T20:29:57Z

Notice the detail in which when I press the mouse on the Border it causes a small blue flicker on the right side.

The cause of the shimmering blue color is due to the GetHighlightColor method returning an ARGB value in the Border_Highlight method.

BDisp · 2024-04-15T20:51:38Z

The other situations are caused by the TextFormatter?.Draw call in the OnDrawContent method of the Adornment class. It is now time for Driver.Clip to be changed to a Rectangle array.

tig · 2024-04-15T20:53:21Z

The other situations are caused by the TextFormatter?.Draw call in the OnDrawContent method of the Adornment class. It is now time for Driver.Clip to be changed to a Rectangle array.

I had reproduced the flicker-bar-thing in cases where there are no Views with (+) Thickness.

Not saying you're wrong, just that it's unlikely the only cause.

tig · 2024-04-15T20:57:27Z

Hewre

The other situations are caused by the TextFormatter?.Draw call in the OnDrawContent method of the Adornment class. It is now time for Driver.Clip to be changed to a Rectangle array.

I had reproduced the flicker-bar-thing in cases where there are no Views with (+) Thickness.

Not saying you're wrong, just that it's unlikely the only cause.

Here's Scrolling with no Adornments. Note OnDrawContent returns immediately if Thickness is Empty.

I set a breakpoint in both Adornment.OnDrawContent and Border.OnDrawcontent and they don't get hit.

tig · 2024-04-15T20:58:59Z

I also set progress.Visible = false in Scrolling.cs and it doesn't stop the flicker.

tig · 2024-04-15T21:37:34Z

Thought maybe it was Driver.FillRect so I re-implemented to access Contents directly (which is a good idea regardless as it'll be faster). Didn't fix anything.

   /// <summary>Fills the specified rectangle with the specified rune, using <see cref="CurrentAttribute"/></summary>
   /// <param name="rect">The Screen-relative rectangle.</param>
   /// <param name="rune">The Rune used to fill the rectangle</param>
   public void FillRect (Rectangle rect, Rune rune = default)
   {
       rect = Rectangle.Intersect (rect, Clip);
       lock (Contents)
       {
           for (int r = rect.Y; r < rect.Y + rect.Height; r++)
           {
               for (int c = rect.X; c < rect.X + rect.Width; c++)
               {
                   Contents [r, c] = new Cell
                   {
                       Rune = (Rune)' ', Attribute = CurrentAttribute, IsDirty = true
                   };
                   _dirtyLines [r] = true;
               }
           }
       }
   }

tig · 2024-04-15T21:38:56Z

This is only happening with WindowsDriver and NetDriver (Windows and WSL). With CursesDriver the flickering doesn't exist.

FWIW, it happens on WindowsDriver with both Force16colors set and not set. The code paths for cursor handling are very different for the two, which indicates it is NOT a cursor problem (but doesn't rule it out).

…e/AddRune

BDisp · 2024-04-15T21:51:36Z

FWIW, it happens on WindowsDriver with both Force16colors set and not set. The code paths for cursor handling are very different for the two, which indicates it is NOT a cursor problem (but doesn't rule it out).

I had already said above that in WindowsDriver it happens with both WriteConsoleOutput and WriteConsole, therefore with Force16colors or not. I think we should just assign the View class the responsibility of all writing operations to the console. I think there must be some conflicts with some views that are using the Driver to access Move, AddStr, AddRune. It must be the View class that obtains the mapping with the screen.

BDisp · 2024-04-15T22:08:16Z

Why does the CheckBox blink when clicking on the Border? Should not.

WindowsTerminal_zAWQtgAdmR.mp4

tig · 2024-04-15T23:08:46Z

Why does the CheckBox blink when clicking on the Border? Should not.

Because SetHightlight changes ColorScheme which calls SetNeedsDisplay() which is overkill. SetHighlight should be smarter about how it caues the Border to highlight, but I didn't think it all the way through.

That Window shouldn't be movable anyway.

tig · 2024-04-15T23:12:34Z

I removed the ability for the border to highlight. Centered flickers randomly by just clicking the border repeatedly.

BDisp · 2024-04-15T23:14:10Z

That Window shouldn't be movable anyway.

Even if it was movable, this shouldn't happen. But if it is the Border's ColorScheme that is changed, it should not affect the CheckBox's ColorScheme.

BDisp · 2024-04-15T23:23:56Z

The horizontal ruler doesn't show anymore on the Scrolling scenario.

tig · 2024-04-15T23:26:29Z

That Window shouldn't be movable anyway.

Even if it was movable, this shouldn't happen. But if it is the Border's ColorScheme that is changed, it should not affect the CheckBox's ColorScheme.

The SetNeedsDisplay is causing the entire View to be Cleared and redrawn.

Regardless, it's not CheckBox that's causing the flicker. You are seeing the weird flickering thing. For you it just happens to be over Checkbox. for me it's over Centered.

tig · 2024-04-15T23:29:13Z

The horizontal ruler doesn't show anymore on the Scrolling scenario.

Sorry about that. I was debugging and hid some stuff. Fixed.

tig · 2024-04-15T23:30:46Z

I'm done trying to debug this flicker thing for now. So frustrating!

BDisp · 2024-04-15T23:31:17Z

The SetNeedsDisplay is causing the entire View to be Cleared and redrawn.

Regardless, it's not CheckBox that's causing the flicker. You are seeing the weird flickering thing. For you it just happens to be over Checkbox. for me it's over Centered.

You're right about that. By coincidence, for me, it appears on the line where the CheckBox is. I'm going to try changing the ConsoleDriver's Clip property so it can check multiple areas instead of one.

tig · 2024-04-15T23:35:43Z

The SetNeedsDisplay is causing the entire View to be Cleared and redrawn.
Regardless, it's not CheckBox that's causing the flicker. You are seeing the weird flickering thing. For you it just happens to be over Checkbox. for me it's over Centered.

You're right about that. By coincidence, for me, it appears on the line where the CheckBox is. I'm going to try changing the ConsoleDriver's Clip property so it can check multiple areas instead of one.

Let's move this discussion here:

#3408

tig · 2024-04-16T12:52:55Z

Now that this has been merged I encourage everyone to read the updated conceptual docs to ensure an understanding of the new model (and to provide feedback on the docs):

Layout - https://gui-cs.github.io/Terminal.GuiV2Docs/docs/layout.html
Drawing - https://gui-cs.github.io/Terminal.GuiV2Docs/docs/drawing.html
Mouse - https://gui-cs.github.io/Terminal.GuiV2Docs/docs/mouse.html
Keyboard - https://gui-cs.github.io/Terminal.GuiV2Docs/docs/keyboard.html
New in v2 -https://gui-cs.github.io/Terminal.GuiV2Docs/docs/newinv2.html

Also the API doc for Viewport etc...:

If you find issues please either just fix them in a PR or open a new Issue.

Reamed Bounds -> ViewPort to see how it feels

f6ff369

tig added 4 commits March 14, 2024 07:39

Reamed Bounds -> Viewport in comments

3a40dec

Reamed Bounds -> Viewport - more rename

0900333

Reamed Bounds -> Viewport - MOAR rename

b187a8e

Reamed Bounds -> Viewport test rename

ebc47de

tig marked this pull request as draft March 15, 2024 00:55

tig added 5 commits March 17, 2024 08:57

Updated API docs

ab021bf

Made View.Move and View.AddRune constrain; added unit tests

7767bd1

Fixed menu code that was using View.Move inproperly

0cde0e1

Merge branch 'v2_3269_Bounds-ContentArea' of tig:tig/Terminal.Gui int…

1549d0d

…o v2_3269_Bounds-ContentArea

Merge branch 'v2_3269_Bounds-ContentArea' of tig:tig/Terminal.Gui int…

9aec5b1

…o v2_3269_Bounds-ContentArea

tig added 2 commits March 18, 2024 09:42

Fixed API doc typo

d915c4e

Fixed Clipping scenario

00c4680

tig mentioned this pull request Mar 18, 2024

Fixes #2489. Build scrolling into View. #3254

Closed

12 tasks

tig added 4 commits March 18, 2024 16:05

Rebased with v2_develop

6b1a429

Added ContentSize property and first unit test

6f664c9

ViewportToScreen now deals with + location. Updated unit tests.

f55d9bb

More unit tests and code cleanup

fd4fdb8

Removed debug code

77f9830

Keep Force16Colors set across apps to ease debugging of flicker issue

50fcb19

Reimplemented Driver.FillRect to directly use Contnets vs calling Mov…

de1a844

…e/AddRune

Made Arrangement = ViewArrangement.Fixed the default for Toplevels

069089e

tig added 2 commits April 15, 2024 17:27

If Parent isn't movable, border now cancels Highlight event

b7547c4

Reverted debug stuff in Scrolling Scenario

ceaa10c

tig mentioned this pull request Apr 15, 2024

Long-standing repaint-flicker issue. #3408

Open

tig added 2 commits April 15, 2024 18:08

Test tweaks

97351c3

Console Driver code cleanup

8d2c64a

tig merged commit 56922b4 into gui-cs:v2_develop Apr 16, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #3169. `Bounds` -> `Viewport`: Content Scrolling in `View` #3323

Fixes #3169. `Bounds` -> `Viewport`: Content Scrolling in `View` #3323

tig commented Mar 14, 2024 •

edited

Loading

BDisp commented Mar 14, 2024

dodexahedron commented Mar 14, 2024

BDisp commented Mar 14, 2024

tig commented Mar 15, 2024

BDisp commented Mar 18, 2024 •

edited

Loading

tig commented Mar 18, 2024

tig commented Mar 18, 2024

BDisp commented Mar 18, 2024

BDisp commented Mar 18, 2024

tig commented Mar 20, 2024

tig commented Mar 20, 2024

BDisp commented Mar 20, 2024

BDisp commented Apr 15, 2024

BDisp commented Apr 15, 2024

BDisp commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024 •

edited

Loading

BDisp commented Apr 15, 2024

BDisp commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

BDisp commented Apr 15, 2024

BDisp commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

BDisp commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 16, 2024

Fixes #3169. Bounds -> Viewport: Content Scrolling in View #3323

Fixes #3169. Bounds -> Viewport: Content Scrolling in View #3323

Conversation

tig commented Mar 14, 2024 • edited Loading

Fixes

Proposed Changes/Todos

Changes that can be done after this is merged to finish the new system

BDisp commented Mar 14, 2024

dodexahedron commented Mar 14, 2024

BDisp commented Mar 14, 2024

tig commented Mar 15, 2024

BDisp commented Mar 18, 2024 • edited Loading

tig commented Mar 18, 2024

tig commented Mar 18, 2024

BDisp commented Mar 18, 2024

BDisp commented Mar 18, 2024

tig commented Mar 20, 2024

tig commented Mar 20, 2024

BDisp commented Mar 20, 2024

BDisp commented Apr 15, 2024

BDisp commented Apr 15, 2024

BDisp commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024 • edited Loading

BDisp commented Apr 15, 2024

BDisp commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

BDisp commented Apr 15, 2024

BDisp commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 15, 2024

BDisp commented Apr 15, 2024

tig commented Apr 15, 2024

tig commented Apr 16, 2024

Fixes #3169. `Bounds` -> `Viewport`: Content Scrolling in `View` #3323

Fixes #3169. `Bounds` -> `Viewport`: Content Scrolling in `View` #3323

tig commented Mar 14, 2024 •

edited

Loading

BDisp commented Mar 18, 2024 •

edited

Loading

tig commented Apr 15, 2024 •

edited

Loading