Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor to use gdk_threads_add_idle instead of gdk_threads_enter/leave #505

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Refactor to use gdk_threads_add_idle instead of gdk_threads_enter/leave #505

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fadf7f1
Refactor to use gdk_threads_add_idle instead of gdk_threads_enter/leave
trupples Sep 4, 2024
272a016
Fix omitted unused parameter
trupples Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 78 additions & 44 deletions dialogs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ struct _Dialogs
GtkWidget *ver_progress_bar;
};

/* Container for the contents of the iio context selection menu, passed when
* refreshing the list asynchronously.
* Allocated by caller, freed by callee. Strings are strdup-ed and not referenced
* anywhere else, so must be freed as well. */
struct refresh_usb_info {
int num_devices;
gint index; // Index of active item
char **device_list;
};

static Dialogs dialogs;
static GtkWidget *serial_num;
static GtkWidget *fru_date;
Expand Down Expand Up @@ -318,10 +328,6 @@ static bool widget_set_cursor(GtkWidget *widget, GdkCursorType type)
watchCursor = gdk_cursor_new_for_display(display, type);
gdk_window_set_cursor(gdkWindow, watchCursor);

while (gtk_events_pending())
gtk_main_iteration();


return true;
}

Expand Down Expand Up @@ -432,6 +438,53 @@ static struct iio_context * get_context(Dialogs *data)
}
}

/* Change UI state to indicate the USB device list is being refreshed.
* Not thread safe by itself - should be queued using gdk_threads_add_idle(), not g_idle_add_full() */
static bool refresh_usb_clear(void *)
{

gtk_list_store_clear(GTK_LIST_STORE(gtk_combo_box_get_model(GTK_COMBO_BOX(dialogs.connect_usbd))));
widget_set_cursor(dialogs.connect, GDK_WATCH);

return false;
}

/* Update UI with refreshed USB device list. Assumes refresh_usb_clear was called befrehand.
* Not thread safe by itself - should be queued using gdk_threads_add_idle(), not g_idle_add_full() */
static bool refresh_usb_update_list(struct refresh_usb_info *info)
{
int i;

widget_use_parent_cursor(dialogs.connect);

if (info->num_devices)
{
for(i = 0; i < info->num_devices; i++)
{
gtk_combo_box_text_append_text(GTK_COMBO_BOX_TEXT(dialogs.connect_usbd), info->device_list[i]);
free(info->device_list[i]);
}
free(info->device_list);

gtk_combo_box_set_active(GTK_COMBO_BOX(dialogs.connect_usbd), info->index);
gtk_widget_set_sensitive(dialogs.connect_usb, true);
gtk_widget_set_sensitive(dialogs.connect_usbd,true);
}
else
{
// There are no devices
gtk_combo_box_text_append_text(GTK_COMBO_BOX_TEXT(dialogs.connect_usbd), NO_DEVICES);

gtk_combo_box_set_active(GTK_COMBO_BOX(dialogs.connect_usbd),0);
gtk_widget_set_sensitive(dialogs.connect_usbd, false);
}

gdk_threads_add_idle(G_SOURCE_FUNC(connect_clear), dialogs.connect_usb);

free(info);

return false;
}

static void refresh_usb_thread(void)
{
Expand All @@ -440,17 +493,19 @@ static void refresh_usb_thread(void)
GtkListStore *liststore;
ssize_t ret;
unsigned int i = 0;
gint index = 0;
gchar *tmp, *tmp1, *pid, *buf;
char *current = NULL;
char filter[sizeof("local:ip:usb:")];
char *p;
bool scan = false;
gchar *active_uri = NULL;
struct refresh_usb_info *usb_devices_info = malloc(sizeof(struct refresh_usb_info));

gdk_threads_enter();
widget_set_cursor(dialogs.connect, GDK_WATCH);
usb_devices_info->device_list = NULL;
usb_devices_info->num_devices = 0;
usb_devices_info->index = 0;

gdk_threads_add_idle(G_SOURCE_FUNC(refresh_usb_clear), NULL);
if (gtk_combo_box_get_active(GTK_COMBO_BOX(dialogs.connect_usbd)) != -1) {
active_uri = gtk_combo_box_text_get_active_text(
GTK_COMBO_BOX_TEXT(dialogs.connect_usbd));
Expand All @@ -471,9 +526,7 @@ static void refresh_usb_thread(void)
g_signal_handler_block(GTK_COMBO_BOX(dialogs.connect_usbd), dialogs.usbd_signals);
}

/* clear everything, and scan again */
liststore = GTK_LIST_STORE(gtk_combo_box_get_model(GTK_COMBO_BOX(dialogs.connect_usbd)));
gtk_list_store_clear(liststore);
/* Scan again */

p = filter;
if (gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(dialogs.filter_local))) {
Expand All @@ -500,9 +553,14 @@ static void refresh_usb_thread(void)
ret = iio_scan_context_get_info_list(ctxs, &info);
if (ret < 0)
goto err_free_ctxs;

usb_devices_info->num_devices = ret;

if (!ret)
goto err_free_info_list;

usb_devices_info->device_list = calloc(ret, sizeof(char *));

for (i = 0; i < (size_t) ret; i++) {
tmp = strdup(iio_context_info_get_description(info[i]));
pid = strdup(iio_context_info_get_description(info[i]));
Expand Down Expand Up @@ -531,25 +589,25 @@ static void refresh_usb_thread(void)
}
}
if (active_pid != -1 && current && !strcmp(pid, current)) {
index = i;
usb_devices_info->index = i;
}

usb_pids[i]=pid;

if (!tmp1)
tmp1 = tmp;
buf = malloc(strlen(iio_context_info_get_uri(info[i])) +
strlen(tmp1) + 5);
strlen(tmp1) + 5); // will be freed by `refresh_usb_update_list`
sprintf(buf, "%s [%s]", tmp1,
iio_context_info_get_uri(info[i]));
tmp1 = NULL;

gtk_combo_box_text_append_text(GTK_COMBO_BOX_TEXT(dialogs.connect_usbd), buf);
usb_devices_info->device_list[i] = buf;

if (active_uri && !g_strcmp0(active_uri, buf)) {
index = i;
usb_devices_info->index = i;
}

free(buf);
free(tmp);
}

Expand All @@ -559,37 +617,16 @@ static void refresh_usb_thread(void)
iio_scan_context_destroy(ctxs);
nope:

widget_use_parent_cursor(dialogs.connect);

if (!i) {
gtk_combo_box_text_append_text(GTK_COMBO_BOX_TEXT(dialogs.connect_usbd), NO_DEVICES);
gtk_combo_box_set_active(GTK_COMBO_BOX(dialogs.connect_usbd),0);
gtk_widget_set_sensitive(dialogs.connect_usbd, false);
/* Force a clear */
connect_clear(dialogs.connect_net);
gdk_threads_leave();
return;
}

gtk_widget_set_sensitive(dialogs.connect_usb, true);
gtk_widget_set_sensitive(dialogs.connect_usbd,true);

gtk_combo_box_set_active(GTK_COMBO_BOX(dialogs.connect_usbd), index);
gdk_threads_add_idle(G_SOURCE_FUNC(refresh_usb_update_list), usb_devices_info);

if (dialogs.usbd_signals) {
g_signal_handler_unblock(GTK_COMBO_BOX(dialogs.connect_usbd), dialogs.usbd_signals);
}


if (current) {
free(current);
current = NULL;
}

/* Fill things in */
connect_clear(dialogs.connect_usb);
gdk_threads_leave();

}


Expand Down Expand Up @@ -839,7 +876,7 @@ static bool connect_clear(GtkWidget *widget)

if (!widget) {
printf("nope - not callback\n");
return true;
return false;
}
if (gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(widget))) {
/* set - fill in */
Expand All @@ -865,7 +902,7 @@ static bool connect_clear(GtkWidget *widget)
g_object_unref(buf);
}

return true;
return false;
}

static bool refresh_connect_attributes()
Expand Down Expand Up @@ -1129,7 +1166,6 @@ static gboolean version_info_show(gpointer data)
if (!release && !data)
return false;

gdk_threads_enter();
internal_vbox = GTK_WIDGET(gtk_builder_get_object(builder,
"msg_dialog_vbox"));

Expand Down Expand Up @@ -1195,8 +1231,6 @@ static gboolean version_info_show(gpointer data)
gtk_widget_hide(_dialogs->latest_version);

end:
gdk_threads_leave();

return false;
}

Expand All @@ -1223,7 +1257,7 @@ static gpointer version_check(gpointer data)

/*
* Periodically animate the progress bar as long as the version checking thread
* is still working. When the thread has finished, signal (using g_idle_add())
* is still working. When the thread has finished, signal (using gdk_threads_add_idle())
* the GUI to run the dialog that contains latest version information.
*/
static gboolean version_progress_update(gpointer data)
Expand All @@ -1235,7 +1269,7 @@ static gboolean version_progress_update(gpointer data)
if (ver_check_done) {
if (_dialogs)
gtk_widget_hide(_dialogs->ver_progress_window);
g_idle_add(version_info_show, (gpointer)_dialogs);
gdk_threads_add_idle(version_info_show, (gpointer)_dialogs);
}

return !ver_check_done;
Expand Down
7 changes: 0 additions & 7 deletions oscmain.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,6 @@ gint main (int argc, char **argv)
break;
}

#ifndef __MINGW__
/* XXX: Enabling threading when compiling for Windows will lock the UI
* as soon as the main window is moved. */
gdk_threads_init();
#endif
gtk_init(&argc, &argv);

signal(SIGTERM, sigterm);
Expand All @@ -242,7 +237,6 @@ gint main (int argc, char **argv)
signal(SIGHUP, sigterm);
#endif

gdk_threads_enter();
init_application();
c = load_default_profile(profile, true);
if (!ctx_destroyed_by_do_quit) {
Expand All @@ -258,7 +252,6 @@ gint main (int argc, char **argv)
} else
application_quit();
}
gdk_threads_leave();

if (profile)
free(profile);
Expand Down
5 changes: 0 additions & 5 deletions oscplot.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4798,11 +4798,6 @@ void cb_saveas_response(GtkDialog *dialog, gint response_id, OscPlot *plot)
gtk_widget_hide(priv->saveas_dialog);

if (priv->save_as_png) {
int i = 0, timeout = 1000;
while (gtk_events_pending() && i < timeout) {
i++;
gtk_main_iteration();
}
screenshot_saveas_png(plot);
priv->save_as_png = false;
}
Expand Down
34 changes: 26 additions & 8 deletions plugins/fmcomms2_adv.c
View file
Open in desktop
Copy link
Contributor

@dNechita dNechita Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This approach seems fine. There are some calls to gdk_threads_enter()/leave() in this file but it's probably due to this PR still being WIP.
My advice is to test this functionality on Windows as well.

Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ struct w_info {
const char * const name;
};

/* Container for passing both the progress bar and fraction to
* set_calibration_progress_ui.
* Allocated by caller, freed by callee. */
struct set_calibration_progress_params {
GtkProgressBar *pbar;
float fraction;
};

static struct w_info attrs[] = {
{SPINBUTTON, "adi,agc-adc-large-overload-exceed-counter"},
{SPINBUTTON, "adi,agc-adc-large-overload-inc-steps"},
Expand Down Expand Up @@ -814,17 +822,27 @@ static int get_cal_samples(long long cal_tone, long long cal_freq)
return env_samples;
}

static void set_calibration_progress(GtkProgressBar *pbar, float fraction)
{
if (gtk_widget_get_visible(GTK_WIDGET(pbar))) {
static bool set_calibration_progress_ui(struct set_calibration_progress_params *params) {
if (gtk_widget_get_visible(GTK_WIDGET(params->pbar))) {
char ptext[64];

gdk_threads_enter();
snprintf(ptext, sizeof(ptext), "Calibration Progress (%.2f %%)", fraction * 100);
gtk_progress_bar_set_text(pbar, ptext);
gtk_progress_bar_set_fraction(pbar, fraction);
gdk_threads_leave();
snprintf(ptext, sizeof(ptext), "Calibration Progress (%.2f %%)", params->fraction * 100);
gtk_progress_bar_set_text(params->pbar, ptext);
gtk_progress_bar_set_fraction(params->pbar, params->fraction);
}

free(params);

return false;
}

static void set_calibration_progress(GtkProgressBar *pbar, float fraction)
{
struct set_calibration_progress_params *params = malloc(sizeof(struct set_calibration_progress_params));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: sizeof(*params)

params->pbar = pbar;
params->fraction = fraction;

gdk_threads_add_idle(G_SOURCE_FUNC(set_calibration_progress_ui), params);
}

static void calibrate (gpointer button)
Expand Down
Loading