io-webp.c: update_func() shall be called for proper signal delivery.

[tsutsui]

After this commit 36b3df08 (after 0.2.1), GdkPixBuf PixbufModuleUpdatedFunc (i.e. context->update_func()) is no longer called in either load_increment() or stop_load().

As GdkPixBuf manual pages say, GdkPixbufLoader uses PixbufModulePreparedFunc to emit the “area_prepared” signal and also uses PixbufModuleUpdatedFunc to emit the “area_updated” signal:
https://docs.gtk.org/gdk-pixbuf/callback.PixbufModulePreparedFunc.html
https://docs.gtk.org/gdk-pixbuf/callback.PixbufModuleUpdatedFunc.html

So if a rendering window is in the main thread and loading and decoding ops are in the background process, the main thread will never be notified completion of decoding in the background process and none will be rendered, unless “area_updated” signal is delivered.

[tsutsui]

FYI, the following mingw webp patch for gdk-pixbuf seems to support incremental load with update_func.
https://github.com/tumagonx/pygi-mingw-patches/blob/master/gdk-pixbuf-2.32.x.patch#L1437

[aruiz]

Hey, thanks a lot for working on this, please bear with me as I don't have a lot of availability for reviews during August. I will look into this once I am back from vacation in September.

[aruiz]

Okay, existing tests pass on the pipeline.

Do you have a test case that would fail with the current implementation and passes with your fix? I rather have a regression test with these merges.

[tsutsui]

My application is "mikutter" that use ruby-gdk_pixbuf2.
However I've also tried geeqie and it looks my changes don't fix the issue mentioned in BestImageViewer/geeqie#1076 (webp-pixbuf-loader 0.2.1 works but not 0.2.4).
I'll check how each signal should be delivered.

[tsutsui]

As I noted in BestImageViewer/geeqie#1076 (comment) , geeqie assumes

• PixbufModulePreparedFunc should be called after pixbuf is prepared and before loaded data is copied into the pixbuf
• PixbufModuleUpdatedFunc should be called after loaded data is copied into the pixbuf

I'm not sure what the GdkPixbuf APIs assume, but the attached patch (against mainline) seems to fix the geeqie issue.

diff --git a/io-webp.c b/io-webp.c
index c67e0a9..de2f83a 100644
--- a/io-webp.c
+++ b/io-webp.c
@@ -143,6 +143,8 @@ stop_load (gpointer data, GError **error)
         {
           if (context->prepare_func)
             context->prepare_func (pb, GDK_PIXBUF_ANIMATION (anim), context->user_data);
+          if (context->update_func)
+            context->update_func (pb, 0, 0, context->width, context->height, context->user_data);
           ret = TRUE;
         }
 
@@ -174,6 +176,9 @@ stop_load (gpointer data, GError **error)
           return FALSE;
         }
 
+      if (context->prepare_func)
+        context->prepare_func (pb, NULL, context->user_data);
+
       if (icc_data)
         {
           gdk_pixbuf_set_option (pb, "icc-profile", icc_data);
@@ -187,10 +192,8 @@ stop_load (gpointer data, GError **error)
                                          context->buffer->len, &config);
       if (status == VP8_STATUS_OK)
         {
-          if (context->prepare_func)
-            context->prepare_func (pb, NULL, context->user_data);
-
-          g_clear_object (&pb);
+          if (context->update_func)
+            context->update_func (pb, 0, 0, context->width, context->height, context->user_data);
 
           ret = TRUE;
         }
@@ -198,6 +201,8 @@ stop_load (gpointer data, GError **error)
         g_set_error (error, GDK_PIXBUF_ERROR, GDK_PIXBUF_ERROR_FAILED,
                           "WebP decoder failed with VP8 status code: %d", status);
       }
+
+      g_clear_object (&pb);
     }
 
   if (context->buffer)

Maybe we need proper tests per callback API definitions on each signal, like https://tmytokai.github.io/open-ed/activity/gtkmm/img/p19.htm for PixbufModuleSizeFunc case.

[tsutsui]

I've updated the branch per previous patch.
https://docs.gtk.org/gdk-pixbuf/callback.PixbufModulePreparedFunc.html says
Defines the type of the function that gets called once the initial setup of pixbuf is done.
so I guess PixbufModulePreparedFunc might update passed pixbuf and it should be called before decoding.

[dimich-dmb]

I can confirm this patch fixes geeqie .webp images displaying issue.

[caclark]

I do some work for the Geeqie project, which has an interest in this patch.
Is there perhaps something that should be changed in the Geeqie code to allow webp images to be displayed?

[aruiz]

Sorry it took a while, I rebased this and pushed it to mainline. Will be making a release soon.

Thanks for working on this!