Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

changed: allow any threaded directory requests to be canceled by abandoni #272

Merged
merged 2 commits into from

2 participants

@elupus
Collaborator

This allows any directory request to be aborted while the busy dialog is up.

The busy dialog will be be modal in a sense to it will swallow events as if it was a true blocking modal. But it will not block whoever made it modal. It will only respond to back or previous menu actions.

  • If any new modal dialog shows up after the busy dialog shows it will go away.
  • If the original modal dialog shows up again, the busy dialog is shown again.
@jmarshallnz
Owner

I think you'll want a check for dialog closing/already shown in Show_Internal given that you're calling it all the time.

I wonder what happens when a modal dialog pops up though - wouldn't that occur only while you're sitting in ProcessRenderLoop(false) at the bottom there (as modal dialogs can only run the process loop from the app thread via app messenger), and thus you won't be hiding the busy dialog at all in that case?

If you want the busy dialog to hide when another dialog pops up it will have to be done in the busy dialog itself (or at some other point in the ProcessRenderLoop()).

Given this, I wonder if a method similar to the progress dialog should be used instead, i.e. where you essentially just grab the busy dialog, throw it up on the screen and repeatedly tell it to process on some interval. The only difference between the methods currently is that the progress dialog allows the window open anim to complete before returning control to the caller (reason is that it's generally used in cases where it can be quite a long time between updates being called).

Once sorted, it would be good to generalize this so that we can execute any (cancellable) CJob in a modal way (i.e. throw up progress/busy while the job is in progress, returning once done or cancelled).

@elupus
Collaborator

It works when processbar shows up for python. But true that is threaded separate, hadn't realized.

I think it only calls Show() once. Once it has routed the busy dialog which i think happens on first process, the topmostdialog is the busy dialog so show won't be called again.

Moving the hiding/showing into busy dialog would make more sense. Ie it should not unload, it should just hide controls if modal dialog shows up ontop of it.

@jmarshallnz
Owner

Yeah - it kinda makes sense to combine progress and busy in some respects - you never need them both up at the same time, the busy is just a more discrete version of it really.

@elupus
Collaborator

How about this. Only call Show() once. It handles it's hide/show internally based on if it's the topmostmodal dialog.

We should however add something like the threading in CDirectory to the CMediaWindow's instead, so we could do the additional processing on the CFileItemList in the job instead of app thread. But that is for another time.

@jmarshallnz
Owner

Better I think, yes. I presume dvdplayer doesn't complain (it uses busy dialog as well on startup?) with this, as it doesn't really matter whether ProcessRenderLoop(true) or ProcessRenderLoop(false) is called.

Agreed about moving this to the mediawindows (and file browser) - generalizing it so it can run an arbitrary job might be the way to go?

@elupus
Collaborator

Yea I was thinking of that. Some sort of wrapper around the the job manager where app thread can wait for a job in main thread to complete while still rendering.

@elupus elupus merged commit c0b1945 into xbmc:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
5 xbmc/Application.cpp
@@ -3127,6 +3127,11 @@ bool CApplication::Cleanup()
CAddonMgr::Get().DeInit();
+#if defined(HAS_LIRC) || defined(HAS_IRSERVERSUITE)
+ CLog::Log(LOGNOTICE, "closing down remote control service");
+ g_RemoteControl.Disconnect();
+#endif
+
CLog::Log(LOGNOTICE, "unload sections");
#ifdef HAS_PERFORMANCE_SAMPLE
View
44 xbmc/dialogs/GUIDialogBusy.cpp
@@ -20,13 +20,57 @@
*/
#include "GUIDialogBusy.h"
+#include "guilib/GUIWindowManager.h"
CGUIDialogBusy::CGUIDialogBusy(void)
: CGUIDialog(WINDOW_DIALOG_BUSY, "DialogBusy.xml")
{
m_loadOnDemand = false;
+ m_bModal = true;
}
CGUIDialogBusy::~CGUIDialogBusy(void)
{
}
+
+void CGUIDialogBusy::Show_Internal()
+{
+ m_bCanceled = false;
+ m_bRunning = true;
+ m_bModal = true;
+ m_bLastVisible = true;
+ m_dialogClosing = false;
+ g_windowManager.RouteToWindow(this);
+
+ // active this window...
+ CGUIMessage msg(GUI_MSG_WINDOW_INIT, 0, 0);
+ OnMessage(msg);
+}
+
+void CGUIDialogBusy::DoProcess(unsigned int currentTime, CDirtyRegionList &dirtyregions)
+{
+ bool visible = g_windowManager.GetTopMostModalDialogID() == WINDOW_DIALOG_BUSY;
+ if(!visible && m_bLastVisible)
+ dirtyregions.push_back(m_renderRegion);
+ m_bLastVisible = visible;
+ CGUIDialog::DoProcess(currentTime, dirtyregions);
+}
+
+void CGUIDialogBusy::Render()
+{
+ if(!m_bLastVisible)
+ return;
+ CGUIDialog::Render();
+}
+
+bool CGUIDialogBusy::OnAction(const CAction &action)
+{
+ if(action.GetID() == ACTION_NAV_BACK
+ || action.GetID() == ACTION_PREVIOUS_MENU)
+ {
+ m_bCanceled = true;
+ return true;
+ }
+ else
+ return CGUIDialog::OnAction(action);
+}
View
9 xbmc/dialogs/GUIDialogBusy.h
@@ -29,4 +29,13 @@ class CGUIDialogBusy: public CGUIDialog
public:
CGUIDialogBusy(void);
virtual ~CGUIDialogBusy(void);
+ virtual bool OnAction(const CAction &action);
+ virtual void DoProcess(unsigned int currentTime, CDirtyRegionList &dirtyregions);
+ virtual void Render();
+
+ bool IsCanceled() { return m_bCanceled; }
+protected:
+ virtual void Show_Internal(); // modeless'ish
+ bool m_bCanceled;
+ bool m_bLastVisible;
};
View
33 xbmc/filesystem/Directory.cpp
@@ -143,8 +143,8 @@ bool CDirectory::GetDirectory(const CStdString& strPath, CFileItemList &items, C
pDirectory->SetUseFileDirectories(bUseFileDirectories);
pDirectory->SetExtFileInfo(extFileInfo);
- bool result = false;
- while (!result)
+ bool result = false, cancel = false;
+ while (!result && !cancel)
{
if (g_application.IsCurrentThread() && allowThreads && !URIUtils::IsSpecial(strPath))
{
@@ -153,30 +153,19 @@ bool CDirectory::GetDirectory(const CStdString& strPath, CFileItemList &items, C
CGetDirectory get(pDirectory, realPath);
if(!get.Wait(TIME_TO_BUSY_DIALOG))
{
- CGUIDialogBusy* dialog = NULL;
+ CGUIDialogBusy* dialog = (CGUIDialogBusy*)g_windowManager.GetWindow(WINDOW_DIALOG_BUSY);
+ dialog->Show();
+
while(!get.Wait(10))
{
CSingleLock lock(g_graphicsContext);
- if(g_windowManager.IsWindowVisible(WINDOW_DIALOG_PROGRESS)
- || g_windowManager.IsWindowVisible(WINDOW_DIALOG_LOCK_SETTINGS))
- {
- if(dialog)
- {
- dialog->Close();
- dialog = NULL;
- }
- g_windowManager.ProcessRenderLoop(false);
- }
- else
+
+ if(dialog->IsCanceled())
{
- if(dialog == NULL)
- {
- dialog = (CGUIDialogBusy*)g_windowManager.GetWindow(WINDOW_DIALOG_BUSY);
- if(dialog)
- dialog->Show();
- }
- g_windowManager.ProcessRenderLoop(true);
+ cancel = true;
+ break;
}
+ g_windowManager.ProcessRenderLoop(false);
}
if(dialog)
dialog->Close();
@@ -191,7 +180,7 @@ bool CDirectory::GetDirectory(const CStdString& strPath, CFileItemList &items, C
if (!result)
{
- if (g_application.IsCurrentThread() && pDirectory->ProcessRequirements())
+ if (!cancel && g_application.IsCurrentThread() && pDirectory->ProcessRequirements())
continue;
CLog::Log(LOGERROR, "%s - Error getting %s", __FUNCTION__, strPath.c_str());
return false;
View
4 xbmc/guilib/GUIDialog.h
@@ -65,8 +65,8 @@ class CGUIDialog :
virtual void UpdateVisibility();
friend class CApplicationMessenger;
- void DoModal_Internal(int iWindowID = WINDOW_INVALID, const CStdString &param = ""); // modal
- void Show_Internal(); // modeless
+ virtual void DoModal_Internal(int iWindowID = WINDOW_INVALID, const CStdString &param = ""); // modal
+ virtual void Show_Internal(); // modeless
void Close_Internal(bool forceClose = false);
bool m_bRunning;
Something went wrong with that request. Please try again.