icon sizing

[Crazy-Hopper]

Hi, I get better results here for skype and psi icons when I set s_embedSize=32.
With 48 the icons are too tiny.

[Crazy-Hopper]

Actually when I unconditionally resize the window to 32*32 I also avoid icon upscaling by KDE resulting in, well, tiny icons but w/o any bluriness brought by upscaling.

[Crazy-Hopper]

The first three are native SNIs.
The fourth (TeamViewer) is 32*32 with avoided upscaling.
5th and 6th are chrome and dropbox are incorrectly upscaled by libappindicator/KDE.
7th and 8th are psi and skype, correctly displayed by xembed-sni-proxy with s_embedSize set to 32.
The rest are native SNIs.

[davidedmundson]

That picture is with your patch?

What do you mean by uncondtional resizing?

What did it look like before?

[Crazy-Hopper]

1. Yes.
2. No condition checking ( if (clientGeom->width < 12 || clientGeom->width > s_embedSize || clientGeom->height < 12 || clientGeom->height > s_embedSize)), just the code in braces. I'll prepare a patch.
3. Without any changes to the master HEAD but the one that lets TeamViewer to appear, i get the following result: TeamViewer icon upscaled and blurred (just like appindicator ones) and psi/skype are 48*48 (s_embedSize) and are too tiny...
   

[Crazy-Hopper]

Actually, the patch is that simple. Sorry, couldn't attach the file properly.

diff --git a/sniproxy.cpp b/sniproxy.cpp
index 945bbac..954e086 100644
--- a/sniproxy.cpp
+++ b/sniproxy.cpp
@@ -44,7 +44,7 @@
 #define SNI_WATCHER_SERVICE_NAME "org.kde.StatusNotifierWatcher"
 #define SNI_WATCHER_PATH "/StatusNotifierWatcher"

-static uint16_t s_embedSize = 48; //max size of window to embed. We no longer resize the embedded window as Chromium acts stupidly.
+static uint16_t s_embedSize = 32; //max size of window to embed. We no longer resize the embedded window as Chromium acts stupidly.

 int SNIProxy::s_serviceCount = 0;

@@ -149,31 +149,18 @@ SNIProxy::SNIProxy(xcb_window_t wid, QObject* parent):
     //tell client we're embedding it
     xembed_message_send(wid, XEMBED_EMBEDDED_NOTIFY, m_containerWid, 0, 0);

-    //move window we're embedding
-    const uint32_t windowMoveConfigVals[2] = { 0, 0 };
+    // move and resize window we're embedding
+    const uint32_t windowMoveConfigVals[4] = { 0, 0, s_embedSize, s_embedSize };

     xcb_configure_window(c, wid,
-                             XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y,
-                             windowMoveConfigVals);
-
-
-    //if the window is a clearly stupid size resize to be something sensible
-    //this is needed as chormium and such when resized just fill the icon with transparent space and only draw in the middle
-    //however spotify does need this as by default the window size is 900px wide.
-    //use an artbitrary heuristic to make sure icons are always sensible
-    if (clientGeom->width < 12 || clientGeom->width > s_embedSize ||
-        clientGeom->height < 12 || clientGeom->height > s_embedSize)
-    {
-        const uint32_t windowMoveConfigVals[2] = { s_embedSize, s_embedSize };
-        xcb_configure_window(c, wid,
-                                XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
-                                windowMoveConfigVals);
-    }
+           XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y |
+           XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
+           windowMoveConfigVals);

     //show the embedded window otherwise nothing happens
     xcb_map_window(c, wid);

-    xcb_clear_area(c, 0, wid, 0, 0, qMin(clientGeom->width, s_embedSize), qMin(clientGeom->height, s_embedSize));
+    xcb_clear_area(c, 0, wid, 0, 0, s_embedSize, s_embedSize);

     xcb_flush(c);

[tehnick]

@Crazy-Hopper Why do not you use sni-qt for Qt4-based applications (for example Psi and Skype)? Just wondering.

[Crazy-Hopper]

@tehnick That's because right now i'm stuck in debian gcc5 pre-transition state where there is no patched qt4 and, most importantly, sni-qt isn't the answer for anything but qt4 apps.

[tehnick]

@Crazy-Hopper Have you tested Chromium after applying of your patch to xembed-sni-proxy?

[Crazy-Hopper]

@tehnick Both google-chrome and chromium use libappindicator to create SNI. How do I test those?

[tehnick]

That's because right now i'm stuck in debian gcc5 pre-transition state where there is no patched qt4

Hmm, patched Qt4 packages are in Debian unstable since Aug 04 and Debian testing since Sep 06.
sni-qt package is also available.

sni-qt isn't the answer for anything but qt4 apps

Yes. But it is more preferable for Qt4-based applications than xembed-sni-proxy project.
@davidedmundson may confirm this.

[tehnick]

@Crazy-Hopper At least for chromium this is not true:
$ ldd /usr/lib/chromium/chromium | grep indicator | wc -l
0
More over as far as I know it does not support libappindicator yet.

I have asked about Chromium just because it is mentioned in the workaround in the code which you have patched.

[Crazy-Hopper]

@tehnick dl_open() ? :-)

Ah-huh! Removed appindicator and now icons are drawn by xembed-sni-proxy.

1-3 SNIs.
4-5 Dropbox and TeamViewer through patched proxy.
6 Google Chrome through libappindicator
7-10 Chromium Hangouts, Chromium, Psi, Skype through patched proxy.
rest are SNIs.

[Crazy-Hopper]

So everything is OK but the Dropbox. But I guess it's Dropbox's fault. (#25)

[Crazy-Hopper]

When I reinstall libappindicator the Chromium and Hangout icons get upscaled, but that looks not as bad as upscaled Dropbox icon is when it's upscaled by libappindicator (as in the first picture).

[Crazy-Hopper]

Is there anything wrong with this approach?