Merge lp://staging/~mc-return/compiz/compiz.merge-fix1173914-low-res-non-power-of-2-icon-texture-res into lp://staging/compiz/0.9.10

"Non-power-of-2 resolutions should *never* be used for textures.

Example: 96x96 becomes 128x128 internally, so the 96 pixels have to be upscaled to 128, which creates scaling artifacts."

Most hardware supports non-power-of-two textures. I have not heard of any hardware that does internal upscaling.

In any case, there is no difference between MAX_ICON_SIZE and ICON_SIZE. The constant can be the same - 512 everywhere. CompWindow::getIcon () will go for the largest sized icon first and then smaller ones after that.

>
> Most hardware supports non-power-of-two textures. I have not heard of any
> hardware that does internal upscaling.
>
Well, *every* graphics hardware I know of does internal upscaling of non-power-of-2 textures.
Of course almost every hardware supports those, but upscaling creates artifacts and makes troubles with mipmapping as well.

> In any case, there is no difference between MAX_ICON_SIZE and ICON_SIZE. The
> constant can be the same - 512 everywhere. CompWindow::getIcon () will go for
> the largest sized icon first and then smaller ones after that.

Well, somewhere around the getIcon () code something's definitely wrong -> see bug #1173916 and bug #1173694.

> Well, somewhere around the getIcon () code something's definitely wrong -> see
> bug #1173916 and bug #1173694.

I'm confused by what you mean by this.

Using the svg icon data from the current desktop icon theme is a very large undertaking, and is often quite a messy procedure because the process used to match applications to windows is at best inexact on the EWMH standard. This is the entire reason why lp:bamf exists.

As for the icons themselves, the EWMH says that the applications can specify an arbitrary array of arbitrarily sized icons in their window properties[1]. It is up to the application to specify a high resolution icon. Note that the application is actually specifying the raw bitmap data that makes up the icon in each item of the array of icon properties as opposed to a filename.

The parameters to getIcon () specify the _maximum_ allowed icon dimensions (both width and height) to be used. So, for example if an application uploads icons sizes 32x32, 64x64, 128x128, 256x256 and 512x512 and we call getIcon with (240), then it will pick the largest icon below that size, that being the 128x128 icon size.

Calling getIcon () with a certain parameter doesn't guarantee that you'll get an icon that size, but it does guarantee that you won't get an icon larger than it.

In any case, setting maximum of 512 should more than cover any icon size bottlenecks on the compiz side - its just up to the individual applications to provide larger icons.

[1] http://standards.freedesktop.org/wm-spec/1.3/ar01s05.html

> > Well, somewhere around the getIcon () code something's definitely wrong ->
> see
> > bug #1173916 and bug #1173694.
>
> I'm confused by what you mean by this.
>
Well, I just meant our switchers should act more intelligent and use the best-quality icons available, not low-res ones.

> Using the svg icon data from the current desktop icon theme is a very large
> undertaking, and is often quite a messy procedure because the process used to
> match applications to windows is at best inexact on the EWMH standard. This is
> the entire reason why lp:bamf exists.
>
Ah -> this explains why the Unity Switcher is doing this thing right (had no time to look at the code yet).
We should, in this case, check if bamf is available and mimic the icon selection behaviour of the Unity Switcher in all other Compiz Switchers then...

> As for the icons themselves, the EWMH says that the applications can specify
> an arbitrary array of arbitrarily sized icons in their window properties[1].
> It is up to the application to specify a high resolution icon. Note that the
> application is actually specifying the raw bitmap data that makes up the icon
> in each item of the array of icon properties as opposed to a filename.
>
A very interesting document. Will study that in detail later.

> The parameters to getIcon () specify the _maximum_ allowed icon dimensions
> (both width and height) to be used. So, for example if an application uploads
> icons sizes 32x32, 64x64, 128x128, 256x256 and 512x512 and we call getIcon
> with (240), then it will pick the largest icon below that size, that being the
> 128x128 icon size.
>
I could not find the problem in getIcon (), so my guess was it is "somewhere around that code"...

> Calling getIcon () with a certain parameter doesn't guarantee that you'll get
> an icon that size, but it does guarantee that you won't get an icon larger
> than it.
>
Ok.

> In any case, setting maximum of 512 should more than cover any icon size
> bottlenecks on the compiz side - its just up to the individual applications to
> provide larger icons.
>
> [1] http://standards.freedesktop.org/wm-spec/1.3/ar01s05.html

I agree.
Nevertheless the Compiz Switchers should be tuned to be able to deliver high quality icons like the Unity Switcher already does...

All adjusted to 512 now...

This here is a good example for bug #1178237 - "Commonly used constants should be defined just once globally and be static" by the way...

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/compiz-autolanding/43/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-autolanding/43/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-autolanding/43/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-autolanding/43/console

Was a stupid typo. Fixed.

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/compiz-autolanding/45/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-autolanding/45/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-autolanding/45/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-autolanding/45/console

My fault -> fixing.

Fixed (and recompiled to make sure).

Improved this a little bit, when I'm already on it.

Question:

How about simply adding

+static const unsigned short MAX_ICON_SIZE = 512;

to composite.h and using this variable everywhere, instead of defining 512 several times throughout the code ? We could keep this variable static in memory and use it everywhere appropriately.

If okay, should this be done in this MP or some follow-up ?

> Question:
>
> How about simply adding
>
> +static const unsigned short MAX_ICON_SIZE = 512;
>
> to composite.h and using this variable everywhere, instead of defining 512
> several times throughout the code ? We could keep this variable static in
> memory and use it everywhere appropriately.
>
> If okay, should this be done in this MP or some follow-up ?

It might be better to change the default parameters of getIcon () to be int w = 512, int h = 512 and making plugins call the function with the default params (eg, as screen->getIcon ()).

In any case, this is fine as is.

It might be better to change the default parameters of getIcon () to be int w = 512, int h = 512 and making plugins call the function with the default params (eg, as screen->getIcon ()).

Sounds good. I'll try to look into that in some follow-up.