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

PiP: Mask window #1192

Closed
wants to merge 14 commits into from
Closed

PiP: Mask window #1192

wants to merge 14 commits into from

Conversation

JoseExposito
Copy link
Member

@JoseExposito JoseExposito commented Aug 5, 2021

Description

Work in progress PR to fix #658

The current implementation of PiP has some usability issues:

  • You only get one shot; if you mess up, you have to close PiP and start over
  • The original window sticks around, meaning you have two copies of the content
  • It's unclear what you're supposed to do when you hit the shortcut
  • Only works on the active window, if you select a different window, you end up with a transparent PiP
  • It get's activate event if it is not possible to use it. For example, on the desktop

Status

  • Do not trigger PiP if it is not possible to use it. For example, on the desktop
  • Draw the selection area as mask over the target window
  • Allow to resize the selection area within the limits of the target window
  • Allow to drag & drop the selection area within the limits of the target window
  • Exit PiP clicking outside the selection area
  • Capture PiP when the enter key is pressed
  • Capture PiP with a confirm button in the middle of the selection area
  • Change mouse pointer icons
  • Hide window areas outside of the mask
  • Don't display the PiP and the original window at the same time
  • Remove unused code (the MIN_SELECTION_SIZE branch in pip/Main.vala)
  • Design & icons (cc @cassidyjames)

@JoseExposito JoseExposito force-pushed the pip-mask branch 2 times, most recently from d2ab9eb to 468d4ea Compare August 18, 2021 15:40
@JoseExposito JoseExposito marked this pull request as ready for review August 18, 2021 15:41
@JoseExposito
Copy link
Member Author

Hey @cassidyjames, I finished the functional changes but there are some UI elements I hope you can help me with:

  • The icon in data/confirm.svg is a random icon. I'm sure you'll be able to draw something good... I just don't have talent
  • In SelectionArea.vala you can play with the constants MIN_SELECTION and CONFIRM_BUTTON_SIZE
  • Colours are set in draw_area in case you want to change them

Feel free to push the changes and I'll squash them into the right commit so we keep the git log useful.

I'll request a code review from desktop devs once UX have checked it 😉

@JoseExposito JoseExposito requested a review from a team August 18, 2021 15:52
@cassidyjames cassidyjames self-assigned this Aug 18, 2021
@cassidyjames
Copy link
Contributor

cassidyjames commented Aug 25, 2021

@JoseExposito hey, this is pretty neat! It feels like it's only partway there, though:

When cropping the window, can we hide the areas outside of the crop box? So it feels more like we're resizing the bounds of the window in on itself? See this motion mockup: https://www.youtube.com/watch?v=SDJYnZlqdn4

After confirming, can we hide the "original" window? I'm not sure what the implications are here regarding Alt+Tabbing, Multitasking View previews, etc. though. I would imagine the original window would not be shown in any of those contexts, but understand if that's difficult.

But ideally with this design, it should feel like PiP is actually resizing/cropping the original window, and then that window becomes the PiP window. Let me know if that makes sense or not. I can also give a try at an updated and more complete motion mockup if that would help.

@JoseExposito
Copy link
Member Author

I can also give a try at an updated and more complete motion mockup if that would help.

@cassidyjames don't worry, that mock up is pretty good, I'll see how to implement those 2 missing bits.

@JoseExposito
Copy link
Member Author

When cropping the window, can we hide the areas outside of the crop box? So it feels more like we're resizing the bounds of the window in on itself?

Hey @cassidyjames, I added a extra commit (pip: Hide window areas outside of the mask) with this functionality. I find it difficult to understand, but it probably is because I'm used to the old way. Have a look and let me know what you think.

After confirming, can we hide the "original" window? I'm not sure what the implications are here regarding Alt+Tabbing, Multitasking View previews, etc. though

I know this is technically possible because on X11 the list of windows managed by the window manager is stored in _NET_CLIENT_LIST. So, removing it from the list should make it disappear from the Alt+Tab switcher, multitasking view, remove the red dot on the dock, etc.

However, I've checked Mutter docs and I've not been able to figure out how to do this. Hiding the window actor (i.e., making it invisible) doesn't work really well when the window is focused, and it leads to weird/broken situations.

If you want to test it, comment this line:
https://github.com/elementary/gala/pull/1192/files?file-filters%5B%5D=.vala#diff-14f5d63441cc9f8fdccc8b33fac80dcafa3f7c89047481881e7dd7ba7759bce8R293
And you'll get an idea of how it (doesn't) work.

Unless I missed an important bit of Mutter's documentation, I think this is not possible to do in a reliable and bug-free way.

@cassidyjames
Copy link
Contributor

@JoseExposito another option could be to leave the window in those contexts, and automatically exit PiP if it is selected to be focused. That way it's still easy to switch back in the normal flow of things and you don't get into a weird state where it feels like it is two separate windows.

In order to allow to select the PiP area using a mask, set the active
actor in the SelectioArea contructor.

Also, avoid showing the selection UI if there is no active actor.
Change the selection area draw_area method to draw a full-screen colored
rectangle with a smaller transparent rectangle inside and 4 circles as
resize handlers.
When the user clicks on one of the resize handlers (+/- a threshold)
allow to resize the selection area within the limits of the target
window.
When the user clicks inside the selection area, allow to drag & drop it
within the limits of the target window.
Move the logic to capture the selected area to its own method.

Refactor, no functional changes.
Add a button in the middle of the selection mask to confirm the
selection.
While resizing or dragging the selection mask, clip the window behind it
to hide the areas that are outside of the selection mask.
@JoseExposito
Copy link
Member Author

another option could be to leave the window in those contexts, and automatically exit PiP if it is selected to be focused. That way it's still easy to switch back in the normal flow of things and you don't get into a weird state where it feels like it is two separate windows.

@cassidyjames we have 2 representations of the window. The Meta.Window, that is the actual window and the Meta.WindowActor, that is what we draw on screen.

If we hide the actor, the actual window is still on screen and you can interact with it, even though you can not see it.

As far as I know, the only way to really hide the window is minimizing it. I though 65586b0 should work, and it does with GTK apps, but not with Chrome. It seems like Chrome is optimized to stop repainting while minimizing, so it is not possible to watch YouTube videos in PiP.

I'm going to revert the patch... As far as I know, this is possible at a lower level, but not using Mutter's API 😕

Let's see if @tintou knows a way of doing it 🤞

The min selection logic is not used anymore, remove it.
@cassidyjames
Copy link
Contributor

@JoseExposito ah, dang. 😢 I suppose another (probably even more hacky?) method could be to put the window on a hidden workspace, or something?

@JoseExposito
Copy link
Member Author

@cassidyjames I believe that even if we hide the workspace from the multitasking view, it'd be possible to navigate to it by using shortcuts... It's the "problem" with using an opinionated API like Mutter instead of an unopinionated one like wlroots. It's great, simplifies things a lot and we get a ton of features for free, but sometimes, we are forced to follow their rules.

This case is especially painful, gtk_window_hide does exactly what we want, Mutter has an API to check if a window is hidden but, it won't allow us to hide a window 😕

Compositor.hide_window looked for a moment like the API to use, but look how pointer events still affect the window. Even tooltips are displayed!

OUT.mp4

I'll try one last hack: Adding an invisible actor that grabs every event on the top of the window. If that fails, I'll quit Gala development xD

@cassidyjames
Copy link
Contributor

@JoseExposito ha, I look forward to your continued experiments. This feels so close UX-wise! And in the end if it doesn't work out, we could put it on the back burner and consider revisiting it if the Mutter API improves.

@JoseExposito
Copy link
Member Author

@cassidyjames it's official, now I have to quit 🤣

The actor approach didn't work. In case someone looks into this, window.shadow, like minimize, stops YouTube videos, so it doesn't work either.

The best we can do is sending the PiP window to the bottom of the window stack so at least looses the focus.

I still think that the mask is easier to use than the current approach. Let me know if you want to merge that part (after adjusting styles, of course). Otherwise I'll close the PR.

@danirabbit danirabbit mentioned this pull request Oct 22, 2021
2 tasks
@cassidyjames
Copy link
Contributor

@JoseExposito so, I'm hesitant about switching to this design if we don't have a way to hide (and stop passing inputs to) the underlying window. If we're unable to do that for now, I think we need to:

  1. Keep the existing design from master, or
  2. Update this to not actually crop the window when you're positioning the PiP area, as it feels weird that it goes away and then comes back.

Maybe it's just best to punt on this for now…

@JoseExposito
Copy link
Member Author

@cassidyjames as you prefer. Option 2 is as easy as dropping the "pip: Hide window areas outside of the mask" commit, in case you want to test it.

If you prefer to stick with the current selection mode, feel free to close this merge request. We can always reopen it in the future.

@cassidyjames
Copy link
Contributor

@JoseExposito I think I'd rather punt on this and close it for now—it feels really close but I fear a halfway-there implementation will feel worse than either our current implementation or an ideal one. Happy to get feedback from @elementary/ux if anyone feels differently, though.

@JoseExposito
Copy link
Member Author

No problem, let's close it for now then.

@danirabbit
Copy link
Member

Should this be "draft" instead of "closed"? Typically closed branches end up getting deleted

@JoseExposito
Copy link
Member Author

Up to you. As far as I know, there is no API to implement the task "Don't display the PiP and the original window at the same time" in Mutter (main branch) at the moment. If we don't want to implement the mask selection, this PR is going to be hanging on GitHub for a long time.

@tintou tintou deleted the pip-mask branch June 14, 2023 13:01
@lenemter lenemter mentioned this pull request Dec 6, 2024
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 this pull request may close these issues.

Make Picture-in-Picture "mask" the window instead of drawing a region
3 participants