zachsaw / MPDN_Extensions

Media Player .Net (MPDN) Open Source Extensions
Other
58 stars 18 forks source link

Minor fixup and add OSD config settings #237

Closed Garteal closed 7 years ago

Garteal commented 7 years ago

The OSD always stayed on-screen because the MouseMove event keeps firing if the pointer is visible on the frame even if you don't move your mouse. This fixes that issue. I've also added the ability to enable or disable the OSD through the settings. By default it is disabled.

I also tried to make this not dependent on the Playlist, but apparently a lot of extensions rely on it, so I guess it doesn't really matter. (Nvm, its out of scope for this PR)

Garteal commented 7 years ago

@Shiandow I prefer not to bind any events if something is not enabled, but doing it this way requires you to restart the player. What do you think?

Also is there a way to request (or force if you will) a repaint? Or maybe another PainterEvent I could use for the OSD instead? I mentioned it before, but If you pause, it repaints at a much lower rate which makes the GUI look unresponsive.

Also you can merge this if you're OK with it.

Shiandow commented 7 years ago

Hi Garteal,

So 'MouseMove' keeps firing even if the mouse is not in fact moving? That's probably not supposed to happen.

In fullscreen it seems to repaint the OSD more often. I suppose I could try to keep painting at a 'high' in frequency windowed mode, at least when MPDN has focus (we wouldn't want to use too much resources when MPDN is running in the background).

To avoid having to restart the player to enable / disable the OSD, it might be better to create "BindEvents()" and "UnBindEvents()" methods, and execute those when the settings have been changed (you probably want to add some safeguards to make sure it doesn't bind things twice).

Of course that means you need some way of checking whether the Settings have changed. One method would probably be to add some events to the "OnScreenDisplaySettings" that trigger when "Enabled" is changed to false or true. Alternatively I suppose the Framework could be extended to give all PlayerExtensions (or technically all ExtensionUis) a "SettingsChanged" event, that triggers whenever the settings are changed. Let me know which you prefer.

Shiandow commented 7 years ago

Oh, by the way, you shouldn't have to handle the save button and close button events manually, you just need to set the DialogResult field of the corresponding buttons.

Garteal commented 7 years ago

So 'MouseMove' keeps firing even if the mouse is not in fact moving? That's probably not supposed to happen.

Yeah, I recall it happening all of a sudden. I suspect some funky thing going on with the NVIDIA drivers. Currently on 381.65.

In fullscreen it seems to repaint the OSD more often. I suppose I could try to keep painting at a 'high' in frequency windowed mode, at least when MPDN has focus (we wouldn't want to use too much resources when MPDN is running in the background).

Yup sounds good. Maybe only when focused?

[...] Of course that means you need some way of checking whether the Settings have changed. One method would probably be to add some events to the "OnScreenDisplaySettings" that trigger when "Enabled" is changed to false or true. Alternatively I suppose the Framework could be extended to give all PlayerExtensions (or technically all ExtensionUis) a "SettingsChanged" event, that triggers whenever the settings are changed. Let me know which you prefer.

Yeah, a SettingsChanged event would be very nice to have. I also wanted to use it with the Playlist but used a dirty workaround to get it working.

Oh, by the way, you shouldn't have to handle the save button and close button events manually, you just need to set the DialogResult field of the corresponding buttons.

I'll change that if I can get VS2017 to cooperate with me lol. It was creating unnecessary files, etc, so I just did it manually for now >.<

Also I had to get back into the project. A few things are undocumented and it took me a bit to figure out why the settings weren't saving.

Shiandow commented 7 years ago

Yeah, I recall it happening all of a sudden. I suspect some funky thing going on with the NVIDIA drivers. Currently on 381.65.

Could be something windows did as well. Lately they seem to have been doing incredibly odd things to .net on windows 10.

Yup sounds good. Maybe only when focused?

That's what I was thinking. Shouldn't be any harm in using a few extra CPU cycles when it still has focus.

Also I had to get back into the project. A few things are undocumented and it took me a bit to figure out why the settings weren't saving.

Yeah, and I think the settings dialog kind of relies on the caller to do the right thing, which isn't ideal. I'll see if I can do something about that real quick.

Shiandow commented 7 years ago

Shall I make the settingsdialog throw an error when it's closed without setting "DialogResult"? At least when the code is in debug mode.

Turns out that's not really possible, at least not easily.