Skip to content

Commit

Permalink
When what we need is a shared_ptr, we should just go ahead and use one.
Browse files Browse the repository at this point in the history
One of the top crashes reported in the Play Console is a ManagedTexture crash, so seeing if this might help.
  • Loading branch information
hrydgard committed May 18, 2017
1 parent 60378a0 commit df6ce90
Show file tree
Hide file tree
Showing 16 changed files with 73 additions and 81 deletions.
2 changes: 1 addition & 1 deletion UI/BackgroundAudio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ int PlayBackgroundAudio() {
if (!g_gameInfoCache)
return 0; // race condition?

GameInfo *gameInfo = g_gameInfoCache->GetInfo(NULL, bgGamePath, GAMEINFO_WANTSND);
std::shared_ptr<GameInfo> gameInfo = g_gameInfoCache->GetInfo(NULL, bgGamePath, GAMEINFO_WANTSND);
if (!gameInfo)
return 0;

Expand Down
2 changes: 1 addition & 1 deletion UI/CwCheatScreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ CwCheatScreen::CwCheatScreen(std::string gamePath)
}

void CwCheatScreen::CreateCodeList() {
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
if (info && info->paramSFOLoaded) {
gameTitle = info->paramSFO.GetValueString("DISC_ID");
}
Expand Down
3 changes: 1 addition & 2 deletions UI/EmuScreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void EmuScreen::bootGame(const std::string &filename) {
SetBackgroundAudioGame("");

//pre-emptive loading of game specific config if possible, to get all the settings
GameInfo *info = g_gameInfoCache->GetInfo(NULL, filename, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(nullptr, filename, 0);
if (info && !info->id.empty()) {
g_Config.loadGameConfig(info->id);
}
Expand Down Expand Up @@ -210,7 +210,6 @@ void EmuScreen::bootGame(const std::string &filename) {
coreParam.pixelWidth = pixel_xres;
coreParam.pixelHeight = pixel_yres;


std::string error_string;
if (!PSP_InitStart(coreParam, &error_string)) {
bootPending_ = false;
Expand Down
20 changes: 11 additions & 9 deletions UI/GameInfoCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ static bool ReadVFSToString(const char *filename, std::string *contents, std::mu

class GameInfoWorkItem : public PrioritizedWorkQueueItem {
public:
GameInfoWorkItem(const std::string &gamePath, GameInfo *info)
GameInfoWorkItem(const std::string &gamePath, std::shared_ptr<GameInfo> &info)
: gamePath_(gamePath), info_(info) {
}

Expand Down Expand Up @@ -617,7 +617,7 @@ class GameInfoWorkItem : public PrioritizedWorkQueueItem {

private:
std::string gamePath_;
GameInfo *info_;
std::shared_ptr<GameInfo> info_;
DISALLOW_COPY_AND_ASSIGN(GameInfoWorkItem);
};

Expand Down Expand Up @@ -676,7 +676,7 @@ void GameInfoCache::PurgeType(IdentifiedFileType fileType) {
}
}

void GameInfoCache::WaitUntilDone(GameInfo *info) {
void GameInfoCache::WaitUntilDone(std::shared_ptr<GameInfo> &info) {
while (info->IsPending()) {
if (gameInfoWQ_->WaitUntilDone(false)) {
// A true return means everything finished, so bail out.
Expand All @@ -690,12 +690,12 @@ void GameInfoCache::WaitUntilDone(GameInfo *info) {


// Runs on the main thread.
GameInfo *GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags) {
GameInfo *info = nullptr;
std::shared_ptr<GameInfo> GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags) {
std::shared_ptr<GameInfo> info;

auto iter = info_.find(gamePath);
if (iter != info_.end()) {
info = iter->second.get();
info = iter->second;
}

// If wantFlags don't match, we need to start over. We'll just queue the work item again.
Expand All @@ -714,7 +714,7 @@ GameInfo *GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gam
}

if (!info) {
info = new GameInfo();
info = std::make_shared<GameInfo>();
}

if (info->IsWorking()) {
Expand All @@ -732,11 +732,13 @@ GameInfo *GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gam
GameInfoWorkItem *item = new GameInfoWorkItem(gamePath, info);
gameInfoWQ_->Add(item);

info_[gamePath] = std::unique_ptr<GameInfo>(info);
// Don't re-insert if we already have it.
if (info_.find(gamePath) == info_.end())
info_[gamePath] = std::shared_ptr<GameInfo>(info);
return info;
}

void GameInfoCache::SetupTexture(GameInfo *info, Draw::DrawContext *thin3d, GameInfoTex &tex) {
void GameInfoCache::SetupTexture(std::shared_ptr<GameInfo> &info, Draw::DrawContext *thin3d, GameInfoTex &tex) {
using namespace Draw;
if (tex.data.size()) {
if (!tex.texture) {
Expand Down
18 changes: 8 additions & 10 deletions UI/GameInfoCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct GameInfoTex {
}
}
std::string data;
ManagedTexture *texture = nullptr;
std::unique_ptr<ManagedTexture> texture;
// The time at which the Icon and the BG were loaded.
// Can be useful to fade them in smoothly once they appear.
double timeLoaded = 0.0;
Expand All @@ -78,10 +78,7 @@ struct GameInfoTex {
data.clear();
dataLoaded = false;
}
if (texture) {
delete texture;
texture = nullptr;
}
texture.reset(nullptr);
}
private:
DISALLOW_COPY_AND_ASSIGN(GameInfoTex);
Expand Down Expand Up @@ -171,20 +168,21 @@ class GameInfoCache {
// but filled in later asynchronously in the background. So keep calling this,
// redrawing the UI often. Only set flags to GAMEINFO_WANTBG or WANTSND if you really want them
// because they're big. bgTextures and sound may be discarded over time as well.
GameInfo *GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags);
std::shared_ptr<GameInfo> GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags);
void FlushBGs(); // Gets rid of all BG textures. Also gets rid of bg sounds.

PrioritizedWorkQueue *WorkQueue() { return gameInfoWQ_; }

void WaitUntilDone(GameInfo *info);
void WaitUntilDone(std::shared_ptr<GameInfo> &info);

private:
void Init();
void Shutdown();
void SetupTexture(GameInfo *info, Draw::DrawContext *draw, GameInfoTex &tex);
void SetupTexture(std::shared_ptr<GameInfo> &info, Draw::DrawContext *draw, GameInfoTex &tex);

// Maps ISO path to info.
std::map<std::string, std::unique_ptr<GameInfo> > info_;
// Maps ISO path to info. Need to use shared_ptr as we can return these pointers -
// and if they get destructed while being in use, that's bad.
std::map<std::string, std::shared_ptr<GameInfo> > info_;

// Work queue and management
PrioritizedWorkQueue *gameInfoWQ_;
Expand Down
22 changes: 11 additions & 11 deletions UI/GameScreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ GameScreen::~GameScreen() {
}

void GameScreen::CreateViews() {
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);

if (info && !info->id.empty())
saveDirs = info->GetSaveDataDirectories(); // Get's very heavy, let's not do it in update()
Expand Down Expand Up @@ -154,7 +154,7 @@ UI::Choice *GameScreen::AddOtherChoice(UI::Choice *choice) {
}

UI::EventReturn GameScreen::OnCreateConfig(UI::EventParams &e) {
GameInfo *info = g_gameInfoCache->GetInfo(nullptr, gamePath_, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(nullptr, gamePath_, 0);
if (!info) {
return UI::EVENT_SKIPPED;
}
Expand All @@ -168,7 +168,7 @@ UI::EventReturn GameScreen::OnCreateConfig(UI::EventParams &e) {

void GameScreen::CallbackDeleteConfig(bool yes) {
if (yes) {
GameInfo *info = g_gameInfoCache->GetInfo(nullptr, gamePath_, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(nullptr, gamePath_, 0);
if (!info) {
return;
}
Expand Down Expand Up @@ -196,7 +196,7 @@ void GameScreen::update() {

Draw::DrawContext *thin3d = screenManager()->getDrawContext();

GameInfo *info = g_gameInfoCache->GetInfo(thin3d, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(thin3d, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);

if (tvTitle_)
tvTitle_->SetText(info->GetTitle() + " (" + info->id + ")");
Expand Down Expand Up @@ -282,7 +282,7 @@ UI::EventReturn GameScreen::OnPlay(UI::EventParams &e) {
}

UI::EventReturn GameScreen::OnGameSettings(UI::EventParams &e) {
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);
if (info && info->paramSFOLoaded) {
std::string discID = info->paramSFO.GetValueString("DISC_ID");
screenManager()->push(new GameSettingsScreen(gamePath_, discID, true));
Expand All @@ -293,7 +293,7 @@ UI::EventReturn GameScreen::OnGameSettings(UI::EventParams &e) {
UI::EventReturn GameScreen::OnDeleteSaveData(UI::EventParams &e) {
I18NCategory *di = GetI18NCategory("Dialog");
I18NCategory *ga = GetI18NCategory("Game");
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);
if (info) {
// Check that there's any savedata to delete
if (saveDirs.size()) {
Expand All @@ -308,7 +308,7 @@ UI::EventReturn GameScreen::OnDeleteSaveData(UI::EventParams &e) {
}

void GameScreen::CallbackDeleteSaveData(bool yes) {
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
if (yes) {
info->DeleteAllSaveData();
info->saveDataSize = 0;
Expand All @@ -319,7 +319,7 @@ void GameScreen::CallbackDeleteSaveData(bool yes) {
UI::EventReturn GameScreen::OnDeleteGame(UI::EventParams &e) {
I18NCategory *di = GetI18NCategory("Dialog");
I18NCategory *ga = GetI18NCategory("Game");
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE);
if (info) {
screenManager()->push(
new PromptScreen(di->T("DeleteConfirmGame", "Do you really want to delete this game\nfrom your device? You can't undo this."), ga->T("ConfirmDelete"), di->T("Cancel"),
Expand All @@ -330,7 +330,7 @@ UI::EventReturn GameScreen::OnDeleteGame(UI::EventParams &e) {
}

void GameScreen::CallbackDeleteGame(bool yes) {
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
if (yes) {
info->Delete();
g_gameInfoCache->Clear();
Expand All @@ -339,7 +339,7 @@ void GameScreen::CallbackDeleteGame(bool yes) {
}

UI::EventReturn GameScreen::OnCreateShortcut(UI::EventParams &e) {
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
if (info) {
host->CreateDesktopShortcut(gamePath_, info->GetTitle());
}
Expand Down Expand Up @@ -414,7 +414,7 @@ void SetBackgroundPopupScreen::CreatePopupContents(UI::ViewGroup *parent) {
void SetBackgroundPopupScreen::update() {
PopupScreen::update();

GameInfo *info = g_gameInfoCache->GetInfo(nullptr, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTBGDATA);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(nullptr, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTBGDATA);
if (status_ == Status::PENDING && info && !info->IsPending()) {
GameInfoTex *pic = nullptr;
if (info->pic1.dataLoaded && info->pic1.data.size()) {
Expand Down
7 changes: 3 additions & 4 deletions UI/MainScreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class GameButton : public UI::Clickable {
};

void GameButton::Draw(UIContext &dc) {
GameInfo *ginfo = g_gameInfoCache->GetInfo(dc.GetDrawContext(), gamePath_, 0);
std::shared_ptr<GameInfo> ginfo = g_gameInfoCache->GetInfo(dc.GetDrawContext(), gamePath_, 0);
Draw::Texture *texture = 0;
u32 color = 0, shadowColor = 0;
using namespace UI;
Expand Down Expand Up @@ -1008,7 +1008,7 @@ void MainScreen::DrawBackground(UIContext &dc) {
bool MainScreen::DrawBackgroundFor(UIContext &dc, const std::string &gamePath, float progress) {
dc.Flush();

GameInfo *ginfo = 0;
std::shared_ptr<GameInfo> ginfo;
if (!gamePath.empty()) {
ginfo = g_gameInfoCache->GetInfo(dc.GetDrawContext(), gamePath, GAMEINFO_WANTBG);
// Loading texture data may bind a texture.
Expand Down Expand Up @@ -1042,8 +1042,7 @@ UI::EventReturn MainScreen::OnGameSelected(UI::EventParams &e) {
#else
std::string path = e.s;
#endif
GameInfo *ginfo = 0;
ginfo = g_gameInfoCache->GetInfo(nullptr, path, GAMEINFO_WANTBG);
std::shared_ptr<GameInfo> ginfo = g_gameInfoCache->GetInfo(nullptr, path, GAMEINFO_WANTBG);
if (ginfo && ginfo->fileType == IdentifiedFileType::PSP_SAVEDATA_DIRECTORY) {
return UI::EVENT_DONE;
}
Expand Down
7 changes: 3 additions & 4 deletions UI/MiscScreens.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ static const uint32_t colors[4] = {
0xC0FFFFFF,
};

static ManagedTexture *bgTexture = nullptr;
static std::unique_ptr<ManagedTexture> bgTexture;

void UIBackgroundInit(UIContext &dc) {
const std::string bgPng = GetSysDirectory(DIRECTORY_SYSTEM) + "background.png";
Expand All @@ -81,8 +81,7 @@ void UIBackgroundInit(UIContext &dc) {
}

void UIBackgroundShutdown() {
delete bgTexture;
bgTexture = nullptr;
bgTexture.reset(nullptr);
}

void DrawBackground(UIContext &dc, float alpha) {
Expand Down Expand Up @@ -128,7 +127,7 @@ void DrawBackground(UIContext &dc, float alpha) {
}

void DrawGameBackground(UIContext &dc, const std::string &gamePath) {
GameInfo *ginfo = nullptr;
std::shared_ptr<GameInfo> ginfo;
if (gamePath.size())
ginfo = g_gameInfoCache->GetInfo(dc.GetDrawContext(), gamePath, GAMEINFO_WANTBG);
dc.Flush();
Expand Down
5 changes: 2 additions & 3 deletions UI/NativeApp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ static UI::Theme ui_theme;
#include "android/android-ndk-profiler/prof.h"
#endif

ManagedTexture *uiTexture;
std::unique_ptr<ManagedTexture> uiTexture;

ScreenManager *screenManager;
std::string config_filename;
Expand Down Expand Up @@ -681,8 +681,7 @@ void NativeShutdownGraphics() {

UIBackgroundShutdown();

delete uiTexture;
uiTexture = nullptr;
uiTexture.reset(nullptr);

delete uiContext;
uiContext = nullptr;
Expand Down
15 changes: 6 additions & 9 deletions UI/PauseScreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@
AsyncImageFileView::AsyncImageFileView(const std::string &filename, UI::ImageSizeMode sizeMode, PrioritizedWorkQueue *wq, UI::LayoutParams *layoutParams)
: UI::Clickable(layoutParams), canFocus_(true), filename_(filename), color_(0xFFFFFFFF), sizeMode_(sizeMode), textureFailed_(false), fixedSizeW_(0.0f), fixedSizeH_(0.0f) {}

AsyncImageFileView::~AsyncImageFileView() {
delete texture_;
}
AsyncImageFileView::~AsyncImageFileView() {}

void AsyncImageFileView::GetContentDimensions(const UIContext &dc, float &w, float &h) const {
if (texture_) {
Expand All @@ -75,16 +73,15 @@ void AsyncImageFileView::SetFilename(std::string filename) {
if (filename_ != filename) {
textureFailed_ = false;
filename_ = filename;
delete texture_;
texture_ = nullptr;
texture_.reset(nullptr);
}
}

void AsyncImageFileView::Draw(UIContext &dc) {
using namespace Draw;
if (!texture_ && !textureFailed_ && !filename_.empty()) {
texture_ = CreateTextureFromFile(dc.GetDrawContext(), filename_.c_str(), DETECT, true);
if (!texture_)
texture_ = std::move(CreateTextureFromFile(dc.GetDrawContext(), filename_.c_str(), DETECT, true));
if (!texture_.get())
textureFailed_ = true;
}

Expand Down Expand Up @@ -414,7 +411,7 @@ UI::EventReturn GamePauseScreen::OnSwitchUMD(UI::EventParams &e) {
void GamePauseScreen::CallbackDeleteConfig(bool yes)
{
if (yes) {
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
g_Config.unloadGameConfig();
g_Config.deleteGameConfig(info->id);
info->hasConfig = false;
Expand All @@ -428,7 +425,7 @@ UI::EventReturn GamePauseScreen::OnCreateConfig(UI::EventParams &e)
g_Config.createGameConfig(gameId);
g_Config.changeGameSpecific(gameId);
g_Config.saveGameConfig(gameId);
GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
if (info) {
info->hasConfig = true;
}
Expand Down
3 changes: 2 additions & 1 deletion UI/PauseScreen.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#pragma once

#include <functional>
#include <memory>

#include "ui/ui_screen.h"
#include "ui/viewgroup.h"
Expand Down Expand Up @@ -91,7 +92,7 @@ class AsyncImageFileView : public UI::Clickable {
uint32_t color_;
UI::ImageSizeMode sizeMode_;

ManagedTexture *texture_ = nullptr;
std::unique_ptr<ManagedTexture> texture_;
bool textureFailed_;
float fixedSizeW_;
float fixedSizeH_;
Expand Down
Loading

0 comments on commit df6ce90

Please sign in to comment.