Move implementation of padding to DxRenderer

[mcpiroman]

Summary of the Pull Request

Moved padding from TermControl to DxRenderer. It solves problems with handling pointer input at padded area, since now this is the same control as the text is (#1367). Since this removes padding from _root element, scrollbar now sticks to the right edge (#1365).

It allows the renderer to draw on that area, which can be now somehow used. Specifically, it prevents potential effects from being cut on the char grid edge, when there is still some space for them (like selection outlining, which I would like).

References

PR Checklist

• Closes When using padding, starting selection from empty area continues previous selection #1367, Scrollbar is inset by padding and probably shouldn't be #1365
• CLA signed. If not, go over here and sign the CLA
• Tests added/passed
• Requires documentation to be updated
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

• I implemented this only on DxRendere since padding is only used there.
• It removes support for floating-point padding, but playing with floating-point pixel dimensions in such cases seems only to cause trouble.

Validation Steps Performed

Set padding, click or select both inside and outside padding, change padding at rutime.

[DHowett-MSFT]

I'm wondering if the better place to handle this is in sizing the SwapChainPanel -- the renderer will happily render into a box of any size, and it'll be safer in general to hold back too many renderer changes 😄

(EDIT)

Specifically, it prevents potential effects from being cut on the char grid edge, when there is still some space for them

This is a good point though!

[DHowett-MSFT]

@miniksa has an opinion here, i'm sure.

[miniksa]

I appreciate the concept here, but all I have is:

1. I haven't been in the renderer code recent enough to make a good call here
2. I don't want to make the wrong call here given that I don't have the renderer in cache
3. I don't think anyone else has rendering in their brain cache either
4. Adding more features to the renderer is going to make it harder for me to move things around when I do get back to doing bulk-renderer-work including the partial redraw region issue

So I am inclined to say "no not right now" because I don't want to make my own life harder when I finally do have time to do more bulk rendering work. But I also don't want to shun something perfectly good.

[zadjii-msft]

This seems superficially fine to me, though I kinda want to know the benefits of this one over #1778 which looks like the same-ish thing.

[mdtauk]

Wouldn't applying the padding to the Swapchain's Margin, also affect the background colour, and any Acrylic brushes on them?

As opposed to changing the position of the character/glyph rendering? Or have I misunderstood it?

[mcpiroman]

@zadjii-msft There are 2 benefits:

1. Right now events that occure outside the _swapChainPanel, this is on the 'padded' area, aren't handled by it (which causes When using padding, starting selection from empty area continues previous selection #1367). Either with padding or margin. So the events might be handled by _root. It was the case, but got changed becouse it caused problems with scrollbar that doesn't handle all its events and which therefore propagate to _root. So there might be another element in the UI tree, inbetween root and _swapChainPanel, which handles events and has its own padding. But:

2. Specifically, it prevents potential effects from being cut on the char grid edge, when there is still some space for them

I started some drafts on selection outlining and the 2. point was the problem. That's why I've choosen renderer solution over xaml one.

[miniksa]

I'd like to take #1778 now and hold this one until a bit later when I get a chance to go deeper into the DX renderer. No slight to you, I hear your arguments why it should be on this side. I just feel more comfortable with the smaller change to the higher level at this point in time.

[DHowett-MSFT]

I'm going to close this one out for now, but that isn't because we're not considering this approach. We're planning on tackling renderer changes in the medium-term future. 😄