Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redraw passed bounds bigger than its client area #2308

Closed
tznind opened this issue Jan 28, 2023 · 8 comments · Fixed by #2309
Closed

Redraw passed bounds bigger than its client area #2308

tznind opened this issue Jan 28, 2023 · 8 comments · Fixed by #2309

Comments

@tznind
Copy link
Collaborator

tznind commented Jan 28, 2023

Describe the bug
I've encountered this bug in developing #2258

After resizing a View in LayoutSubviews to be smaller then the subsequent redraw that is triggered is passing a larger Rect into Redraw than the bounds.

image

I think this issue is to do with view.NeedDisplay.Width being stale and the parent View passing the larger width/height of the two:

Width = Math.Max (view.Bounds.Width, view.NeedDisplay.Width),
Height = Math.Max (view.Bounds.Height, view.NeedDisplay.Height)

Surely a View should never be asked to redraw a bigger area than it currently occupies?

Or is this to do with menus/spill overs etc where a view deliberately paints outside the box? If so then I think quite a few of my views will need to be adjusted to only listen to Bounds and not the passed parameter bounds.

@BDisp
Copy link
Collaborator

BDisp commented Jan 28, 2023

Yes, that was done to ensure that a view on shrink clear all the previous bounds and not let some chunk around it. You mustn't rely on it but only on View.Bounds.

@tznind
Copy link
Collaborator Author

tznind commented Jan 28, 2023

Cool, is this a recent change in behavior then? Or was it always like that for a long time?

@BDisp
Copy link
Collaborator

BDisp commented Jan 28, 2023

Cool, is this a recent change in behavior then? Or was it always like that for a long time?

Was changed by me on 6 Dec 2021 with the commit 6d108fd.
Does your Git Blame is working on VS2022? It's beginning to seeking the information and end up with Unknown message.

@BDisp
Copy link
Collaborator

BDisp commented Jan 28, 2023

Use only the bounds parameter to clear the view before redraw it again using then the View.Bounds. You may not seen the issue caused by this behavior, if you have another view that redraw under the remaining space of the shrink view. Try override the Redraw and Text methods on a derived view. Set the Text with a long string. The Redraw only clear the view and call AddRune to write the text. Then shrink the width and you will see that the previous remaining text still persists on the screen. But for this, you must remove the changes I mad before.

@tznind
Copy link
Collaborator Author

tznind commented Jan 28, 2023

No worries, I think its just a nuance of the way I am doing resizing as part of layout. I will close this as its been this way for over a year and hasn't caused any problems in other use cases.

I can work around it I think.

@tznind tznind closed this as completed Jan 28, 2023
@BDisp
Copy link
Collaborator

BDisp commented Jan 28, 2023

@tznind I undo-ed that change and all the test are passed. I think I did this change to try fix an issue on clearing a label on shrink the width, but after the fix was introduced in this line:

Rect containerBounds = GetContainerBounds ();

Furthermore, the derived views that override the Redraw method must handle with the correct redraw of his view. In my opinion this change must be reverted and if you want you can reopen this issue.

@BDisp
Copy link
Collaborator

BDisp commented Jan 29, 2023

I just realized that setting the Width/Height causes the NeedDisplay to be the same as the new values. Contrary, setting the Frame causes the NeedDisplay to be the previous values. So, I think we must discuss how these values must be handled. In my opinion the NeedDisplay must be set to the previous values to provide to the user that the previous bounds was changed to allow perform some action. We really need to decide about this.

@tznind tznind reopened this Jan 29, 2023
@tznind
Copy link
Collaborator Author

tznind commented Jan 29, 2023

In my opinion this change must be reverted and if you want you can reopen this issue.

Ok reopening, sounds like theres things that could be addressed :) .

BDisp added a commit to BDisp/Terminal.Gui that referenced this issue Jan 29, 2023
@tig tig closed this as completed in #2309 Feb 5, 2023
tig added a commit that referenced this issue Feb 5, 2023
Fixes #2308. Redraw passed bounds bigger than its client area.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants