Use Gio everywhere.

[JasonLG1979]

Everything you're doing with GdkPixbuf and reading file paths can be done with Gio.BytesIcon. You can feed it bytes from either the pixmap array or read the file with Gio.File.load_bytes_async and feed it bytes from the result. For bonus points you could also use Gio.EmblemedIcon for the overlay icons.

Not only is it so much cleaner and easier the huge advantage is you don't really have to care about sizing the icons or indicators at all. All you have to do is set the style class of the parent St.Icon to 'system-status-icon' and all of your indicators will have consistent theme defined icon size and padding that matches pretty much the other 99% of indicator extensions.

[JasonLG1979]

Here is a quick example of Gio.EmblemedIcon:

const { Gio, GObject, St } = imports.gi;
const Panel = imports.ui.main.panel;
const { Button } = imports.ui.panelMenu;

const ROLE = 'DummyButton';

function enable() {
    if (!Panel.statusArea[ROLE]) {
        Panel.addToStatusArea(ROLE, new DummyButton());
    }
}

function disable() {
    let indicator = Panel.statusArea[ROLE];
    if (indicator) {
        indicator.destroy();
    }
}

const DummyButton = GObject.registerClass({
    GTypeName: 'DummyButton'
}, class DummyButton extends Button {
    _init() {
        super._init(0.5, 'DummyButton', true);

        let indicator = new St.Icon({
            style_class: 'system-status-icon'
        });

        let mainIcon = Gio.ThemedIcon.new('face-smile');
        let emblem = Gio.Emblem.new(Gio.ThemedIcon.new('dialog-ok'));
        let emblemIcon = Gio.EmblemedIcon.new(mainIcon, emblem);

        indicator.gicon = emblemIcon;

        this.add_child(indicator);
    }
});

Here is an example function to read an image file asynchronously and spit out a Gio.Icon from the result that catches errors.

function getIconFromFilePath(path, callback, cancellable) {
    Gio.File.new_for_path(path).load_bytes_async(
        cancellable || null,
        (file, result) => {
            try {
                let bytes = file.load_bytes_finish(result)[0];
                let gicon = Gio.BytesIcon.new(bytes);
                callback(gicon);
            } catch (error) {
                if (!error.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) {
                    // Cancelling counts as an error...
                    // maybe do stuff on error otherwise?
                }
            }
        }
    );
}

Going bytes to Gio.Icon is as simple as:

let gicon = Gio.BytesIcon.new(bytes);

Once you get the image bytes out of the pixmap array.

[3v1n0]

Thanks, I agree this should be the way... I hope to find some time to refactor this soon, in the mean time: patches welcome :)

[lestcape]

@JasonLG1979 do you tested if bytes are in the correct order? See: https://gitlab.gnome.org/GNOME/gjs/issues/220

Last time i check this was not possible, and i was forced to use Colg

gnome-shell-extension-appindicator/appIndicator.js

Line 455 in 126cb93

image.set_bytes(bytes,

I think will be good if some how we can get a gicon directly from the bytes, but the only way i get the bytes in the right position was loading a Cogl.Texture inside Clutter.Image, but this is not the same as get a gicon. I prefer to use St instead of the just Clutter.

[JasonLG1979]

@JasonLG1979 do you tested if bytes are in the correct order?

No. I've never had an issue with that.

Last time i check this was not possible, and i was forced to use Colg

Nope it's worked just fine for me for over 2 years.

I think will be good if some how we can get a gicon directly from the bytes, but the only way i get the bytes in the right position was loading a Cogl.Texture inside Clutter.Image, but this is not the same as get a gicon. I prefer to use St instead of the just Clutter.

I've been using pretty close to the same code as above in my extension forever and before that in my previous MPRIS extension.

https://github.com/JasonLG1979/gnome-shell-extension-mpris-indicator-button/blob/master/mprisindicatorbutton%40JasonLG1979.github.io/widgets.js#L92-L112

You can get a gicon from bytes. That's the whole point of Gio.BytesIcon. Why would there be Gio.BytesIcon.new that takes bytes as an arg if you couldn't turn image bytes into a gicon?

[JasonLG1979]

The above code is roughly equivalent to:

let gicon = Gio.FileIcon.new(Gio.File.new_for_path(path));

Except for 2 important differences. My version is cancel-able and it can catch errors.

Gio.File.new_for_path does no I/O by itself and never really fails. Gio.FileIcon.new does do I/O but fails silently and just sets the icon to the broken image icon.

Silent failure is not your friend.

[3v1n0]

So... I've implemented most of this with 8b84559 however, there are some cases where we can't go with just bytes, but we should instead use GtkPixbuf (that implements Gio.Icon anyways).

As @lestcape mentioned, the byte order is different though (as mentioned at https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/ it's ARGB32), so I needed to apply some conversion here. Would have been nicer to do this in some C function or by the shell when loading the texture, but this is just not possible from what I see.

However I don't notice any slowdown by doing it.

[lestcape]

@3v1n0 the refactor is really nice, but not the argbToRgba.

If you can not notice the lag right now, this don't means that is good idea. What about:

1. Open a pull request to support ARGB32 as a Cairo format?
2. Add an new util function to the shell, like this one: https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/master/src/shell-util.c#L332.
3. Add a function to St.Icon to load an image like in ClutterImage, but set it as a gicon instead of a context.
4. Probably something that create a gicon from a Cogl.Texture?

The true is that to me there are not a good way right now to do that.

[lestcape]

Meaybe if we created a Cogl.Texture and then get_data in a valid gio format, this should work without have frezed. I had never try in that way, is just a new idea.

[3v1n0]

@3v1n0 the refactor is really nice, but not the argbToRgba.

@lestcape I agree and know that there are better ways to do this at libraries level, probably having GdkPixbuf to include GdkColorspace by default would just be the best choice.

However.... At this (extension) level we have to be pragmatic, and it's not realistic to be able to implement such functions in a C library when we're about to freeze the codebase (could be good for next iterations though).

By the way... I've done some tests and actually it looks like that the simplest solution that I wrote initially (not pushed here) is actually fast enough.

Check this jsfiddle: https://jsfiddle.net/5azjpyt3/

As you can see (and test) with a 4Mb image it only takes 8-10ms that is pretty acceptable (although having WebWorker's in gjs would be nice), considering that most of the images we get are maximum 64x64 or 256x256 we really can ignore this aspect as it shouldn't affect rendering.

It's true that I would like some more caching, but the fact we cache such data into the proxy, it's a bit complicated to change things. But latest commits ensure that we won't emit changed signals for icons that have no content changes anyways.

[lestcape]

@3v1n0 I understand. I'm just throwing ideas, like this one. Please note, I'm on 18.04, so nothing of this is working for me, but in theory that need to work in 20.04:

let ctx = new Cogl.Context(null);

or instead:

let renderer = new Cogl.Renderer();
let display = new Cogl.Display(renderer, new Cogl.OnscreenTemplate(new Cogl.SwapChain()));
let ctx = new Cogl.Context(display);

or

let ctx = Clutter.get_default_backend().get_cogl_context();

Then:

let tex = Cogl.Texture2D.new_with_size(ctx, rowstride, 1);
tex.set_data_bytes(Cogl.PixelFormat.ARGB_8888, rowstride, data, 0);

Or

let tex = Cogl.Texture2D.new_with_size(ctx, rowstride, 1);
tex.set_data(Cogl.PixelFormat.ARGB_8888, rowstride, data, 0);

Then get the data back, but now in RGBA instead of in ARGB.

let data = tex.get_data (Cogl.PixelFormat.RGBA_8888, rowstride);

[3v1n0]

@lestcape

let data = tex.get_data (Cogl.PixelFormat.RGBA_8888, rowstride);

This is a nice idea, but... Unfortunately I don't think we can use it right now in gnome-shell, in fact:

• Cogl.Context constructor is not introspected, nor is get_cogl_context() from ClutterBackend. In the same way also Cogl.Texture2D.new_from_data is explicitly skipped for introspection.
• We could indeed create new Clutter.Image as we did before, and get the texture from it, but... Unfortunately the introspection for it is wrong, so creating a new ClutterImage(), adding the bytes and calling get_texture() would just return a generic Cogl.Object and not a texture we can get the data from...
• Similar to the above (as Clutter.Image uses it internally), using the deprecated Cogl.Texture.new_from_data(1, 2, 0, Cogl.PixelFormat.ARGB_8888, Cogl.PixelFormat.ANY, 4 * 2, arr) would again not work as it would return just a Cogl.Object.

As you can see:

So, this is indeed the faster path, but I would still need to fix upstream mutter to make the get_texture introspection to work as expected.

The only way I know to get a CoglContext, would be to do it at paint time, but not sure I like the idea to store it the first time we get a paint signal callback...

[lestcape]

I was thinking, that things don't work for me just because I was on 18.04. I guess I ran out of ideas.

Thanks for the work on this anyway. No always we can have what we want in the time we want it. At least I tried to find a way, but not luck to me this time. The introspection is something "funny" some times, like this one...

[lestcape]

Probably this will help with Cogl: https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1160