From 67cf1469f01267d0aa8b2839a970d91fd965099b Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Fri, 25 Jan 2019 15:16:17 +0100 Subject: [PATCH 1/3] Change the way the shape is done in the background container Previously, the background container "just" used the shape and drew a line around it. This means that half the line will be inside of the shape and half of it will be outside. Thus, this hides the actual shape that is used. This commit changes that so that the line is added outside of the shape. It does this via some tricks: - In :before_draw_children(), :push_group() is used to redirect drawing of the child widget to a temporary surface. - In :after_draw_children(), the border is added to this group. + For this, another temporary surface is created. It will be used as a mask. + The inside of the shape on this mask is cleared, everything else is filled. Thus, the mask now contains everything "not content". + Everything inside the mask is filled with the background color. - Also in :after_draw_children(), the group is drawn to the actual target surface. + Again, this needs a mask. + This time, we draw the shape to the mask with twice the border width. Thus, half of this line will be outside of the shape. + Then, the shape itself is also filled so that the mask contains the shape and the border. + This mask is then used to copy the right parts of the temporary surface were the child widget and border was drawn to the actual target surface that will be visible on screen. This approach has some upsides. Because we no longer have "half the border" above content, colors with some transparency work fine for the border. Also, this should avoid issues with anti-aliasing, because e.g. the border is not just drawn with the border width, but also further out to everything else so that the background cannot "bleed through". Fixes: https://github.com/awesomeWM/awesome/issues/2516 Signed-off-by: Uli Schlachter --- lib/wibox/container/background.lua | 108 +++++++++++++++++++---------- 1 file changed, 72 insertions(+), 36 deletions(-) diff --git a/lib/wibox/container/background.lua b/lib/wibox/container/background.lua index ed533b49..3ecbffaa 100644 --- a/lib/wibox/container/background.lua +++ b/lib/wibox/container/background.lua @@ -20,30 +20,28 @@ local unpack = unpack or table.unpack -- luacheck: globals unpack (compatibility local background = { mt = {} } --- Draw this widget -function background:draw(context, cr, width, height) - if not self._private.widget or not self._private.widget:get_visible() then - return +-- Make sure a surface pattern is freed *now* +local function dispose_pattern(pattern) + local status, s = pattern:get_surface() + if status == "SUCCESS" then + s:finish() end +end - -- Keep the shape path in case there is a border - self._private.path = nil +-- Prepare drawing the children of this widget +function background:before_draw_children(context, cr, width, height) + local source = self._private.foreground or cr:get_source() + -- Redirect drawing to a temporary surface if there is a shape if self._private.shape then - -- Only add the offset if there is something to draw - local offset = ((self._private.shape_border_width and self._private.shape_border_color) - and self._private.shape_border_width or 0) / 2 - - cr:translate(offset, offset) - self._private.shape(cr, width - 2*offset, height - 2*offset, unpack(self._private.shape_args or {})) - cr:translate(-offset, -offset) - self._private.path = cr:copy_path() - cr:clip() + cr:push_group_with_content(cairo.Content.COLOR_ALPHA) end + -- Draw the background if self._private.background then cr:set_source(self._private.background) - cr:paint() + cr:rectangle(0, 0, width, height) + cr:fill() end if self._private.bgimage then if type(self._private.bgimage) == "function" then @@ -51,36 +49,74 @@ function background:draw(context, cr, width, height) else local pattern = cairo.Pattern.create_for_surface(self._private.bgimage) cr:set_source(pattern) - cr:paint() + cr:rectangle(0, 0, width, height) + cr:fill() end end + cr:set_source(source) end -- Draw the border -function background:after_draw_children(_, cr) - -- Draw the border - if self._private.path and self._private.shape_border_width and self._private.shape_border_width > 0 then - cr:append_path(self._private.path) +function background:after_draw_children(_, cr, width, height) + if not self._private.shape then + return + end + + -- Okay, there is a shape. Get it as a path. + local bw = self._private.shape_border_width or 0 + + cr:translate(bw, bw) + self._private.shape(cr, width - 2*bw, height - 2*bw, unpack(self._private.shape_args or {})) + cr:translate(-bw, -bw) + + if bw > 0 then + -- Now we need to do a border, somehow. We begin with another + -- temporary surface. + cr:push_group_with_content(cairo.Content.ALPHA) + + -- Mark everything as "this is border" + cr:set_source_rgba(0, 0, 0, 1) + cr:paint() + + -- Now remove the inside of the shape to get just the border + cr:set_operator(cairo.Operator.SOURCE) + cr:set_source_rgba(0, 0, 0, 0) + cr:fill_preserve() + + local mask = cr:pop_group() + -- Now actually draw the border via the mask we just created. cr:set_source(color(self._private.shape_border_color or self._private.foreground or beautiful.fg_normal)) + cr:set_operator(cairo.Operator.SOURCE) + cr:mask(mask) - cr:set_line_width(self._private.shape_border_width) - cr:stroke() - self._private.path = nil - end -end - --- Prepare drawing the children of this widget -function background:before_draw_children(_, cr) - if self._private.foreground then - cr:set_source(self._private.foreground) + dispose_pattern(mask) end - -- Clip the shape - if self._private.path and self._private.shape_clip then - cr:append_path(self._private.path) - cr:clip() - end + -- We now have the right content in a temporary surface. Copy it to the + -- target surface. For this, we need another mask + cr:push_group_with_content(cairo.Content.ALPHA) + + -- Draw the border with 2 * border width (this draws both + -- inside and outside, only half of it is outside) + cr.line_width = 2 * bw + cr:set_source_rgba(0, 0, 0, 1) + cr:stroke_preserve() + + -- Now fill the whole inside so that it is also include in the mask + cr:fill() + + local mask = cr:pop_group() + local source = cr:pop_group() -- This pops what was pushed in before_draw_children + + -- This now draws the content of the background widget to the actual + -- target, but only the part that is inside the mask + cr:set_operator(cairo.Operator.OVER) + cr:set_source(source) + cr:mask(mask) + + dispose_pattern(mask) + dispose_pattern(source) end -- Layout this widget From ba75da7976e6a31ee6898243475ecb7807399150 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 27 Jan 2019 11:30:08 +0100 Subject: [PATCH 2/3] Work around a bug in LGI cairo_get_source() is not bound correctly, leading to use-after-free bugs. Cairo catches this and crashes. Work around this by preserving the current source in a different way. Instead of using cairo_get_source() and later cairo_set_source(), this commit wraps everything that changes the current source between cairo_save() and cairo_restore(). Thus, cairo saves the current source for us without us having to grab an explicit reference. Works-around: https://github.com/pavouk/lgi/issues/210 Signed-off-by: Uli Schlachter --- lib/wibox/container/background.lua | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/wibox/container/background.lua b/lib/wibox/container/background.lua index 3ecbffaa..4e30042b 100644 --- a/lib/wibox/container/background.lua +++ b/lib/wibox/container/background.lua @@ -30,8 +30,6 @@ end -- Prepare drawing the children of this widget function background:before_draw_children(context, cr, width, height) - local source = self._private.foreground or cr:get_source() - -- Redirect drawing to a temporary surface if there is a shape if self._private.shape then cr:push_group_with_content(cairo.Content.COLOR_ALPHA) @@ -39,11 +37,14 @@ function background:before_draw_children(context, cr, width, height) -- Draw the background if self._private.background then + cr:save() cr:set_source(self._private.background) cr:rectangle(0, 0, width, height) cr:fill() + cr:restore() end if self._private.bgimage then + cr:save() if type(self._private.bgimage) == "function" then self._private.bgimage(context, cr, width, height,unpack(self._private.bgimage_args)) else @@ -52,9 +53,8 @@ function background:before_draw_children(context, cr, width, height) cr:rectangle(0, 0, width, height) cr:fill() end + cr:restore() end - - cr:set_source(source) end -- Draw the border From 84eb175ccd239b37d01381c87e34912c08af7171 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Mon, 28 Jan 2019 14:55:53 +0100 Subject: [PATCH 3/3] background container: Deprecate shape_clip property The previous commit removed the implementation of this property. Signed-off-by: Uli Schlachter --- lib/wibox/container/background.lua | 16 ++++----- .../wibox/container/background/clip.lua | 35 ------------------- 2 files changed, 6 insertions(+), 45 deletions(-) delete mode 100644 tests/examples/wibox/container/background/clip.lua diff --git a/lib/wibox/container/background.lua b/lib/wibox/container/background.lua index 4e30042b..c4ee51ff 100644 --- a/lib/wibox/container/background.lua +++ b/lib/wibox/container/background.lua @@ -268,20 +268,16 @@ function background:get_shape_border_color() return self._private.shape_border_color end ---- When a `shape` is set, make sure nothing is drawn outside of it. ---@DOC_wibox_container_background_clip_EXAMPLE@ --- @property shape_clip --- @tparam boolean value If the shape clip is enable - function background:set_shape_clip(value) - if self._private.shape_clip == value then return end - - self._private.shape_clip = value - self:emit_signal("widget::redraw_needed") + if value then return end + require("gears.debug").print_warning("shape_clip property of background container was removed." + .. " Use wibox.layout.stack instead if you want shape_clip=false.") end function background:get_shape_clip() - return self._private.shape_clip or false + require("gears.debug").print_warning("shape_clip property of background container was removed." + .. " Use wibox.layout.stack instead if you want shape_clip=false.") + return true end --- The background image to use diff --git a/tests/examples/wibox/container/background/clip.lua b/tests/examples/wibox/container/background/clip.lua deleted file mode 100644 index 0e7d84c7..00000000 --- a/tests/examples/wibox/container/background/clip.lua +++ /dev/null @@ -1,35 +0,0 @@ ---DOC_GEN_IMAGE --DOC_HIDE -local parent = ... --DOC_HIDE -local wibox = require("wibox") --DOC_HIDE -local gears = {shape = require("gears.shape")} --DOC_HIDE -local beautiful = require("beautiful") --DOC_HIDE - -parent : setup { - { - -- Some content may be outside of the shape - { - text = "Hello\nworld!", - widget = wibox.widget.textbox - }, - shape = gears.shape.circle, - bg = beautiful.bg_normal, - shape_border_color = beautiful.border_color, - widget = wibox.container.background - }, - { - -- To solve this, clip the content - { - text = "Hello\nworld!", - widget = wibox.widget.textbox - }, - shape_clip = true, - shape = gears.shape.circle, - bg = beautiful.bg_normal, - shape_border_color = beautiful.border_color, - widget = wibox.container.background - }, - spacing = 10, - layout = wibox.layout.fixed.vertical -} - ---DOC_HIDE vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80