-
-
Notifications
You must be signed in to change notification settings - Fork 276
Allow attaching overlay window to specified background element rather than the VideoView #393
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
base: 3.x
Are you sure you want to change the base?
Conversation
Thanks for your contribution.
I guess that's a breaking change for people who are doing this already, but it's probably niche and acceptable behavior change.
Understood. This would be worth a note in https://code.videolan.org/videolan/LibVLCSharp/-/blob/3.x/src/LibVLCSharp.WPF/README.md, I believe, as well as how to use this feature otherwise it will likely remain unknown to users. Maybe I'll add a sample later. |
That won't be a problem since BackgroundElement is added by the commit, it didn't exist previously.
That sounds like a good idea. If you have any questions feel free to let me know. |
Allow attaching overlay window to specified background element rather than it being constrained to the video dimensions. This means the foreground window overlay is a predictable position and size and makes overlaying controls on top of the video much more intuitive.
Could you please add an I've tried modifying Example1 with your changes but no text is shown.
|
I rebased your branch FYI (no merge commits allowed). I will rebase again 3.x when about to merge, you don't need to worry about it. |
Description of Change
Allow the foreground window to be "attached" to a specified background element (copying its position and size) rather than being constrained by the VideoView position and size.
This makes the position and size of the foreground window overlay much more predictable so that controls in the overlay are always in the same place relative to the main window even if the position and dimensions of the VideoView changes.
If VideoView.BackgroundElement is not specified, the behavior is as before, so this change should not break anything.
It's only a minor change to code but is a big improvement to usability in my humble opinion.
Take my case as an example. My application is a fancy image gallery with video playback functions. The image zoom and drag functions in my app also work for videos, but they affect the foreground window, which is undesired behavior. So for cases such as mine being able to attach the foreground window overlay to a different control rather than the VideoView is necessary.
This pull request concerns only LibVLCSharp.WPF since that is what I use but other platforms may benefit from similar changes.
Issues Resolved
Variability in location/size of foreground overlay which is sometimes undesirable.
API Changes
Added:
FrameworkElement? BackgroundElement { get; set; }
Platforms Affected
WPF
Behavioral/Visual Changes
There is no change in functionality or appearance unless VideoView.BackgroundElement is set to a control. If set, the position and size of the ForegroundWindow (and consequently the child controls of the VideoView) will follow that background element, rather than following the VideoView. The video is not affected, only the foreground window.
Before/After Screenshots
Testing Procedure
For this test, I used the following XAML, which resembles the layout in my actual application, but greatly simplified.
Fixed width and height of the VideoView is used here for demonstration purposes.
Without changing BackgroundElement, the window will look like the first screenshot (same behavior as current LibVLCSharp.WPF)
By setting BackgroundElement in the window's constructor (or in Window_Initialized), as such:
The window becomes like the second screenshot.
It seems important that this is set in the constructor or in Window_Initialized, otherwise VideoView.Visibility must start out Collapsed and the property must be set before the VideoView is shown. Otherwise the change does not take. It seems that the ForegroundWindow is being created as soon as the VideoView is fully loaded. So that is one small but important caveat to keep in mind. Making it recreate (or modify) the ForegroundWindow whenever BackgroundElement is changed would be possible, but would require more extensive code changes. I've tried to keep my code changes to a minimum.
For the test it is not necessary to play any video or even initialize the core, the difference is apparent with just that one line of code.
PR Checklist