Skip to content

Commit

Permalink
Replace global access to Settings with a reference
Browse files Browse the repository at this point in the history
A dependency is the use of a variable in another part of Kodi. These can
be identified by searching StereoscopicsManager.cpp for all globals
(starting with g_) and external services (starting with CServiceBroker::).

Here is a list of all dependencies in StereoscopicsManager.cpp:

* CServiceBroker::GetSettings()
* CServiceBroker::GetDataCacheCore()
* CServiceBroker::GetRenderSystem()
* g_advancedSettings
* g_windowManager
* g_localizeStrings
* g_graphicsContext
* g_application

We'll ignore the g_ globals for now and focus on removing the
CServiceBroker ones. If you're feeling adventurous, you can recursively kill
the globals and singletons you come across.

Because CStereoscopicsManager now belongs to CServiceManager, we can remove
the use of CServiceBroker by passing references to the constructor upon
creation. For example, here we replace access to CSettings with a
reference that comes from CServiceManager.

Note that the reference was placed first in the list of private variables.
Some helpful comments were added so future authors will know where to add
new variables.
  • Loading branch information
garbear committed Mar 30, 2018
1 parent 201e2ce commit cbcf525
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 10 deletions.
2 changes: 1 addition & 1 deletion xbmc/ServiceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ bool CServiceManager::StartAudioEngine()
// stage 3 is called after successful initialization of WindowManager
bool CServiceManager::InitStageThree()
{
m_stereoscopicsManager.reset(new CStereoscopicsManager);
m_stereoscopicsManager.reset(new CStereoscopicsManager(*m_settings));
m_stereoscopicsManager->Initialize();

// Peripherals depends on strings being loaded before stage 3
Expand Down
17 changes: 9 additions & 8 deletions xbmc/guilib/StereoscopicsManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ static const struct StereoModeMap StringToGuiModeMap[] =
};


CStereoscopicsManager::CStereoscopicsManager(void)
CStereoscopicsManager::CStereoscopicsManager(CSettings &settings) :
m_settings(settings)
{
m_stereoModeSetByUser = RENDER_STEREO_MODE_UNDEFINED;
m_lastStereoModeSetByUser = RENDER_STEREO_MODE_UNDEFINED;
Expand All @@ -114,7 +115,7 @@ void CStereoscopicsManager::Initialize(void)

RENDER_STEREO_MODE CStereoscopicsManager::GetStereoMode(void)
{
return (RENDER_STEREO_MODE) CServiceBroker::GetSettings().GetInt(CSettings::SETTING_VIDEOSCREEN_STEREOSCOPICMODE);
return (RENDER_STEREO_MODE) m_settings.GetInt(CSettings::SETTING_VIDEOSCREEN_STEREOSCOPICMODE);
}

void CStereoscopicsManager::SetStereoModeByUser(const RENDER_STEREO_MODE &mode)
Expand All @@ -140,7 +141,7 @@ void CStereoscopicsManager::SetStereoMode(const RENDER_STEREO_MODE &mode)
{
if (!CServiceBroker::GetRenderSystem().SupportsStereo(applyMode))
return;
CServiceBroker::GetSettings().SetInt(CSettings::SETTING_VIDEOSCREEN_STEREOSCOPICMODE, applyMode);
m_settings.SetInt(CSettings::SETTING_VIDEOSCREEN_STEREOSCOPICMODE, applyMode);
}
}

Expand Down Expand Up @@ -287,7 +288,7 @@ const std::string &CStereoscopicsManager::GetLabelForStereoMode(const RENDER_STE

RENDER_STEREO_MODE CStereoscopicsManager::GetPreferredPlaybackMode(void)
{
return (RENDER_STEREO_MODE) CServiceBroker::GetSettings().GetInt(CSettings::SETTING_VIDEOSCREEN_PREFEREDSTEREOSCOPICMODE);
return (RENDER_STEREO_MODE) m_settings.GetInt(CSettings::SETTING_VIDEOSCREEN_PREFEREDSTEREOSCOPICMODE);
}

int CStereoscopicsManager::ConvertVideoToGuiStereoMode(const std::string &mode)
Expand Down Expand Up @@ -506,7 +507,7 @@ bool CStereoscopicsManager::IsVideoStereoscopic()

void CStereoscopicsManager::OnStreamChange()
{
STEREOSCOPIC_PLAYBACK_MODE playbackMode = (STEREOSCOPIC_PLAYBACK_MODE) CServiceBroker::GetSettings().GetInt(CSettings::SETTING_VIDEOPLAYER_STEREOSCOPICPLAYBACKMODE);
STEREOSCOPIC_PLAYBACK_MODE playbackMode = (STEREOSCOPIC_PLAYBACK_MODE) m_settings.GetInt(CSettings::SETTING_VIDEOPLAYER_STEREOSCOPICPLAYBACKMODE);
RENDER_STEREO_MODE mode = GetStereoMode();

// early return if playback mode should be ignored and we're in no stereoscopic mode right now
Expand All @@ -517,7 +518,7 @@ void CStereoscopicsManager::OnStreamChange()
{
// exit stereo mode if started item is not stereoscopic
// and if user prefers to stop 3D playback when movie is finished
if (mode != RENDER_STEREO_MODE_OFF && CServiceBroker::GetSettings().GetBool(CSettings::SETTING_VIDEOPLAYER_QUITSTEREOMODEONSTOP))
if (mode != RENDER_STEREO_MODE_OFF && m_settings.GetBool(CSettings::SETTING_VIDEOPLAYER_QUITSTEREOMODEONSTOP))
SetStereoMode(RENDER_STEREO_MODE_OFF);
return;
}
Expand All @@ -538,7 +539,7 @@ void CStereoscopicsManager::OnStreamChange()
// users selecting this option usually have to manually switch their TV into 3D mode
// and would be annoyed by having to switch TV modes when next movies comes up
// @todo probably add a new setting for just this behavior
if (CServiceBroker::GetSettings().GetBool(CSettings::SETTING_VIDEOPLAYER_QUITSTEREOMODEONSTOP) == false)
if (m_settings.GetBool(CSettings::SETTING_VIDEOPLAYER_QUITSTEREOMODEONSTOP) == false)
return;

// only change to new stereo mode if not yet in preferred stereo mode
Expand Down Expand Up @@ -604,7 +605,7 @@ void CStereoscopicsManager::OnStreamChange()
void CStereoscopicsManager::OnPlaybackStopped(void)
{
RENDER_STEREO_MODE mode = GetStereoMode();
if (CServiceBroker::GetSettings().GetBool(CSettings::SETTING_VIDEOPLAYER_QUITSTEREOMODEONSTOP) && mode != RENDER_STEREO_MODE_OFF)
if (m_settings.GetBool(CSettings::SETTING_VIDEOPLAYER_QUITSTEREOMODEONSTOP) && mode != RENDER_STEREO_MODE_OFF)
SetStereoMode(RENDER_STEREO_MODE_OFF);
// reset user modes on playback end to start over new on next playback and not end up in a probably unwanted mode
if (m_stereoModeSetByUser != RENDER_STEREO_MODE_OFF)
Expand Down
7 changes: 6 additions & 1 deletion xbmc/guilib/StereoscopicsManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "rendering/RenderSystem.h"

class CAction;
class CSettings;

enum STEREOSCOPIC_PLAYBACK_MODE
{
Expand All @@ -45,7 +46,7 @@ class CStereoscopicsManager : public ISettingCallback,
public IMsgTargetCallback
{
public:
CStereoscopicsManager(void);
CStereoscopicsManager(CSettings &settings);
~CStereoscopicsManager(void) override;

void Initialize(void);
Expand Down Expand Up @@ -90,6 +91,10 @@ class CStereoscopicsManager : public ISettingCallback,
std::string GetVideoStereoMode();
bool IsVideoStereoscopic();

// Construction parameters
CSettings &m_settings;

// Stereoscopic parameters
RENDER_STEREO_MODE m_stereoModeSetByUser;
RENDER_STEREO_MODE m_lastStereoModeSetByUser;
};

0 comments on commit cbcf525

Please # to comment.