Another try at a viewport widget.

[jneem]

Here's another stab at a viewport widget. This one differs from the previous one in that it doesn't make ScrollComponent depend on the viewport widget. It does make other changes to ScrollComponent, though. Specifically, it separates the "viewport state" (which I've called RectPort) from the scrollbar state. The scroll component no longer stores the viewport state; in methods that need to modify the viewport state, it gets a mutable reference. The reason for this was that I wanted to make these modifications explicit for the caller, because the caller will typically need to do something in response to the viewport changing.

[ForLoveOfCats]

I see a few terminology things I'm not fond of and I can't do an in-depth review at this moment. However at first glance this seems to be a reasonable compromise!

[jneem]

If you have better naming suggestions, I'd be happy to hear them! Like, I know that RectPort is a bad name, but Viewport was already taken...

I'm also in no rush to merge.

[derekdreery]

Just saying thanks for all the work on a big PR.

[ForLoveOfCats]

I'm sorry I can't give a very thorough review at this moment but I wanted to give at least a bit of feedback. I don't have anything against the name of RectPort, what I was referring to are the references to scrolling in Viewport which doesn't line up with the potential usage cases.

[rjwittams]

I feel like there must be a better name than this but I can't think of one right now...

[cmyr]

We could call this Viewport and call the widget ClipContainer or something? just spitballing...

[ForLoveOfCats]

Viewport is rapidly becoming a very overloaded term. In some cases it can be the size of the window, in other situations it refers to the paint rect of a widget, and now we are adding two new things which each reference related topics with similar terminology. I don't have much against RectPort but I do like renaming the widget to ClipContainer or something else.

[jneem]

Hmm, ClipContainer is missing half the story for me, since I see this widget as being about both clipping and panning. I'm going to try sticking with Viewport for now and renaming RectPort to ViewportPos...

[cmyr]

Can you clarify how the distinction between clipping and panning? I think of panning as just a particular subjective experience of something being clipped. 🤔

[jneem]

I don't really have a very strong opinion on this, but when I hear "clip" I just think of the graphical operation. Like, even without a transformation, a ClipContainer might be able to clip a child so that it has rounded corners or something? That does admittedly sound pretty contrived...

[cmyr]

Cool, just curious. I think it's fairly common for UI toolkits to call this sort of thing a 'clip view'; at least, that's what it's called in AppKit.

[jneem]

@ForLoveOfCats Did I imagine it or did you leave some comments on some of the API docs? I could have sworn you did but I can't find it now, either here or in the old PR.

[ForLoveOfCats]

Nope, I've barely skimmed the updated doc comments

[ForLoveOfCats]

I've poked at this some more and while I feel good about this PR on the whole I do have a few concerns which I've voiced a bit earlier inline but I want to cover a bit more. However I don't want to block on these concerns alone.

I'm worried about the term "viewport". At this point we have quite a few meanings for the word and this PR adds another new meaning to it. Ironically in several locations in the impl for the Viewport widget it constructs a local rect variable called viewport. Furthermore the ViewportPos feels non-descriptive as it does not directly contain a position value (the closest is ViewportPos::rect.origin()) which leads to Viewport having the .pos(&self) method which does not return an actual position. Again I don't want to explicitly block on any of this. Naming things is hard and I don't really have any ready suggestions at this exact moment. My worry is the increased mental load of keeping track of these things and the non-specificity which the term viewport is heading towards.

[cmyr]

I agree that the names don't feel perfect. The only real suggestion I have is ClipBox for the widget, which gives us Viewport or the thing describing the visible region. 🤷

[ruffson]

I am really interested in using this. Are there any blockers?

[jneem]

No, I've just been busy on other things, sorry. I'll try changing the names to ClipBox/Viewport and see if that flies.

[ForLoveOfCats]

I feel more comfortable with the data structure being called Viewport over the widget. I do still have concerns as we generally refer to the paint rect as the "viewport" leading to code which touches two separate values with the name viewport but containing completely different data and different purposes mere lines apart. That said I do feel like clarifying our terminology in this area can be a separate conversation in the same vein as the discussions on resolution terminology.