I've been looking again at the status bar issues on RISC OS.
We've solved the main problem, which was that curl was sending excessive
update messages, causing the status bar to get stuck in a redraw loop.
However, I thought making the RISC OS front end a bit more 'friendly' with
respect to multitasking was a good idea.
Here are the diffs for this patch, with some notes.
Firstly, a minor change to the status bar structure to reflect a change in
the string that's passed in - it's now allocated and freed externally so we
treat it as a const.
Index: riscos/gui/status_bar.c
=================================================================== ---
riscos/gui/status_bar.c (revision 10102) +++ riscos/gui/status_bar.c
(working copy) @@ -45,7 +45,7 @@
struct status_bar {
wimp_w w; /**< status bar window handle */
wimp_w parent; /**< parent window handle */
- char *text; /**< status bar text */
+ const char *text; /**< status bar text */
struct progress_bar *pb; /**< progress bar */
unsigned int scale; /**< current status bar scale */
int width; /**< current status bar width */
This is just the prototype for a new redraw function I added.
@@ -104,7 +104,10 @@
static void ro_gui_status_bar_redraw(wimp_draw *redraw);
static void ro_gui_status_position_progress_bar(struct status_bar *sb);
+/* rik - redraw only on Null poll events */
+static void ro_gui_status_bar_redraw_callback(void *handle);
This is a minor bug fix - to be strictly ANSI compliant you have to
explicitly set pointers to NULL, you can't rely on calloc() doing it for
you.
@@ -136,6 +139,8 @@
sb->scale = width;
sb->visible = true;
+ sb->text = NULL; // calloc doesn't make pointers NULL
+
ro_gui_wimp_event_set_user_data(sb->w, sb);
ro_gui_wimp_event_register_open_window(sb->w,
ro_gui_status_bar_open);
As the string displayed by the status bar is allocated elsewhere, and the
status bar doesn't copy the string, we should not free it when destroying a
status bar.
Also when destroying the status bar, we ensure there are no outstanding
redraw events in the scheduler queue.
@@ -168,10 +173,15 @@
ro_gui_progress_bar_destroy(sb->pb);
- if (sb->text)
- free(sb->text);
+ /* Should this be freed here? We don't allocate this memory */
+ /* It's actually freed in browser_window_destroy_internal */
+// if (sb->text)
+// free(sb->text);
free(sb);
+
+ /* delete any scheduled redraw callbacks */
+ schedule_remove(ro_gui_status_bar_redraw_callback, (void *)sb);
}
This is the redraw function called by the scheduler.
@@ -305,6 +315,14 @@
}
+/* Called by the scheduler */
+static void ro_gui_status_bar_redraw_callback(void *handle) {
+ struct status_bar *sb = (struct status_bar *) handle;
+
+ wimp_force_redraw(sb->w, 0, 0, sb->width - WIDGET_WIDTH, 65536);
+}
These changes to ro_gui_status_bar_set_text() schedule the redraw for 1cs
in the future, rather than performing it immediately. Changes (below) to
the way the scheduler is called means that this ensures the redraw only
happens on a Null poll event.
@@ -314,12 +332,21 @@
{
assert(sb);
sb->text = text;
- /* redraw the window */
- if ((sb->visible) && (text != NULL))
- xwimp_force_redraw(sb->w, 0, 0, sb->width - WIDGET_WIDTH,
65536);
+ /* redraw the window 1 cs later - this is done to ensure
+ * that the redraw is only done on a Null poll event.
+ * schedule() removes any previously scheduled calls to
+ * the same fn with the same parameter.
+ */
+ if ((sb->visible) && (text != NULL)) {
+ schedule(1, ro_gui_status_bar_redraw_callback, (void *)sb);
}
+}
Finally, the changes to how the scheduler is called, we only call it on a
Null poll event.
/**
Index: riscos/gui.c
===================================================================
--- riscos/gui.c (revision 10102)
+++ riscos/gui.c (working copy)
@@ -962,7 +962,10 @@
xhourglass_on();
gui_last_poll = clock();
ro_gui_handle_event(event, &block);
- schedule_run();
+
+ /* rik - only schedule callbacks on a NULL poll event */
+ if (event == wimp_NULL_REASON_CODE) schedule_run();
+
ro_gui_window_update_boxes();
if (browser_reformat_pending && event == wimp_NULL_REASON_CODE)
I've been using a build of Netsurf with these changes for about a week,
without seeing any adverse effects. The scheduler changes don't fix any
immediate problems, but they make Netsurf multitask in a more friendly
manner under RISCOS, and hopefully guard against another 'redraw loop' type
bug being introduced in the future.
--
Rik Griffin
Software Engineer, Denbridge Marine Ltd
Registered in England and Wales at DSG, 43 Castle St, Liverpool. L2 9TL.
Registered Number 4850477.
