GNOME Bugzilla – Bug 723077
Use the GrlSource::source-icon property in grilo-test-ui
Last modified: 2014-01-30 08:43:12 UTC
Commit https://git.gnome.org/browse/grilo/commit/?id=c8423dac91 introduced the GrlSource::source-icon property to let sources set a custom icon. This patch uses this property in the browsing treeview in grilo-test-ui, falling back to the folder icon if not set.
Created attachment 267278 [details] [review] grilo-test-ui: Use the icon provided by each GrlSource Use the newly introduced Icon property of GrlSource to display a custom icon (if available) in the browser list. Signed-off-by: Emanuele Aina <emanuele.aina@collabora.com>
Review of attachment 267278 [details] [review]: Looks good to me.
Comment on attachment 267278 [details] [review] grilo-test-ui: Use the icon provided by each GrlSource commit cd1d69cdd02c1f8e8aee7eb2199e86365e6b2a1f Author: Emanuele Aina <emanuele.aina@collabora.com> Date: Sun Jan 26 23:09:35 2014 +0100 grilo-test-ui: Use the icon provided by each GrlSource Use the newly introduced Icon property of GrlSource to display a custom icon (if available) in the browser list. Signed-off-by: Emanuele Aina <emanuele.aina@collabora.com> https://bugzilla.gnome.org/show_bug.cgi?id=723077 tools/grilo-test-ui/main.c | 52 ++++++++++++++++++---------------------------------- 1 file changed, 18 insertions(+), 34 deletions(-)
I'm reopening this. On my system, this makes grilo-test-ui unusably slow, as it does sync calls for hover on a row.
Bastien, I'm not sure how to reproduce it as I don't see any hover effect on rows and in general the program stays responsive (I'm using a quite up-to-date jhbuild checkout, but my only icon-providing DLNA server is Rygel on localhost). Does it happens only with the sources using GrlSource:source-icon or it happens with the folder icon too? I only did a cursory investigation on how the treeview resolves the GIcon, and my understanding is that the whole icon machinery is synchronous, which indeed may be the cause of the slowness you're experiencing. I'm not sure how to fix the problem in this case, I had hoped that icon resolution would be a non-blocking operation but it doesn't seems to be the case (but I'd like someone more expert than me on gtk internals to confirm).
(In reply to comment #5) > Bastien, I'm not sure how to reproduce it as I don't see any hover effect on > rows There's no hover effect, but as the icon might change depending on the row's state (hovered, selected, etc.), the gicon is fetched, and re-downloaded. > and in general the program stays responsive (I'm using a quite up-to-date > jhbuild checkout, but my only icon-providing DLNA server is Rygel on > localhost). I have 3 UPnP servers, 2 of which are on the LAN rather than on localhost. One of them has a broken icon. > Does it happens only with the sources using GrlSource:source-icon or it happens > with the folder icon too? Doesn't happen with the folder icon, it's local and cached. GFileIcons aren't, and if they point to remote location, you're re-downloading it every time. > I only did a cursory investigation on how the treeview resolves the GIcon, and > my understanding is that the whole icon machinery is synchronous, which indeed > may be the cause of the slowness you're experiencing. Yep. > I'm not sure how to fix the problem in this case, I had hoped that icon > resolution would be a non-blocking operation but it doesn't seems to be the > case (but I'd like someone more expert than me on gtk internals to confirm). I would keep the old icons in place (just don't call grl_source_get_icon()) and implement showing the source metadata on the right hand-side, as for GrlMedias. The GIcon can be serialised to a string using g_icon_to_string(). This would be similar to the behaviour when selecting GrlMedias, we don't set a GIcon for their thumbnails.
Created attachment 267387 [details] [review] grilo-test-ui: Fix slowness when remote GFileIcons are used
Created attachment 267388 [details] [review] grilo-test-ui: Show source metadata on right pane FIXME: Implement GrlMediaType to string
Review of attachment 267387 [details] [review]: Looks good, I've reported https://bugzilla.gnome.org/show_bug.cgi?id=723151 for the sync issue. ::: tools/grilo-test-ui/main.c @@ +2026,1 @@ + icon = g_themed_icon_new ("folder"); I'd be tempted to add a comment saying that we don't use grl_source_get_icon() since remote icons will work badly until the synchronous icon handling inside GTK is fixed. I've opened bug #723151 for this purpose, so adding a pointer would be nice for later implementors.
Created attachment 267412 [details] [review] grilo-test-ui: Show source metadata on right pane
Comment on attachment 267387 [details] [review] grilo-test-ui: Fix slowness when remote GFileIcons are used commit f1e99181a4a745bda047eae9f659de328ad6df18 Author: Bastien Nocera <hadess@hadess.net> Date: Tue Jan 28 10:17:01 2014 +0100 grilo-test-ui: Fix slowness when remote GFileIcons are used https://bugzilla.gnome.org/show_bug.cgi?id=723077 tools/grilo-test-ui/main.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
Review of attachment 267412 [details] [review]: ::: tools/grilo-test-ui/main.c @@ +981,3 @@ + + gtk_widget_set_sensitive (view->show_btn, FALSE); + gtk_widget_set_sensitive (view->store_metadata_btn, FALSE); What is this store_metadata_btn?
(In reply to comment #12) > Review of attachment 267412 [details] [review]: > > ::: tools/grilo-test-ui/main.c > @@ +981,3 @@ > + > + gtk_widget_set_sensitive (view->show_btn, FALSE); > + gtk_widget_set_sensitive (view->store_metadata_btn, FALSE); > > What is this store_metadata_btn? Stuff that shouldn't be there (a button I added to make it store the currently selected GrlMedia's metadata to the metadata-store).
Created attachment 267493 [details] [review] grilo-test-ui: Show source metadata on right pane
commit 39ce2d2e702ae9117a6f48e8d38a1a03768280e3 Author: Bastien Nocera <hadess@hadess.net> Date: Tue Jan 28 10:40:17 2014 +0100 grilo-test-ui: Show source metadata on right pane https://bugzilla.gnome.org/show_bug.cgi?id=723077 tools/grilo-test-ui/main.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-)