From 43896f68ca3ab82848352f348b060a0539bfe844 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Fri, 15 Jan 2016 18:37:08 +0100 Subject: [PATCH 1/7] drawable.surface: Return nil if there is a surface Before this commit, it would return a NULL pointer as a lightuserdata. Signed-off-by: Uli Schlachter --- objects/drawable.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/objects/drawable.c b/objects/drawable.c index 5339529c2..1d4b3e4d9 100644 --- a/objects/drawable.c +++ b/objects/drawable.c @@ -124,8 +124,11 @@ drawable_set_geometry(lua_State *L, int didx, area_t geom) static int luaA_drawable_get_surface(lua_State *L, drawable_t *drawable) { - /* Lua gets its own reference which it will have to destroy */ - lua_pushlightuserdata(L, cairo_surface_reference(drawable->surface)); + if (drawable->surface) + /* Lua gets its own reference which it will have to destroy */ + lua_pushlightuserdata(L, cairo_surface_reference(drawable->surface)); + else + lua_pushnil(L); return 1; } From 2e58a9c6eb7e8f819ed1022e4fe3761a7445070a Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Fri, 15 Jan 2016 18:38:23 +0100 Subject: [PATCH 2/7] awesome.load_image(): Return errors instead of "throwing" them Signed-off-by: Uli Schlachter --- draw.c | 15 +++++---------- draw.h | 3 ++- luaa.c | 16 ++++++++++++---- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/draw.c b/draw.c index d9795bf84..1d720783d 100644 --- a/draw.c +++ b/draw.c @@ -229,23 +229,18 @@ draw_dup_image_surface(cairo_surface_t *surface) /** Load the specified path into a cairo surface * \param L Lua state * \param path file to load + * \param error A place to store an error message, if needed * \return A cairo image surface or NULL on error. */ cairo_surface_t * -draw_load_image(lua_State *L, const char *path) +draw_load_image(lua_State *L, const char *path, GError **error) { - GError *error = NULL; cairo_surface_t *ret; - GdkPixbuf *buf = gdk_pixbuf_new_from_file(path, &error); + GdkPixbuf *buf = gdk_pixbuf_new_from_file(path, error); - if (!buf) { - luaL_where(L, 1); - lua_pushstring(L, error->message); - lua_concat(L, 2); - g_error_free(error); - lua_error(L); + if (!buf) + /* error was set above */ return NULL; - } ret = draw_surface_from_pixbuf(buf); g_object_unref(buf); diff --git a/draw.h b/draw.h index 7ad794141..224a9c5ef 100644 --- a/draw.h +++ b/draw.h @@ -25,6 +25,7 @@ #include #include #include +#include /* for GError */ #include "common/util.h" @@ -70,7 +71,7 @@ a_iso2utf8(const char *str, ssize_t len, char **dest, ssize_t *dlen) cairo_surface_t *draw_surface_from_data(int width, int height, uint32_t *data); cairo_surface_t *draw_dup_image_surface(cairo_surface_t *surface); -cairo_surface_t *draw_load_image(lua_State *L, const char *path); +cairo_surface_t *draw_load_image(lua_State *L, const char *path, GError **error); xcb_visualtype_t *draw_find_visual(const xcb_screen_t *s, xcb_visualid_t visual); xcb_visualtype_t *draw_default_visual(const xcb_screen_t *s); diff --git a/luaa.c b/luaa.c index 446cc7ea9..9653bbbdc 100644 --- a/luaa.c +++ b/luaa.c @@ -147,16 +147,24 @@ luaA_restart(lua_State *L) /** Load an image from a given path. * * @param name The file name. - * @return A cairo surface as light user datum. + * @return[1] A cairo surface as light user datum. + * @return[2] nil + * @treturn[2] string Error message * @function load_image */ static int luaA_load_image(lua_State *L) { + GError *error = NULL; const char *filename = luaL_checkstring(L, 1); - cairo_surface_t *surface = draw_load_image(L, filename); - if (!surface) - return 0; + cairo_surface_t *surface = draw_load_image(L, filename, &error); + if (!surface) { + lua_pushnil(L); + lua_pushstring(L, error->message); + g_error_free(error); + return 2; + } + /* lua has to make sure to free the ref or we have a leak */ lua_pushlightuserdata(L, surface); return 1; From 58ecd92b8e41347156f4781af66ef280f8aec7af Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Fri, 15 Jan 2016 18:27:22 +0100 Subject: [PATCH 3/7] Add better error handling to gears.surface Previously, a Lua error was thrown when loading a file failed. Most callers are not prepared for this and the result is less than optimal. This commit makes the functions print the errors and return nil instead. For callers that want to handle errors themselves, "_silent" variants of the functions are introduced which just return errors to the caller. Signed-off-by: Uli Schlachter --- lib/gears/surface.lua | 69 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/lib/gears/surface.lua b/lib/gears/surface.lua index c7a96d749..598d00b1d 100644 --- a/lib/gears/surface.lua +++ b/lib/gears/surface.lua @@ -9,6 +9,7 @@ local setmetatable = setmetatable local type = type local capi = { awesome = awesome } local cairo = require("lgi").cairo +local gdebug = require("gears.debug") -- Keep this in sync with build-utils/lgi-check.sh! local ver_major, ver_minor, ver_patch = string.match(require('lgi.version'), '(%d)%.(%d)%.(%d)') @@ -19,13 +20,22 @@ end local surface = { mt = {} } local surface_cache = setmetatable({}, { __mode = 'v' }) +local function get_empty_surface() + return cairo.ImageSurface(cairo.Format.ARGB32, 0, 0) +end + --- Try to convert the argument into an lgi cairo surface. -- This is usually needed for loading images by file name. -function surface.load_uncached(_surface) +-- @param _surface The surface to load or nil +-- @param default The default value to return on error; when nil, then a surface +-- in an error state is returned. +-- @return The loaded surface, or the replacement default +-- @return An error message, or nil on success +function surface.load_uncached_silently(_surface, default) local file - -- Nil is not changed + -- On nil, return an empty surface if not _surface then - return nil + return get_empty_surface() end -- Remove from cache if it was cached surface_cache[_surface] = nil @@ -35,8 +45,15 @@ function surface.load_uncached(_surface) end -- Strings are assumed to be file names and get loaded if type(_surface) == "string" then + local err file = _surface - _surface = capi.awesome.load_image(file) + _surface, err = capi.awesome.load_image(file) + if not _surface then + if type(default) == 'nil' then + default = get_empty_surface() + end + return default, err + end end -- Everything else gets forced into a surface _surface = cairo.Surface(_surface, true) @@ -47,14 +64,53 @@ function surface.load_uncached(_surface) return _surface end -function surface.load(_surface) +--- Try to convert the argument into an lgi cairo surface. +-- This is usually needed for loading images by file name and uses a cache. +-- In contrast to `load()`, errors are returned to the caller. +-- @param _surface The surface to load or nil +-- @param default The default value to return on error; when nil, then a surface +-- in an error state is returned. +-- @return The loaded surface, or the replacement default, or nil if called with +-- nil. +-- @return An error message, or nil on success +function surface.load_silently(_surface, default) if type(_surface) == "string" then local cache = surface_cache[_surface] if cache then return cache end end - return surface.load_uncached(_surface) + return surface.load_uncached_silently(_surface, default) +end + +local function do_load_and_handle_errors(_surface, func) + if type(_surface) == 'nil' then + return get_empty_surface() + end + local result, err = func(_surface, false) + if result then + return result + end + gdebug.print_error("Failed to load '" .. tostring(_surface) .. "': " .. tostring(err)) + return get_empty_surface() +end + +--- Try to convert the argument into an lgi cairo surface. +-- This is usually needed for loading images by file name. Errors are handled +-- via `gears.debug.print_error`. +-- @param _surface The surface to load or nil +-- @return The loaded surface, or nil +function surface.load_uncached(_surface) + return do_load_and_handle_errors(_surface, surface.load_uncached_silently) +end + +--- Try to convert the argument into an lgi cairo surface. +-- This is usually needed for loading images by file name. Errors are handled +-- via `gears.debug.print_error`. +-- @param _surface The surface to load or nil +-- @return The loaded surface, or nil +function surface.load(_surface) + return do_load_and_handle_errors(_surface, surface.load_silently) end function surface.mt:__call(...) @@ -70,7 +126,6 @@ function surface.get_size(surf) return w - x, h - y end - --- Create a copy of a cairo surface. -- The surfaces returned by `surface.load` are cached and must not be -- modified to avoid unintended side-effects. This function allows to create From 86f2d05382edac78567104e0c993721578fb31e0 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Fri, 15 Jan 2016 18:43:10 +0100 Subject: [PATCH 4/7] awful.menu: Remove useless surface() call The imagebox calls surface() for us. Signed-off-by: Uli Schlachter --- lib/awful/menu.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/awful/menu.lua b/lib/awful/menu.lua index ca8a90f33..c4a0af0f5 100644 --- a/lib/awful/menu.lua +++ b/lib/awful/menu.lua @@ -558,7 +558,7 @@ function menu.entry(parent, args) if type(args.cmd) == "table" then if args.theme.submenu_icon then submenu = wibox.widget.imagebox() - submenu:set_image(surface.load(args.theme.submenu_icon)) + submenu:set_image(args.theme.submenu_icon) else submenu = wibox.widget.textbox() submenu:set_font(args.theme.font) From 4d562306d247a81849eac1d8b7713c252e4c022f Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Fri, 15 Jan 2016 18:43:56 +0100 Subject: [PATCH 5/7] taglist: Remove dead code The image "library" is long gone. Signed-off-by: Uli Schlachter --- lib/awful/widget/taglist.lua | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/awful/widget/taglist.lua b/lib/awful/widget/taglist.lua index 46ac8b7f1..f632a8618 100644 --- a/lib/awful/widget/taglist.lua +++ b/lib/awful/widget/taglist.lua @@ -107,9 +107,7 @@ function taglist.taglist_label(t, args) text = text .. "" end if not taglist_disable_icon then - if tag.geticon(t) and type(tag.geticon(t)) == "image" then - icon = tag.geticon(t) - elseif tag.geticon(t) then + if tag.geticon(t) then icon = surface.load(tag.geticon(t)) end end From 9d97a98b4df4e2d2d683ae50c32a6d1c68a817f1 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Fri, 15 Jan 2016 18:45:44 +0100 Subject: [PATCH 6/7] naughty: Remove dead code Since some commits, surface.load_uncached() handles errors itself and thus we don't have to print an error message here any more. Signed-off-by: Uli Schlachter --- lib/naughty/core.lua | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/naughty/core.lua b/lib/naughty/core.lua index ee41e72b7..4c30f0311 100644 --- a/lib/naughty/core.lua +++ b/lib/naughty/core.lua @@ -562,13 +562,7 @@ function naughty.notify(args) end -- is the icon file readable? - local success, res = pcall(function() return surface.load_uncached(icon) end) - if success then - icon = res - else - io.stderr:write(string.format("naughty: Couldn't load image '%s': %s\n", tostring(icon), res)) - icon = nil - end + local icon = surface.load_uncached(icon) -- if we have an icon, use it if icon then From 76b1d348a15f2a81f14ab4feee369e50fbe2a41c Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Fri, 15 Jan 2016 18:47:09 +0100 Subject: [PATCH 7/7] imagebox: Remove dead code Since some commits, surface.load() handles printing an error message for us. Signed-off-by: Uli Schlachter --- lib/wibox/widget/imagebox.lua | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/wibox/widget/imagebox.lua b/lib/wibox/widget/imagebox.lua index 14eb1315a..40e4a6443 100644 --- a/lib/wibox/widget/imagebox.lua +++ b/lib/wibox/widget/imagebox.lua @@ -72,17 +72,16 @@ end --- Set an imagebox' image -- @param image Either a string or a cairo image surface. A string is -- interpreted as the path to a png image file. +-- @return true on success, false if the image cannot be used function imagebox:set_image(image) local image = image if type(image) == "string" then - local success, result = pcall(surface.load, image) - if not success then - print("Error while reading '" .. image .. "': " .. result) + image = surface.load(image) + if not image then print(debug.traceback()) return false end - image = result end image = surface.load(image)