Skip to content

Commit

Permalink
hidpi: fix tray icon spacing with window-scaling > 1
Browse files Browse the repository at this point in the history
  • Loading branch information
lukefromdc authored and raveit65 committed Mar 22, 2018
1 parent a32c8da commit d02fcc5
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
10 changes: 10 additions & 0 deletions applets/notification_area/system-tray/na-tray-child.c
Expand Up @@ -120,6 +120,8 @@ na_tray_child_get_preferred_width (GtkWidget *widget,
gint *minimal_width,
gint *natural_width)
{
gint scale;
scale = gtk_widget_get_scale_factor (widget);
GTK_WIDGET_CLASS (na_tray_child_parent_class)->get_preferred_width (widget,
minimal_width,
natural_width);
Expand All @@ -129,13 +131,18 @@ na_tray_child_get_preferred_width (GtkWidget *widget,

if (*natural_width < 16)
*natural_width = 16;

*minimal_width = *minimal_width / scale;
*natural_width = *natural_width / scale;
}

static void
na_tray_child_get_preferred_height (GtkWidget *widget,
gint *minimal_height,
gint *natural_height)
{
gint scale;
scale = gtk_widget_get_scale_factor (widget);
GTK_WIDGET_CLASS (na_tray_child_parent_class)->get_preferred_height (widget,
minimal_height,
natural_height);
Expand All @@ -145,6 +152,9 @@ na_tray_child_get_preferred_height (GtkWidget *widget,

if (*natural_height < 16)
*natural_height = 16;

*minimal_height = *minimal_height / scale;
*natural_height = *natural_height / scale;
}

static void
Expand Down
3 changes: 2 additions & 1 deletion applets/notification_area/system-tray/na-tray.c
Expand Up @@ -127,7 +127,8 @@ tray_added (NaTrayManager *manager,

na_host_emit_item_added (NA_HOST (tray), NA_ITEM (icon));

gtk_widget_show (GTK_WIDGET (icon));
/*Does not seem to be needed anymore and can cause a render issue with hidpi*/
/*gtk_widget_show (GTK_WIDGET (icon));*/
}

static void
Expand Down

5 comments on commit d02fcc5

@raveit65
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukefromdc @vkareh
Does this commit affect the applet if build with -with-in-process-applets=all ?
I have have an serious report about a memleak if panel is build with this option.
https://bugzilla.redhat.com/show_bug.cgi?id=1575091

@lukefromdc
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sure about this, but I don't see anything in na-tray-c or na-tray-child.c that should be affected by building in or out of process. #ifdef NOTIFICATION_AREA_INPROCESS is found only in main.c, and there is an unconditional instance of gtk_widget_show_all (GTK_WIDGET (applet)) there. I doubt that commit caused the memory leak unless a bisect found it to be the first bad commit, and in that case I have no idea why.

I have not had that problem at all, though I saw one report of a memory leak with and only with Wine apps show tray icons (https://bugzilla.redhat.com/show_bug.cgi?id=1575091), and I would suspect something other than this commit to be the cause again unless a bisect found the problem starting with this commit (rather than a build containing it but which also was the first build of a series with the tray in-process).

@raveit65
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user confirmed that building the panel with --with-in-process-applets=none fixes the issue.
https://bugzilla.redhat.com/show_bug.cgi?id=1575091#c17
I just reverted this commit here in another build with --with-in-process-applets=all and wait for his answer.
And i ask him to file a report at github about.

@lukefromdc
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't test WINE with this, as I do not want to run Windows binaries on an encrypted computer containing raw activists video clips. Also WINE often does not work on anything but finished releases and not Ubuntu alphas or Debian Unstable.

I think we may end up recommending that the tray be built out-of-process but the window list in-process. In the old days, GNOME said panel applets were lighter if in process, more reliable if out of process.

@raveit65
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this commit isn't the problem.
https://bugzilla.redhat.com/show_bug.cgi?id=1575091#c20
With a rpm where this commit was reverted, but with in-process-build, the user report that the memleak still exists if using wine tray-applets.
That means the error is somewhere else but caused by build in-process.

Sadly, this is the second report about build in-process in fedora, so i am not very happy with my decision to switch.
Looks like it is better to fix HIDPI issues if panel is build with out-of-process for the moment.

Please sign in to comment.