Skip to content

Advanced Library Filtering: add a delay of 500ms before triggering the live filter for text and range filters #1661

Merged
merged 3 commits into from Oct 24, 2012

4 participants

@Montellese
Team Kodi member

As discussed in #1648 these changes use CTimer to trigger a 500ms delay on every change of value in a text or range filter which allows the user to do further changes before the filter is triggered. On not so powerful machines the filter, which basically reloads the whole list and filters the matching items, can take some time to finish so triggering the filter after every change in a range or text filter can make it rather unusable. I just tried 500ms and the delay didn't feel odd or anything.

This delay only applies to the live filter and not to updating the controls of the filter dialog which means that we will still update all the controls after every change. This could probably be done with a delay as well if necessary.

This would need the changes to the Xcode project files as well if deemed Frodo-material.

@Voyager1
Team Kodi member

have looked both at code and usability and this seems to do the trick really well. I'm all for this to be frodo material, since the sliders are simply unusable in its current form.

@jmarshallnz jmarshallnz and 1 other commented on an outdated diff Oct 22, 2012
xbmc/dialogs/GUIDialogMediaFilter.cpp
@@ -606,6 +634,15 @@ void CGUIDialogMediaFilter::UpdateControls()
}
}
+void CGUIDialogMediaFilter::TriggerFilter() const
+{
+ if (m_filter == NULL)
+ return;
+
+ CGUIMessage message(GUI_MSG_NOTIFY_ALL, GetID(), 0, GUI_MSG_FILTER_ITEMS, 10); // 10 for advanced
+ g_windowManager.SendMessage(message);
@jmarshallnz
Team Kodi member
jmarshallnz added a note Oct 22, 2012

Thread message?

@Montellese
Team Kodi member
Montellese added a note Oct 22, 2012

Ah I forgot to change that (had it in mind). Thanks for pointing it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Montellese
Team Kodi member

@jmarshallnz OK replaced SendMessage() with SendThreadMessage().

@Voyager1
Team Kodi member

Although it was good on my dev desktop I've tested it as is on an AMD Fusion and it's still too slow. The solution I found is to move the UpdateControls() call (which does db queries) to OnTimeout(). You also remove it from OnSettingChanged() (the end) but put it also there in the "if (!delay)" block so the case of no delay remains intact.

@Montellese
Team Kodi member

That's what I mentioned in the PR description. I only have an i7 to test which can deal with immediate live filtering without any hassle so I'm grateful for feedback from ppl with low power machines.

@Voyager1
Team Kodi member

I fully agree - I was just expecting that all the db stuff had been moved to after the delay (which is not (yet) the case).

@Montellese
Team Kodi member

OK I included UpdateControls() in the delay triggered functionality. I can even see a bit of speedup on my i7 with this change.

@jmarshallnz
Team Kodi member

Depending on what UpdateControls does, you might need to do it on app thread, perhaps in response to the message you're sending?

@Montellese
Team Kodi member

@jmarshallnz It does some SQL queries (which don't need to run in the app thread) and some calls to CONTROL_ENABLE, CONTROL_DISABLE and SET_CONTROL_LABEL (see https://github.com/xbmc/xbmc/blob/master/xbmc/dialogs/GUIDialogMediaFilter.cpp#L583). I guess SET_CONTROL_LABEL could be replaced with SET_CONTROL_LABEL_THREAD_SAFE but AFAIK there are no thread-safe versions of CONTROL_ENABLE and CONTROL_DISABLE. Should I send a message (like GUI_MSG_REFRESH_LIST or something like that) and handle it on CGUIDialogMediaFilter::OnMessage() by calling UpdateControls from there?

@nephalim3

Does this also change when changing the sort method? One of my machines is very slow when changing the sort, especially when changing from my wife's favorite sort of "Date Added" to mine of Alphabetical by name. which requires multiple changes to get from one to the other. If it also waited 500ms I could flip to the one i want quickly and not need to reload the list 5x.

@Montellese
Team Kodi member

No, this only affects changes in the advanced library filtering dialog. But I can see that it would make sense to have this for sorting as well but that's a different code area so won't be part of this PR.

@jmarshallnz
Team Kodi member

@Montellese Ideally any label setting + control enabling/disabling would be done on the app thread, yeah. If you can do the queries offthread and the control updating onthread that would be the ideal. The control enabling/disabling is probably OK to do offthread anyway (they're setting a boolean pretty much) - the potential problem there would be if the message processing hits while a window is being deinit'd or some such.

@Montellese
Team Kodi member

Seperating the queries and the updating is probably not possible because we do the query for one rule/filter at a time and then directly write the result into the control's label (we don't store the label anywhere else). But I can move all of that into the app thread. The queries don't really take long because they only retrieve the total number of results, they don't actually retrieve the results.

@Montellese
Team Kodi member

OK I've moved UpdateControls() to be executed in the app thread as well.

@jmarshallnz
Team Kodi member

Nice work - looks good to me.

@Montellese Montellese merged commit b18878c into xbmc:master Oct 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.