GNOME Bugzilla – Bug 612759
Fix app icon fading
Last modified: 2010-03-13 17:51:08 UTC
The way we were loading data into a CoglTexture, then pulling it out and manipulating it on the CPU, then loading it back into a texture was a bit lame. Clean things up a bit here by loading directly into the CPU. This patch also changes things so we cache the faded texture data inside ShellApp, which is conceptually slightly unclean, but should be workable in practice.
Created attachment 156036 [details] [review] Fix app icon fading
Created attachment 156039 [details] [review] Fix app icon fading
Created attachment 156040 [details] [review] Fix app icon fading
Review of attachment 156040 [details] [review]: Looks good, even though we still don't know what was going on, pulling data from the video memory, processing it on the CPU and uploading it back to the GPU is not very efficient anyway. And having it cached now is an added bonus. Just some minor comments: ::: js/ui/panel.js @@ +275,3 @@ this._label.setText(this._focusedApp.get_name()); } else if (this._activeSequence != null) { icon = this._activeSequence.create_icon(AppDisplay.APPICON_SIZE); Looks inconsistent .. don't we want to create a faded icon here too? ::: src/shell-app.c @@ +112,3 @@ + + gdk_pixbuf_get_width (pixbuf) * ((gdk_pixbuf_get_n_channels (pixbuf)* gdk_pixbuf_get_bits_per_sample (pixbuf) + 7) / 8); + + pixels = g_malloc0 (rowstride * height); Should be pixbuf_byte_size right? @@ +121,3 @@ + for (j = 0; j < height; j++) + { + guchar *pixel = &pixels[j * rowstride + i * 4]; Instead of hard coding 4 the result of gdk_pixbuf_get_n_channels () should be used. Seems odd to assume 4 here and check for an alpha channel later when creating the texture.
*** Bug 612697 has been marked as a duplicate of this bug. ***
(In reply to comment #4) > > Looks inconsistent .. don't we want to create a faded icon here too? I'd prefer not to actually for now - when I land my startup-notification work, we'll much more consistently have an app in this case, and not having it will make the untracked case stand out more. > ::: src/shell-app.c > @@ +112,3 @@ > + + gdk_pixbuf_get_width (pixbuf) * ((gdk_pixbuf_get_n_channels (pixbuf)* > gdk_pixbuf_get_bits_per_sample (pixbuf) + 7) / 8); > + > + pixels = g_malloc0 (rowstride * height); > > Should be pixbuf_byte_size right? Hmmm...I'm not entirely sure if cogl is going to handle that and, I don't think it matters much in the big picture; it's not going to be very many bytes, and we free it quickly anyways. > > @@ +121,3 @@ > + for (j = 0; j < height; j++) > + { > + guchar *pixel = &pixels[j * rowstride + i * 4]; > > Instead of hard coding 4 the result of gdk_pixbuf_get_n_channels () should be > used. > Seems odd to assume 4 here and check for an alpha channel later when creating > the texture. Fixed, thanks.
Attachment 156040 [details] pushed as 3aea09b - Fix app icon fading