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.

Sign up for GitHub

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

Screensaver inhibitor. #1229

Merged
merged 19 commits into from
Apr 9, 2017
Merged

Screensaver inhibitor. #1229

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
7f76595
Initial version of screensaver inhibitor.
JosepMaJAZ Mar 25, 2017
0fb221a
buildfix mac
JosepMaJAZ Mar 25, 2017
7d8a0e1
buildfix mac
JosepMaJAZ Mar 25, 2017
1ec06ce
buildfix mac
JosepMaJAZ Mar 25, 2017
67d3641
Implemented user setting for enable/disable the inhibitor
JosepMaJAZ Mar 26, 2017
4b52ac0
Implemented inhibit on play
JosepMaJAZ Mar 26, 2017
21ce197
moved settings and logic to MixxMainWindow. Changed logic on ScreenSa…
JosepMaJAZ Mar 28, 2017
e7ae731
merge master
JosepMaJAZ Mar 28, 2017
1ae1af9
added code to trigger user activity to the ScreenSaverHelper class.
JosepMaJAZ Mar 28, 2017
e091467
UserActivity triggering from controllers. Independently of screensave…
JosepMaJAZ Mar 28, 2017
aa97512
several fixes and code reorganization
JosepMaJAZ Mar 29, 2017
5b810df
merge master
JosepMaJAZ Mar 29, 2017
5c0bb98
merge master
JosepMaJAZ Mar 30, 2017
411e6f8
that merge went really bad...
JosepMaJAZ Mar 31, 2017
7133d1c
fix order of loading setting. caused it to not be correct in the sett…
JosepMaJAZ Mar 31, 2017
d9a8fe7
Merge branch 'master' into prevent-screensaver
JosepMaJAZ Apr 1, 2017
72f7c37
fix for linux screensaver user activity and fallback method using xlib
JosepMaJAZ Apr 6, 2017
5b2f437
Merge branch 'prevent-screensaver' of https://github.com/JosepMaJAZ/m…
JosepMaJAZ Apr 6, 2017
305e697
use only xlib methodfor useractivity
JosepMaJAZ Apr 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion build/depends.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ def configure(self, build, conf):
raise Exception('Qt >= 5.0 not found')
elif not qt5 and not conf.CheckForPKG('QtCore', '4.6'):
raise Exception('QT >= 4.6 not found')


qt_modules.extend(['QtDBus'])
# This automatically converts QtXXX to Qt5XXX where appropriate.
if qt5:
build.env.EnableQt5Modules(qt_modules, debug=False)
Expand Down Expand Up @@ -1117,6 +1118,7 @@ def sources(self, build):
"util/logging.cpp",
"util/cmdlineargs.cpp",
"util/audiosignal.cpp",
"util/screensaver.cpp",

'#res/mixxx.qrc'
]
Expand Down
2 changes: 1 addition & 1 deletion src/mixer/playerinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class PlayerInfo : public QObject {
TrackPointer getTrackInfo(const QString& group);
void setTrackInfo(const QString& group, const TrackPointer& trackInfoObj);
TrackPointer getCurrentPlayingTrack();
int getCurrentPlayingDeck();
QMap<QString, TrackPointer> getLoadedTracks();
bool isTrackLoaded(const TrackPointer& pTrack) const;
bool isFileLoaded(const QString& track_location) const;
Expand Down Expand Up @@ -62,7 +63,6 @@ class PlayerInfo : public QObject {
void clearControlCache();
void timerEvent(QTimerEvent* pTimerEvent);
void updateCurrentPlayingDeck();
int getCurrentPlayingDeck();
DeckControls* getDeckControls(int i);

PlayerInfo();
Expand Down
39 changes: 37 additions & 2 deletions src/mixxx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "dialog/dlgabout.h"
#include "preferences/dialog/dlgpreferences.h"
#include "preferences/dialog/dlgprefeq.h"
#include "preferences/constants.h"
#include "dialog/dlgdevelopertools.h"
#include "engine/enginemaster.h"
#include "effects/effectsmanager.h"
Expand Down Expand Up @@ -64,6 +65,7 @@
#include "skin/launchimage.h"
#include "preferences/settingsmanager.h"
#include "widget/wmainmenubar.h"
#include "util/screensaver.h"

#ifdef __VINYLCONTROL__
#include "vinylcontrol/vinylcontrolmanager.h"
Expand Down Expand Up @@ -367,6 +369,18 @@ void MixxxMainWindow::initialize(QApplication* pApp, const CmdlineArgs& args) {
}
emit(newSkinLoaded());

// Inhibit the screensaver if the option is set.
int inhibit = pConfig->getValue<int>(ConfigKey("[Config]","InhibitScreensaver"),-1);
if (inhibit == -1) {
inhibit = static_cast<int>(mixxx::ScreenSaverPreference::PREVENT_ON);
pConfig->setValue<int>(ConfigKey("[Config]","InhibitScreensaver"), inhibit);
}
mixxx::ScreenSaverPreference inhiPref = mixxx::ScreenSaverPreference(inhibit);
if (inhiPref == mixxx::ScreenSaverPreference::PREVENT_ON) {
mixxx::ScreenSaverHelper::inhibit();
}


// Wait until all other ControlObjects are set up before initializing
// controllers
m_pControllerManager->setUpDevices();
Expand Down Expand Up @@ -433,11 +447,15 @@ void MixxxMainWindow::initialize(QApplication* pApp, const CmdlineArgs& args) {
m_pPlayerManager->slotLoadToDeck(musicFiles.at(i), i+1);
}
}

connect(&PlayerInfo::instance(),
SIGNAL(currentPlayingTrackChanged(TrackPointer)),
this, SLOT(slotUpdateWindowTitle(TrackPointer)));

connect(&PlayerInfo::instance(),
SIGNAL(currentPlayingDeckChanged(int)),
this, SLOT(slotChangedPlayingDeck(int)));

// this has to be after the OpenGL widgets are created or depending on a
// million different variables the first waveform may be horribly
// corrupted. See bug 521509 -- bkgood ?? -- vrince
Expand All @@ -451,7 +469,15 @@ void MixxxMainWindow::finalize() {
Timer t("MixxxMainWindow::~finalize");
t.start();

// Save the current window state (position, maximized, etc)
UserSettingsPointer pConfig = m_pSettingsManager->settings();
int inhibit = pConfig->getValue<int>(ConfigKey("[Config]","InhibitScreensaver"));
mixxx::ScreenSaverPreference inhiPref = mixxx::ScreenSaverPreference(inhibit);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This casing form is not easy recognizable as cast.
Can we switch to
auto inhiPref = static_castmixxx::ScreenSaverPreference(inhibit);
?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case we could also avoid casting to enum and cast to int below.
The line is not aware the new PREVENT_ON_PLAY feature.

So it may look finally like that:
if (inhiPref != static_cast<int>(mixxx::ScreenSaverPreference::PREVENT_OFF)) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use of "auto" and explicit cast sounds interesting. use of static_cast sounds in contradiction with the use of "enum class".
I would preffer the oppinion of someone else before changing that from what it is now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can keep it, if you like.

I have just looked in
https://google.github.io/styleguide/cppguide.html#Casting
In this case your enum class has no constructor so I think static_cast<> it the best choice here.
We only user auto rarely, except if it helps not to repeat ourselves like in this case.

if (inhiPref == mixxx::ScreenSaverPreference::PREVENT_ON) {
mixxx::ScreenSaverHelper::uninhibit();
}


// Save the current window state (position, maximized, etc)
m_pSettingsManager->settings()->set(ConfigKey("[MainWindow]", "geometry"),
QString(saveGeometry().toBase64()));
m_pSettingsManager->settings()->set(ConfigKey("[MainWindow]", "state"),
Expand Down Expand Up @@ -1079,6 +1105,15 @@ void MixxxMainWindow::slotNoMicrophoneInputConfigured() {
m_pPrefDlg->showSoundHardwarePage();
}

void MixxxMainWindow::slotChangedPlayingDeck(int deck) {
UserSettingsPointer pConfig = m_pSettingsManager->settings();
int inhibit = pConfig->getValue<int>(ConfigKey("[Config]","InhibitScreensaver"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not parse the preferences over and over again.
Please save inhibit as member variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I will need a slot to call when I change the setting from the preferences.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a direct call, because the DlgPrefControls holds a pointer to MixxxMainWindow.

mixxx::ScreenSaverPreference inhiPref = mixxx::ScreenSaverPreference(inhibit);
if (inhiPref == mixxx::ScreenSaverPreference::PREVENT_ON_PLAY) {
mixxx::ScreenSaverHelper::inhibitOnCondition(deck!=-1);
}
}

void MixxxMainWindow::slotHelpAbout() {
DlgAbout* about = new DlgAbout(this);
about->show();
Expand Down
1 change: 1 addition & 0 deletions src/mixxx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class MixxxMainWindow : public QMainWindow {
void slotDeveloperToolsClosed();

void slotUpdateWindowTitle(TrackPointer pTrack);
void slotChangedPlayingDeck(int deck);

// Warn the user when inputs are not configured.
void slotNoMicrophoneInputConfigured();
Expand Down
7 changes: 7 additions & 0 deletions src/preferences/constants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ enum class TooltipsPreference {
TOOLTIPS_ONLY_IN_LIBRARY = 2,
};

// Settings to enable or disable the prevention to run the screensaver.
enum class ScreenSaverPreference {
PREVENT_OFF = 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INHIBIT_OFF sounds more natural to me, though I am not a native speaker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, to me it's the word inhibit that isn't natural. I'm using it because that's the name used in the linux part of the solution and I kept that name.
http://dictionary.cambridge.org/dictionary/english/prevent
http://dictionary.cambridge.org/dictionary/english/prevent

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked google and you are probably right:
"prevent screensaver" 4550 Hits
"inhibit screensaver" 1020 Hits

PREVENT_ON = 1,
PREVENT_ON_PLAY = 2
};

} // namespace mixxx

#endif /* PREFERENCES_CONSTANTS_H */
46 changes: 45 additions & 1 deletion src/preferences/dialog/dlgprefcontrols.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
#include "skin/skinloader.h"
#include "skin/legacyskinparser.h"
#include "mixer/playermanager.h"
#include "mixer/playerinfo.h"
#include "control/controlobject.h"
#include "mixxx.h"
#include "util/screensaver.h"
#include "defs_urls.h"

DlgPrefControls::DlgPrefControls(QWidget * parent, MixxxMainWindow * mixxx,
Expand Down Expand Up @@ -305,6 +307,21 @@ DlgPrefControls::DlgPrefControls(QWidget * parent, MixxxMainWindow * mixxx,
ConfigKey("[Config]", "StartInFullscreen")).toInt()==1);
connect(checkBoxStartFullScreen, SIGNAL(toggled(bool)),
this, SLOT(slotSetStartInFullScreen(bool)));

//
// Screensaver mode
//
comboBoxScreensaver->clear();
comboBoxScreensaver->addItem(tr("Allow screensaver to run"),
static_cast<int>(mixxx::ScreenSaverPreference::PREVENT_OFF));
comboBoxScreensaver->addItem(tr("Prevent screensaver from running"),
static_cast<int>(mixxx::ScreenSaverPreference::PREVENT_ON));
comboBoxScreensaver->addItem(tr("Prevent screensaver while playing"),
static_cast<int>(mixxx::ScreenSaverPreference::PREVENT_ON_PLAY));

int inhibitsettings = pConfig->getValue<int>(ConfigKey("[Config]","InhibitScreensaver"));
comboBoxScreensaver->setCurrentIndex(comboBoxScreensaver->findData(inhibitsettings));

//
// Tooltip configuration
//
Expand Down Expand Up @@ -456,6 +473,10 @@ void DlgPrefControls::slotResetToDefaults() {
// Don't start in full screen.
checkBoxStartFullScreen->setChecked(false);

// Inhibit the screensaver
comboBoxScreensaver->setCurrentIndex(comboBoxScreensaver->findData(
static_cast<int>(mixxx::ScreenSaverPreference::PREVENT_ON)));

// Tooltips on everywhere.
radioButtonTooltipsLibraryAndSkin->setChecked(true);

Expand Down Expand Up @@ -493,7 +514,6 @@ void DlgPrefControls::slotSetRateRange(int pos) {
slotSetRateRangePercent(ComboBoxRateRange->itemData(pos).toInt());
}


void DlgPrefControls::slotSetRateRangePercent (int rateRangePercent) {
double rateRange = rateRangePercent / 100.;

Expand Down Expand Up @@ -681,6 +701,30 @@ void DlgPrefControls::slotApply() {

int configSPAutoReset = BaseTrackPlayer::RESET_NONE;

// screensaver mode update
int inhibitcombo = comboBoxScreensaver->itemData(
comboBoxScreensaver->currentIndex()).toInt();
int inhibitsettings = m_pConfig->getValue<int>(ConfigKey("[Config]","InhibitScreensaver"));
if (inhibitcombo != inhibitsettings) {
m_pConfig->set(ConfigKey("[Config]", "InhibitScreensaver"), ConfigValue(inhibitcombo));

mixxx::ScreenSaverPreference inhiOld = mixxx::ScreenSaverPreference(inhibitsettings);
if (inhiOld == mixxx::ScreenSaverPreference::PREVENT_ON) {
mixxx::ScreenSaverHelper::uninhibit();
} else if (inhiOld == mixxx::ScreenSaverPreference::PREVENT_ON_PLAY) {
mixxx::ScreenSaverHelper::inhibitOnCondition(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we call a plain uninhibit in this case as well?
Maybe we need to move the check if it is already uninhibited inside the mixxx::ScreenSaverHelper::uninhibit();
and simplify this code here much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conceptually yes, but in practice no.
Some of the implementations require that you call the pair inhibit as many times as uninhibit. (as in, calling inhibit twice and unhinibit once would not uninhibit. So just in case I try to keep that).

I probably need to find a better name for this method. What it really does is either inhibit or uninhibit, depending if the parameter is the same or different than the current status. like this:

Current status: inhibit on
inhibitOnCondition(true) -> does nothing
inhibitOnCondition(false) -> unhinibit.
Current status: inhibit off
inhibitOnCondition(true) -> inhibit
inhibitOnCondition(false) -> does nothing

The end result is always that true leaves it enabled and false leaves it disabled, but internally it decides if it has to change anything or not.

But now that I think about it, I'm not sure why i needed it as separate method. Will check and comment later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I ended incorporating the logic inside inhibit and uninhibit.

}

mixxx::ScreenSaverPreference inhiNew = mixxx::ScreenSaverPreference(inhibitcombo);
if (inhiNew == mixxx::ScreenSaverPreference::PREVENT_ON) {
mixxx::ScreenSaverHelper::inhibit();
} else if (inhiNew == mixxx::ScreenSaverPreference::PREVENT_ON_PLAY) {
mixxx::ScreenSaverHelper::inhibitOnCondition(
PlayerInfo::instance().getCurrentPlayingDeck()!=-1);
}
}


if (m_speedAutoReset && m_pitchAutoReset) {
configSPAutoReset = BaseTrackPlayer::RESET_PITCH_AND_SPEED;
}
Expand Down
Loading