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

Rework OS screen saver handling #12342

Merged
merged 10 commits into from
Jun 27, 2017
Merged

Conversation

yol
Copy link
Member

@yol yol commented Jun 23, 2017

Replace the WinSystemBase OS screen saver interface

Description

Current Kodi OS screen saver handling is not adapted to modern desktop windowing systems (which mostly use inhibitors instead of simulated activity) and it's unclear what it's trying to achieve. Some options might have been lost in time, especially on X11. New approach discussed with @FernetMenta:

  • Default Kodi screensaver is unset when windowing systems announces support for OS screensaver, screensaver.xbmc.builtin.dim otherwise (embedded systems)
  • OS screensaver is not reset periodically, but inhibited and uninhibited to match how modern desktop environments handle this
  • OS screensaver is inhibited
    • at all times when a Kodi screensaver is set
    • only during video playback or fullscreen music visualization (i.e. when the Kodi screensaver would be inhibited) when no Kodi screensaver is set
  • New separate screensaver interface so implementations can be shared between windowing systems (e.g. based on standardized DBus interfaces for x11/mir/wayland on Linux), similar to video clock implementations

from yol#7 (comment)

Motivation and Context

I looked into screen saver handling for the Wayland windowing implementation and noticed while reading the OSX and X11 code that the current state is quite inconsistent.

How Has This Been Tested?

X11 backend tested with various setting combinations.

OSX is untested and really not my area of expertise, so any feedback would be much appreciated.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki (not sure)
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed


uint32_t sdlFlags = 0;

#if defined(TARGET_DARWIN_OSX)

This comment was marked as spam.

CLog::LogF(LOGNOTICE, "Setup SDL");

/* Clean up on exit, exit on window close and interrupt */
std::atexit(SDL_Quit);

This comment was marked as spam.

@yol
Copy link
Member Author

yol commented Jun 26, 2017

Rebased on #12353 so it builds on all platforms (hopefully)

@Rechi
Copy link
Member

Rechi commented Jun 26, 2017

OSX built an older commit
jenkins build this please

@yol
Copy link
Member Author

yol commented Jun 26, 2017

Build looks OK now - opinions?

@FernetMenta
Copy link
Contributor

+1

@Rechi
Copy link
Member

Rechi commented Jun 26, 2017

please rebase on master to get rid of the commits from #12353 in this PR

pkerling added 10 commits June 27, 2017 09:33
This isn't C
This changes the rules and functionality of the OS screen saver
inhibition to be more in line in with how modern desktop environments
work. Constant resetting of the screen saver is not necessary usually.
If it is, the platform-specific backend will have to implement it.
On desktop machines (that have an OS screen saver), the default should
be to use that. If the user then chooses to activate the Kodi screen
saver, this should completely override the OS screen saver as far as
permitted by the desktop environment. During video playback etc.,
the OS screen saver must always be inhibited regardless of whether
the Kodi screen saver is enabled.
In embedded environments (that do not have an OS screen saver), the
default should be to use the Kodi screen saver.
@yol yol force-pushed the screensaver-rework branch from 889758d to 14e9359 Compare June 27, 2017 07:33
@yol
Copy link
Member Author

yol commented Jun 27, 2017

rebased

@Rechi
Copy link
Member

Rechi commented Jun 27, 2017

jenkins build this please

2 similar comments
@MartijnKaijser
Copy link
Member

jenkins build this please

@Rechi
Copy link
Member

Rechi commented Jun 27, 2017

jenkins build this please

@MartijnKaijser MartijnKaijser added this to the L 18.0-alpha1 milestone Jun 27, 2017
@MartijnKaijser MartijnKaijser added Type: Improvement non-breaking change which improves existing functionality v18 Leia labels Jun 27, 2017
@MartijnKaijser MartijnKaijser merged commit dcb0e22 into xbmc:master Jun 27, 2017
@yol yol deleted the screensaver-rework branch June 27, 2017 10:57
@yol yol restored the screensaver-rework branch June 29, 2017 13:52
@yol yol deleted the screensaver-rework branch June 29, 2017 13:52
@h324
Copy link

h324 commented Jul 11, 2017

This change seems to be causing Kodi on my Ubuntu 16.04 system to kill the screen after a few minutes. This never happened before. I didn't change anything about my OS or Kodi settings other than upgrading to a newer nightly.

@yol
Copy link
Member Author

yol commented Jul 11, 2017

What exactly do you mean it "kills" your screen?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants