GNOME Bugzilla – Bug 696730
Compile failure with 1.14.0 due to extra arg in bitmap functions
Last modified: 2013-04-04 15:32:49 UTC
Created attachment 239987 [details] [review] Patch to fix broken build Compilation fails with 1.14.0 using configure flags: --enable-introspection=no --enable-gdk-pixbuf=no --enable-quartz-image=yes --without-x --enable-glx=no This is because cogl-bitmap.c has added an "error" parameter to some functions, but that error parameter is not being passed in calls from cogl-bitmap-pixbuf.c The attached patch passes a "NULL" for the error parameter (ie ignores errors). Better would be to actually check for errors and do something appropriate probably...
Review of attachment 239987 [details] [review]: looks good to me.
Thanks for the patch. I'm about to attach another version of the patch which propagates the error out of the function. I also ran into a problem while testing it where it wouldn't compile because the quartz image functions are unresolved. I'm attaching a second patch which addresses this. I'm not sure why no-one has complained about this before so I don't know if it's something specific with my setup.
Created attachment 240069 [details] [review] quartz-image: Pass a CoglError argument to the bitmap functions Since 67cad9c0 and f7735e141a the bitmap allocation and mapping functions now take an extra error argument. The quartz image backend was missed in this update so Cogl would fail to compile if --enable-quartz-image is used.
Created attachment 240070 [details] [review] quartz-image: Pass -framework ApplicationServices in the linker flags The quartz image backend is using functions from the ApplicationServices framework and I was getting linker errors if I didn't use this option. I'm not sure how anyone managed to build it before without this.
I use --enable-quartz-backend but not --enable-quartz-image, so that's how I guess I missed your problem :) I actually add all these to LDFLAGS to get cogl to link correctly: -framework AppKit -framework CoreFoundation -framework CoreGraphics
I'm sorry, ignore that part about no quartz image; I am using that :)
Review of attachment 240069 [details] [review]: That *partially* propagates the error out. It'd probably be best to at least log the error with a g_warning or g_error. Freeing the image is definitely a good thing to do :)
Review of attachment 240070 [details] [review]: Yes, this seems to work. Does not need the other frameworks I specified if this one is included.
(In reply to comment #7) > That *partially* propagates the error out. It'd probably be best to at least > log the error with a g_warning or g_error. Freeing the image is definitely a > good thing to do :) The error is passed up outside of _cogl_bitmap_from_file using the **error argument so it will be passed right out to the application. That way the application has an opportunity to recover from the error or it can print its own warning. The application can also pass in NULL for the error parameter which will cause it to print a warning and abort. I don't think there's any need to use g_warning directly here because that will be annoying for applications that can recover.
Review of attachment 240069 [details] [review]: Doh! Never mind, this is why I shouldn't review bugs before my morning coffee. I thought you'd created a local error instead of using one that was already passed in.
Neil, these patches look good to land to me
Thanks, I've pushed them to master and the cogl-1.14 branch.