Added functionality for pinching in touchpad settings (#4793)

[Jag-Marcel]

• I have read CONTRIBUTING.md
• This implements/fixes issue Provide pinch to setting that expands window into full screen #4793.

---

Description:

When the window is within 20 pixels of its largest size, it will now switch to fullscreen if the user pinches out. Likewise, it will move out of fullscreen when pinching in while in fullscreen.

• Not sure what the threshold should be, 2% of height seemed to work fine and felt responsive on a 13in macbook screen, alternatively something like 30 pixels of wiggle room could work

• May lead to confusion for users who don't want max size to turn to fullscreen; depending on experience it might make sense to make this functionality the main "Fullscreen" option and have "Adjust window size" be the version without fullscreen toggle (and have it be renamed to "Only adjust window size")

[low-batt]

Thank you for your contribution to IINA.

For GUI design I defer to the other developers. I'm unsure as to what their view will be on the questions you raised about how to add this functionality.

Currently we are working on a bug fix release. After that the plan is to start on the long planned release that is focused on adding functionality. Likely the other developers will not comment on this one for a while as we first have to finish the bug fix release.

[svobs]

Not sure what the threshold should be, 2% of height seemed to work fine and felt responsive on a 13in macbook screen, alternatively something like 30 pixels of wiggle room could work

Would it be possible to only activate it if, at the start of the pinch, the window was already using all of the screen's visible area (0px threshold)? Then pinching in from full screen would just go back to maximized, and would need a another pinch in to go smaller.. I think that would be the most useful/intuitive, because it's well defined and easy to control without needing to be too precise.

So if the window was small, the user would need to pinch twice: first to expand the window to be as large as possible in windowed mode, then again to enter full screen.

Some might be annoyed at having to pinch twice if the window is only a few pixels smaller than its max size. But I don't think that would be a frequent occurence. I think more would be unhappy if they wanted only to make the window larger and they end up activating full screen.

[Jag-Marcel]

Would it be possible to only activate it if, at the start of the pinch, the window was already using all of the screen's visible area (0px threshold)?

I tried it with a 0px threshold, but the window would never perfectly maximize. It would take me several tries to get it to the maximum size so it would switch to fullscreen, and at other times it would never work.
Might have to do with the fact that the window never exactly stays in the same place (it always slightly drifts to the bottom left if you pinch in and out repeatedly), may also be an unrelated issue.
I admit I don't know enough right now about how Swift works to fix this (tl: this is known as an "excuse"), so I guess the 2% threshold was just a band-aid fix, might be worth another issue on here.

(edit: figured out the fix, next commit has a working version without a threshold; weird drifting to the lower left corner still happens though)

[svobs]

(edit: figured out the fix, next commit has a working version without a threshold; weird drifting to the lower left corner still happens though)

Nice. Looks like there are a couple things still - take a look at my feedback.

I wouldn't worry about the window drifting over time. That looks like an existing bug.

[svobs]

Looks like it works well if the video's aspect ratio is tall enough to reach the top of the screen first. But I think you should also be checking whether the width reaches the sides of the screen. In other words, try:
if (window.frame.height >= screenFrame.height || window.frame.width >= screenFrame.width) && lastMagnification > 0 {
Otherwise it won't work for very wide videos which may run out of screen width before running out of screen height.

[svobs]

Can just delete this last else if. It will end up forcing the window to expand to fill both the width and height of the screen. But doing that will not honor the video's aspect ratio, which is not in line with how IINA currently works.

[svobs]

This looks like a bug in the original code, but I think you want to change all the < and > in this line to <= and >=. This is probably why you had problems getting the window to reach 100% height.

[Jag-Marcel]

Regarding your comments:

Looks like it works well if the video's aspect ratio is tall enough to reach the top of the screen first.But I think you should also be checking whether the width reaches the sides of the screen.

This first part is true, I didn't check for ultrawide video, the same thing applies to the if statement that controls the window resize where I've had to add a && newWidth <= screenFrame.width as a condition.
As for the other two,

This looks like a bug in the original code, but I think you want to change all the < and > in this line to <= and >=. This is probably why you had problems getting the window to reach 100% height.

This did help with getting the window closer to the edges of the screen, but was still very inconsistent as the threshold for the exact edges is very small.

Can just delete this last else if. It will end up forcing the window to expand to fill both the width and height of the screen. But doing that will not honor the video's aspect ratio, which is not in line with how IINA currently works.

Also true, I forgot about that, sorry. I can't completely remove the else if statement because of the issue I stated above, I now have two different implementations that both consider this behaviour, and I'm not sure which would be cleaner and/or more efficient:

if newHeight > screenFrame.height {
  newHeight = screenFrame.height
  newWidth = screenFrame.height * window.aspectRatio.aspect
}
if newWidth > screenFrame.width {
  newWidth = screenFrame.width
  newHeight = screenFrame.width / window.aspectRatio.aspect
}
if newHeight <= screenFrame.height && newWidth <= screenFrame.width && newHeight >= minSize.height && newWidth >= minSize.width {
  let newSize = NSSize(width: newWidth, height: newHeight)
  window.setFrame(window.frame.centeredResize(to: newSize), display: true)
}

(would mean that newWidth and newHeight would be changed to vars instead of lets)

if newHeight <= screenFrame.height && newWidth <= screenFrame.width && newHeight >= minSize.height && newWidth >= minSize.width {
  let newSize = NSSize(width: newWidth, height: newHeight)
  window.setFrame(window.frame.centeredResize(to: newSize), display: true)
} else if newHeight > screenFrame.height {
  let newSize = NSSize(width: screenFrame.height * window.aspectRatio.aspect, height: screenFrame.height)
  window.setFrame(window.frame.centeredResize(to: newSize), display: true)
} else if newWidth > screenFrame.width {
  let newSize = NSSize(width: screenFrame.width, height: screenFrame.width / window.aspectRatio.aspect)
  window.setFrame(window.frame.centeredResize(to: newSize), display: true)
}

Personally I'd rather go with the first option as I find it more readable, but both seem to work fine. I'll wait with the commit until I've heard your opinion, maybe there's another implementation I haven't thought of.

[svobs]

@Jag-Marcel Ah yes, you're right about needing to check for both dimension separately instead of all at once. Perhaps there is some elegant geometry theorem which we could rely on to make it more elegant, but I don't think it's too bad.

I agree; I like the first form you listed. And in fact, after your two bounds checks and any scaling they perform, I don't think it shouldn't be possible for the window not to fit completely inside the screen. So you can simplify the last if:

if newHeight > screenFrame.height {
  newHeight = screenFrame.height
  newWidth = screenFrame.height * window.aspectRatio.aspect
}
if newWidth > screenFrame.width {
  newWidth = screenFrame.width
  newHeight = screenFrame.width / window.aspectRatio.aspect
}
if newHeight >= minSize.height && newWidth >= minSize.width {
  let newSize = NSSize(width: newWidth, height: newHeight)
  window.setFrame(window.frame.centeredResize(to: newSize), display: true)
}

This made me realize another possible improvement, though it's out of scope for this PR: enforce minSize the same way. Currently if the gesture is requesting a window size which is smaller than either min width or min height, then setFrame() does not get called at all. But this can make it hard to get down to min window size because the user has to land on it directly, which usually results in the window being a little larger than min on the first pinch. But then you'd have to add two more checks. Not worth bothering about right now.

I pieced together code from the comments in order to test this out and it looks good to me. You should push an update to the PR! Although disclaimer: I am not part of the "core" IINA team and can't guarantee it will get merged into the official develop branch - but I hope it does :)

[svobs]

Latest version seems to work well. I wonder if the core devs might have an issue with augmenting Adjust window size to basically also do what Fullscreen does. I wonder if people would be opposed to combining the two options?

[anohren]

Latest version seems to work well. I wonder if the core devs might have an issue with augmenting Adjust window size to basically also do what Fullscreen does. I wonder if people would be opposed to combining the two options?

#4793 (comment)