GNOME Bugzilla – Bug 339318
Allow page rendering to (optionally) happen in a thread
Last modified: 2008-12-19 16:54:36 UTC
It would be nice if there was a way to optionally make page rendering when printing happen on a thread. That way threadsafe applications could avoid to lock up the window for a potentially long time.
I'm thinking about using gtkprintoperation in Evince, at least for the PDF backend, instead of exporting to a PS file. The problem is that with some documents pages take too long to render and I don't want to block the main loop while printing. I would like to fix this, do you have already an idea about how to implement it?
I've thinking about how to implement this. I think something like this might work. GtkPrintOperationResult gtk_print_operation_run_async (GtkPrintOperation *op, GtkPrintOperationAction action, GtkWindow *parent, GtkPrintOperationDrawPage callback, GError **error); This new method might be a little bit confusing because we already have gtk_print_operation_set_allow_async(), so I'm not sure about the name. In any case, the idea is to provide a callback that is called instead of emitting the draw-page signal. In such a callback the user could send the cairo context to the rendering thread and once the page is rendered, it would call gtk_print_operation_draw_page_finished(). What do you guys think?
Created attachment 122184 [details] [review] a threaded page drawing proposal Hi all, here is my proposal of implementation of drawing pages in another thread. There are three new functions: gtk_print_operation_set_allow_threads () gtk_print_operation_get_allow_threads () gtk_print_operation_page_drawing_finished (). The first function allows GtkPrintOperation to use threads for page drawing. It should be called before gtk_print_operation_run. The third function has to be called at the end of drawing of given page. I'll send an example asap. Regards Marek
Created attachment 122185 [details] the example This is the promised example. It uses pthreads. Marek
Any chance of having this fixed before for the next gtk+ unstable release? I would really like to rework the printing stuff in evince for 2.26.
Comment on attachment 122184 [details] [review] a threaded page drawing proposal As a general comment before going into the details of the patch, I don't really like the allow_threads boolean. My ideal api would be if the ::draw-page handler would be required to call drawing_finished(). Then rendering in a thread would simply consist in not calling drawing_finished() from the signal handler, but from a thread instead. Since that is not compatible with the current ::draw-page semantics, how about we add another method, defer_drawing(), to tell the printoperation that it needs to wait for a drawing_finished() call before considering the page complete. >+/** >+ * gtk_print_operation_page_drawing_finished: >+ * @op: a #GtkPrintOperation >+ * >+ * Finish drawing of page. >+ * >+ * Since: 2.16 >+ **/ Once we have settled on the api, this doc comment needs quite a bit more detail. >+void >+gtk_print_operation_page_drawing_finished (GtkPrintOperation *op) >+{ >+ GtkPrintOperationPrivate *priv = op->priv; >+ GtkPageSetup *page_setup; >+ GtkPrintContext *print_context; >+ cairo_t *cr; >+ >+ print_context = priv->print_context; >+ page_setup = gtk_print_context_get_page_setup (print_context); >+ >+ cr = gtk_print_context_get_cairo_context (print_context); >+ >+ priv->end_page (op, print_context); >+ >+ cairo_restore (cr); >+ >+ g_object_unref (page_setup); >+ >+ if (gtk_print_operation_get_allow_threads (op)) >+ priv->is_drawing_page = FALSE; I think we should set is_drawing_page unconditionally here. No need to check for allow_threads. >-static gboolean >-print_pages_idle (gpointer user_data) >+void >+prepare_data (PrintPagesData *data) > { >- PrintPagesData *data; >- GtkPrintOperationPrivate *priv; >- GtkPageSetup *page_setup; >- gboolean done = FALSE; >- gint i; >+ GtkPrintOperationPrivate *priv; >+ GtkPageSetup *page_setup; >+ gint i; > >- data = (PrintPagesData*)user_data; > priv = data->op->priv; > >- if (priv->status == GTK_PRINT_STATUS_PREPARING) >+ if (!data->initialized) > { >- if (!data->initialized) >- { >- data->initialized = TRUE; >- page_setup = create_page_setup (data->op); >- _gtk_print_context_set_page_setup (priv->print_context, >- page_setup); >- g_object_unref (page_setup); >- >- g_signal_emit (data->op, signals[BEGIN_PRINT], 0, priv->print_context); >- >- if (priv->manual_collation) >- { >- data->uncollated_copies = priv->manual_num_copies; >- data->collated_copies = 1; >- } >- else >- { >- data->uncollated_copies = 1; >- data->collated_copies = priv->manual_num_copies; >- } >+ data->initialized = TRUE; >+ page_setup = create_page_setup (data->op); >+ _gtk_print_context_set_page_setup (priv->print_context, >+ page_setup); >+ g_object_unref (page_setup); > >- goto out; >- } >- >- if (g_signal_has_handler_pending (data->op, signals[PAGINATE], 0, FALSE)) >- { >- gboolean paginated = FALSE; >+ g_signal_emit (data->op, signals[BEGIN_PRINT], 0, priv->print_context); > >- g_signal_emit (data->op, signals[PAGINATE], 0, priv->print_context, &paginated); >- if (!paginated) >- goto out; >- } >- >- /* Initialize parts of PrintPagesData that depend on nr_of_pages >- */ >- if (priv->print_pages == GTK_PRINT_PAGES_RANGES) >- { >- if (priv->page_ranges == NULL) >- { >- g_warning ("no pages to print"); >- priv->cancelled = TRUE; >- goto out; >- } >- data->ranges = priv->page_ranges; >- data->num_ranges = priv->num_page_ranges; >- for (i = 0; i < data->num_ranges; i++) >- if (data->ranges[i].end == -1 || >- data->ranges[i].end >= priv->nr_of_pages) >- data->ranges[i].end = priv->nr_of_pages - 1; >- } >- else if (priv->print_pages == GTK_PRINT_PAGES_CURRENT && >- priv->current_page != -1) >- { >- data->ranges = &data->one_range; >- data->num_ranges = 1; >- data->ranges[0].start = priv->current_page; >- data->ranges[0].end = priv->current_page; >- } >+ if (priv->manual_collation) >+ { >+ data->uncollated_copies = priv->manual_num_copies; >+ data->collated_copies = 1; >+ } > else >- { >- data->ranges = &data->one_range; >- data->num_ranges = 1; >- data->ranges[0].start = 0; >- data->ranges[0].end = priv->nr_of_pages - 1; >- } >- >- clamp_page_ranges (data); >+ { >+ data->uncollated_copies = 1; >+ data->collated_copies = priv->manual_num_copies; >+ } > >- if (priv->manual_reverse) >- { >- data->range = data->num_ranges - 1; >- data->inc = -1; >- } >- else >- { >- data->range = 0; >- data->inc = 1; >- } >- find_range (data); >- >- /* go back one page, since we preincrement below */ >- data->page = data->start - data->inc; >- data->collated = data->collated_copies - 1; >+ return; >+ } > >- _gtk_print_operation_set_status (data->op, >- GTK_PRINT_STATUS_GENERATING_DATA, >- NULL); >+ if (g_signal_has_handler_pending (data->op, signals[PAGINATE], 0, FALSE)) >+ { >+ gboolean paginated = FALSE; > >- goto out; >+ g_signal_emit (data->op, signals[PAGINATE], 0, priv->print_context, &paginated); >+ if (!paginated) >+ return; > } > >- data->total++; >- data->collated++; >- if (data->collated == data->collated_copies) >+ /* Initialize parts of PrintPagesData that depend on nr_of_pages >+ */ >+ if (priv->print_pages == GTK_PRINT_PAGES_RANGES) > { >- data->collated = 0; >- if (!increment_page_sequence (data)) >- { >- done = TRUE; >- >- goto out; >- } >+ if (priv->page_ranges == NULL) >+ { >+ g_warning ("no pages to print"); >+ priv->cancelled = TRUE; >+ return; >+ } >+ data->ranges = priv->page_ranges; >+ data->num_ranges = priv->num_page_ranges; >+ for (i = 0; i < data->num_ranges; i++) >+ if (data->ranges[i].end == -1 || >+ data->ranges[i].end >= priv->nr_of_pages) >+ data->ranges[i].end = priv->nr_of_pages - 1; > } >- >- if (data->is_preview && !priv->cancelled) >+ else if (priv->print_pages == GTK_PRINT_PAGES_CURRENT && >+ priv->current_page != -1) > { >- done = TRUE; >- >- g_signal_emit_by_name (data->op, "ready", priv->print_context); >- goto out; >+ data->ranges = &data->one_range; >+ data->num_ranges = 1; >+ data->ranges[0].start = priv->current_page; >+ data->ranges[0].end = priv->current_page; > } >+ else >+ { >+ data->ranges = &data->one_range; >+ data->num_ranges = 1; >+ data->ranges[0].start = 0; >+ data->ranges[0].end = priv->nr_of_pages - 1; >+ } > I couldn't really follow all these changes. What is going on here ? Just some rearrangement that caused lots of indentation changes, or something substantial ? >+static gboolean >+print_pages_idle (gpointer user_data) >+{ >+ PrintPagesData *data; >+ GtkPrintOperationPrivate *priv; >+ gboolean done = FALSE; >+ >+ data = (PrintPagesData*)user_data; >+ priv = data->op->priv; >+ >+ if (!(gtk_print_operation_get_allow_threads (data->op) && (priv->is_drawing_page))) I think instead of a simple idle that checks the is_drawing_page flag, essentially in a busy loop, you may want to go to an approach where drawing_finished re-adds the idle. Not sure. >+ >+ if (gtk_print_operation_get_allow_threads (data->op)) >+ priv->is_drawing_page = TRUE; Again, I think this should be set unconditionally. Also, why is this not done inside common_render_page ? I wonder if the other caller of common_render_page() (for rendering previews) needs some adjustment too ? >+ >+ common_render_page (data->op, data->page); >+
Hi, I changed the API in the way mentioned in the comment #6. To print in another thread, you need to call gtk_print_operation_defer_drawing i n the "draw-page" handler. After the page is completed you have to call gtk_prin t_operation_page_drawing_finished. Unfortunatelly I needed more states than 2 (in is_drawing_page), so I created a new enum GtkPageDrawingState with 3 states (ready, drawing, deferred_drawing). T his is enough to express state for controlling thread printing. I extended comments of those 2 new functions. The big portion of the code were only indentation changes + two conditions. There were quite a big portion of independent code. So, I moved it to a separated function prepare_data to simplify print_pages_idle function. I also modified the code for preview. The readding of idle would include change of _idle_done functions for preview and for real printing. It shouldn't be changed for non-thread printing. It would be helpful to have some function which pauses idle, but I didn't find any. Marek
Created attachment 124992 [details] [review] the modified patch
Created attachment 124994 [details] [review] the modified patch without indentation changes
hmm, I would suggest: gtk_print_operation_defer_drawing -> gtk_print_operation_set_defer_drawing gtk_print_operation_page_drawing_finished -> gtk_print_operation_draw_page_finish What happens during the actual page rendering with the print_pages_idle? does it always return TRUE so that it's running all the time until the page is rendered? If so, I would also suggest to return FALSE when common_render_page is called and reschedule the idle in the finished method.
Created attachment 125001 [details] an example
The patch in comment 9 looks pretty good. The naming changes that Carlos proposed are probaby a good idea. Some details: - It would be good to have g_return_if_fail (priv->page_drawing_state == GTK_PAGE_DRAWING_STATE_DRAWING) in gtk_print_operation_defer_drawing, to give people a warning if they are calling it from the wrong context. - In doc comments, please add () after function names (without a space) to get proper cross-referencing.
Hi all, I committed the patch modified according to the comments #10 and #12. I didn't include the remove-add idle way yet. But I'll file a new feature request for this and I'll return to it later. Thanks you for your comments Marek