Merge pull request #1056 from Elv13/better_tag_index

Test multiple screens on the CI + fixes
This commit is contained in:
Emmanuel Lepage Vallée 2016-09-07 01:23:58 -04:00 committed by GitHub
commit 7a4bdca9c2
9 changed files with 780 additions and 32 deletions

View File

@ -122,20 +122,41 @@ function ewmh.activate(c, context, hints) -- luacheck: no unused args
end
end
-- Get tags that are on the same screen as the client. This should _almost_
-- always return the same content as c:tags().
local function get_valid_tags(c, s)
local tags, new_tags = c:tags(), {}
for _, t in ipairs(tags) do
if screen[s] == t.screen then
table.insert(new_tags, t)
end
end
return new_tags
end
--- Tag a window with its requested tag.
--
-- It is the default signal handler for `request::tag` on a `client`.
--
-- @signalhandler awful.ewmh.tag
-- @client c A client to tag
-- @tag[opt] t A tag to use. If omitted, then the client is made sticky.
-- @tparam[opt] tag|boolean t A tag to use. If true, then the client is made sticky.
-- @tparam[opt={}] table hints Extra information
function ewmh.tag(c, t, hints) --luacheck: no unused
-- There is nothing to do
if not t and #c:tags() > 0 then return end
if not t and #get_valid_tags(c, c.screen) > 0 then return end
if not t then
c:to_selected_tags()
if c.transient_for then
c.screen = c.transient_for.screen
if not c.sticky then
c:tags(c.transient_for:tags())
end
else
c:to_selected_tags()
end
elseif type(t) == "boolean" and t then
c.sticky = true
else

View File

@ -12,6 +12,7 @@ local util = require("awful.util")
local ascreen = require("awful.screen")
local beautiful = require("beautiful")
local object = require("gears.object")
local timer = require("gears.timer")
local pairs = pairs
local ipairs = ipairs
local table = table
@ -259,15 +260,14 @@ end
-- stickied tags to.
-- @tparam[opt=false] boolean force Move even non-sticky clients to the fallback
-- tag.
-- @return Returns true if the tag is successfully deleted, nil otherwise.
-- @return Returns true if the tag is successfully deleted.
-- If there are no clients exclusively on this tag then delete it. Any
-- stickied clients are assigned to the optional 'fallback_tag'.
-- If after deleting the tag there is no selected tag, try and restore from
-- history or select the first tag on the screen.
function tag.object.delete(self, fallback_tag, force)
-- abort if the taf isn't currently activated
if not self.activated then return end
if not self.activated then return false end
local target_scr = get_screen(tag.getproperty(self, "screen"))
local tags = target_scr.tags
@ -275,7 +275,7 @@ function tag.object.delete(self, fallback_tag, force)
local ntags = #tags
-- We can't use the target tag as a fallback.
if fallback_tag == self then return end
if fallback_tag == self then return false end
-- No fallback_tag provided, try and get one.
if fallback_tag == nil then
@ -284,7 +284,7 @@ function tag.object.delete(self, fallback_tag, force)
-- Abort if we would have un-tagged clients.
local clients = self:clients()
if ( #clients > 0 and ntags <= 1 ) or fallback_tag == nil then return end
if #clients > 0 and fallback_tag == nil then return false end
-- Move the clients we can off of this tag.
for _, c in pairs(clients) do
@ -309,9 +309,11 @@ function tag.object.delete(self, fallback_tag, force)
tag.setproperty(tags[i], "index", i-1)
end
-- If no tags are visible, try and view one.
if target_scr.selected_tag == nil and ntags > 0 then
tag.history.restore(nil, 1)
-- If no tags are visible (and we did not delete the lasttag), try and
-- view one. The > 1 is because ntags is no longer synchronized with the
-- current count.
if target_scr.selected_tag == nil and ntags > 1 then
tag.history.restore(target_scr, 1)
if target_scr.selected_tag == nil then
local other_tag = tags[tags[1] == self and 2 or 1]
if other_tag then
@ -465,6 +467,7 @@ function tag.object.set_screen(t, s)
-- Change the screen
tag.setproperty(t, "screen", s)
tag.setproperty(t, "index", #s:get_tags(true))
-- Make sure the client's screen matches its tags
for _,c in ipairs(t:clients()) do
@ -473,10 +476,8 @@ function tag.object.set_screen(t, s)
end
-- Update all indexes
for _,screen in ipairs {old_screen, s} do
for i,t2 in ipairs(screen.tags) do
tag.setproperty(t2, "index", i)
end
for i,t2 in ipairs(old_screen.tags) do
tag.setproperty(t2, "index", i)
end
-- Restore the old screen history if the tag was selected
@ -1277,21 +1278,25 @@ function tag.attached_connect_signal(screen, ...)
end
-- Register standard signals.
capi.client.connect_signal("manage", function(c)
-- If we are not managing this application at startup,
-- move it to the screen where the mouse is.
-- We only do it for "normal" windows (i.e. no dock, etc).
if not awesome.startup and c.type ~= "desktop" and c.type ~= "dock" then
if c.transient_for then
c.screen = c.transient_for.screen
if not c.sticky then
c:tags(c.transient_for:tags())
capi.client.connect_signal("property::screen", function(c)
-- First, the delayed timer is necessary to avoid a race condition with
-- awful.rules. It is also messing up the tags before the user have a chance
-- to set them manually.
timer.delayed_call(function()
local tags, new_tags = c:tags(), {}
for _, t in ipairs(tags) do
if t.screen == c.screen then
table.insert(new_tags, t)
end
else
c.screen = ascreen.focused()
end
end
c:connect_signal("property::screen", function() c:to_selected_tags() end)
if #new_tags == 0 then
c:emit_signal("request::tag", nil, {reason="screen"})
elseif #new_tags < #tags then
c:tags(new_tags)
end
end)
end)
-- Keep track of the number of urgent clients.

View File

@ -59,6 +59,8 @@ local function get_widget_context(self)
end
local function do_redraw(self)
if type(self.drawable) ~= "drawable" then return end --FIXME See #1070
local surf = surface.load_silently(self.drawable.surface, false)
-- The surface can be nil if the drawable's parent was already finalized
if not surf then return end

View File

@ -2304,6 +2304,10 @@ luaA_client_swap(lua_State *L)
*
* Use the `first_tag` field to access the first tag of a client directly.
*
* **Signal:**
*
* * *property::tags*
*
* @tparam table tags_table A table with tags to set, or `nil` to get the
* current tags.
* @treturn table A table with all tags.
@ -2342,7 +2346,10 @@ luaA_client_tags(lua_State *L)
lua_pushnil(L);
while(lua_next(L, 2))
tag_client(L, c);
lua_pop(L, 1);
luaA_object_emit_signal(L, -1, "property::tags", 0);
}
lua_newtable(L);

512
tests/_multi_screen.lua Normal file
View File

@ -0,0 +1,512 @@
-- Helper module to replicate a set of steps across multiple screen scenarios
-- Ideas for improvements:
-- * Export an iterator so the suits can call the screen change step by step
-- themselves instead of the current all or nothing
-- * Offer multiple scenarios such as screen add, remove, resize/swap instead
-- of having to test all corner cases all the time.
local wibox = require("wibox")
local surface = require("gears.surface")
local wallpaper = require("gears.wallpaper")
local color = require("gears.color")
local shape = require("gears.shape")
local module = {}
-- Get the original canvas size before it is "lost"
local canvas_w, canvas_h = root.size()
local half_w, half_h = math.floor(canvas_w/2), math.floor(canvas_h/2)
local quarter_w = math.floor(canvas_w/4)
-- Create virtual screens.
-- The steps will be executed on each set of screens.
local dispositions = {
-- A simple classic, 2 identical screens side by side
{
function()
return {
x = 0,
y = 0,
width = half_w,
height = canvas_h,
}
end,
function()
return {
x = half_w,
y = 0,
width = half_w,
height = canvas_h,
}
end,
},
-- Take the previous disposition and swap the screens
{
function()
return {
x = half_w,
y = 0,
width = half_w,
height = canvas_h,
}
end,
function()
return {
x = 0,
y = 0,
width = half_w,
height = canvas_h,
}
end,
keep = true,
},
-- Take the previous disposition and resize the leftmost one
{
function()
return {
x = quarter_w,
y = 0,
width = 3*quarter_w,
height = canvas_h,
}
end,
function()
return {
x = 0,
y = 0,
width = quarter_w,
height = half_h,
}
end,
keep = true,
},
-- Take the previous disposition and remove the leftmost screen
{
function()
return {
x = quarter_w,
y = 0,
width = 3*quarter_w,
height = canvas_h,
}
end,
function()
return nil
end,
keep = true,
},
-- Less used, but still quite common vertical setup
{
function()
return {
x = 0,
y = 0,
width = canvas_w,
height = half_h,
}
end,
function()
return {
x = 0,
y = half_h,
width = canvas_w,
height = half_h,
}
end
},
-- Another popular setup, 22" screens with a better 31" in the middle.
-- So, 2 smaller vertical screen with a larger central horizontal one.
{
-- Left
function()
return {
x = 0,
y = 0,
width = quarter_w,
height = canvas_h,
}
end,
-- Right, this test non-continous screen index on purpose, as this setup
-- Often requires dual-GPU, it _will_ happen.
function()
return {
x = canvas_w-quarter_w,
y = 0,
width = quarter_w,
height = canvas_h,
}
end,
-- Center
function()
return {
x = quarter_w,
y = 0,
width = half_w,
height = math.floor(canvas_h*(9/16)),
}
end,
},
-- Same as the previous one, but with the gap centered
{
-- Left
function()
return {
x = 0,
y = 0,
width = quarter_w,
height = canvas_h,
}
end,
-- Right, this test non-continous screen index on purpose, as this setup
-- Often requires dual-GPU, it _will_ happen.
function()
return {
x = canvas_w-quarter_w,
y = 0,
width = quarter_w,
height = canvas_h,
}
end,
-- Center
function()
return {
x = quarter_w,
y = math.ceil((canvas_h-(canvas_h*(9/16)))/2),
width = half_w,
height = math.floor(canvas_h*(9/16)),
}
end,
-- Keep the same screens as the last test, just move them
keep = true,
},
-- AMD Eyefinity / (old) NVIDIA MOSAIC style config (symmetric grid)
{
function()
return {
x = 0,
y = 0,
width = half_w,
height = half_h,
}
end,
function()
return {
x = half_w,
y = 0,
width = half_w,
height = half_h,
}
end,
function()
return {
x = 0,
y = half_h,
width = half_w,
height = half_h,
}
end,
function()
return {
x = half_w,
y = half_h,
width = half_w,
height = half_h,
}
end,
},
-- Corner case 1: Non-continuous screens
-- If there is nothing and client is drag&dropped into that area, some
-- geometry callbacks may break in nil index.
{
function()
return {
x = 0,
y = 0,
width = quarter_w,
height = canvas_h,
}
end,
function()
return {
x = canvas_w-quarter_w,
y = 0,
width = quarter_w,
height = canvas_h,
}
end,
},
-- Corner case 2: Nothing at 0x0.
-- As some position may fallback to 0x0 this need to be tested often. It
-- also caused issues such as #154
{
function()
return {
x = 0,
y = half_w,
width = half_w,
height = half_w,
}
end,
function()
return {
x = half_w,
y = 0,
width = half_w,
height = canvas_h,
}
end
},
-- Corner case 3: Many very small screens.
-- On the embedded side of the compuverse, it is possible
-- to buy 32x32 RGB OLED screens. They are usually used to display single
-- status icons, but why not use them as a desktop! This is a critical
-- market for AwesomeWM. Here's a 256px wide strip of tiny screens.
-- This may cause wibars to move to the wrong screen accidentally
{
function() return { x = 0 , y = 0, width = 32, height = 32, } end,
function() return { x = 32 , y = 0, width = 32, height = 32, } end,
function() return { x = 64 , y = 0, width = 32, height = 32, } end,
function() return { x = 96 , y = 0, width = 32, height = 32, } end,
function() return { x = 128, y = 0, width = 32, height = 32, } end,
function() return { x = 160, y = 0, width = 32, height = 32, } end,
function() return { x = 192, y = 0, width = 32, height = 32, } end,
function() return { x = 224, y = 0, width = 32, height = 32, } end,
},
-- Corner case 4: A screen taller than more than 1 other screen.
-- this may cause some issues with client resize and drag&drop move
{
function()
return {
x = 0,
y = 0,
width = half_w,
height = canvas_h,
}
end,
function()
return {
x = half_w,
y = 0,
width = half_w,
height = math.floor(canvas_h/3),
}
end,
function()
return {
x = half_w,
y = math.floor(canvas_h/3),
width = half_w,
height = math.floor(canvas_h/3),
}
end,
function()
return {
x = half_w,
y = 2*math.floor(canvas_h/3),
width = half_w,
height = math.floor(canvas_h/3),
}
end
},
-- The overlapping corner case isn't supported. There is valid use case,
-- such as a projector with a smaller resolution than the laptop screen
-- in non-scaling mirror mode, but it isn't worth the complexity it brings.
}
local function check_tag_indexes()
for s in screen do
for i, t in ipairs(s.tags) do
assert(t.index == i)
assert(t.screen == s)
end
end
end
local colors = {
"#000030",
"#300000",
"#043000",
"#302E00",
"#002C30",
"#300030",
"#301C00",
"#140030",
}
-- Paint it black
local function clear_screen()
for s in screen do
local sur = surface.widget_to_surface(
wibox.widget {
bg = "#000000",
widget = wibox.container.background
},
s.geometry.width,
s.geometry.height
)
wallpaper.fit(sur, s, "#000000")
end
end
-- Make it easier to debug the tests by showing the screen geometry when the
-- tests are executed.
local function show_screens()
wallpaper.set(color("#000000")) -- Should this clear the wallpaper? It doesn't
-- Add a wallpaper on each screen
for i=1, screen.count() do
local s = screen[i]
local w = wibox.widget {
{
text = table.concat{
"Screen: ",i,"\n",
s.geometry.width,"x",s.geometry.height,
"+",s.geometry.x,",",s.geometry.y
},
valign = "center",
align = "center",
widget = wibox.widget.textbox,
},
bg = colors[i],
fg = "#ffffff",
shape_border_color = "#ff0000",
shape_border_width = 1,
shape = shape.rectangle,
widget = wibox.container.background
}
local sur = surface.widget_to_surface(
w,
s.geometry.width,
s.geometry.height
)
wallpaper.fit(sur, s)
end
end
local function add_steps(real_steps, new_steps)
for _, dispo in ipairs(dispositions) do
-- Cleanup
table.insert(real_steps, function()
if #client.get() == 0 then return true end
for _, c in ipairs(client.get()) do
c:kill()
end
end)
table.insert(real_steps, function()
clear_screen()
local keep = dispo.keep or false
local old = {}
local geos = {}
check_tag_indexes()
if keep then
for _, sf in ipairs(dispo) do
local geo = sf and sf() or nil
-- If the function return nothing, assume the screen need to
-- be destroyed.
table.insert(geos, geo or false)
end
-- Removed screens need to be explicit.
assert(#geos >= screen.count())
-- Keep a cache to avoid working with invalid data
local old_screens = {}
for s in screen do
table.insert(old_screens, s)
end
for i, s in ipairs(old_screens) do
-- Remove the screen (if no new geometry is given
if not geos[i] then
s:fake_remove()
else
local cur_geo = s.geometry
for _, v in ipairs {"x", "y", "width", "height" } do
cur_geo[v] = geos[i][v] or cur_geo[v]
end
s:fake_resize(
cur_geo.x,
cur_geo.y,
cur_geo.width,
cur_geo.height
)
end
end
-- Add the remaining screens
for i=#old_screens + 1, #geos do
local geo = geos[i]
screen.fake_add(geo.x, geo.y, geo.width, geo.height)
end
check_tag_indexes()
else
-- Move all existing screens out of the way (to avoid temporary overlapping)
for s in screen do
s:fake_resize(canvas_w, canvas_h, 1, 1)
table.insert(old, s)
end
-- Add the new screens
for _, sf in ipairs(dispo) do
local geo = sf()
screen.fake_add(geo.x, geo.y, geo.width, geo.height)
table.insert(geos, geo)
end
-- Remove old screens
for _, s in ipairs(old) do
s:fake_remove()
end
end
show_screens()
-- Check the result is correct
local expected_count = 0
for _,v in ipairs(geos) do
expected_count = expected_count + (v and 1 or 0)
end
assert(expected_count == screen.count())
for k, geo in ipairs(geos) do
if geo then
local sgeo = screen[k].geometry
assert(geo.x == sgeo.x)
assert(geo.y == sgeo.y)
assert(geo.width == sgeo.width )
assert(geo.height == sgeo.height)
end
end
return true
end)
for _, step in ipairs(new_steps) do
table.insert(real_steps, step)
end
end
end
return setmetatable(module, {
__call = function(_,...) return add_steps(...) end
})

View File

@ -194,7 +194,7 @@ start_awesome() {
# Count errors.
errors=0
# Seconds after when awesome gets killed.
timeout_stale=60
timeout_stale=120 # FIXME This should be no more than 60s
for f in $tests; do
echo "== Running $f =="

View File

@ -1,4 +1,5 @@
local awful = require("awful")
local test_client = require("_client")
awful.util.deprecate = function() end
@ -64,9 +65,6 @@ assert(c:tags()[1] == t2 or c:tags()[2] == t2)
awful.client.toggletag(t2, c)
assert(c:tags()[1] == t and c:tags()[1] ~= t2 and c:tags()[2] == nil)
-- Test movetoscreen
--FIXME I don't have the hardware to test
-- Test floating
assert(c.floating ~= nil and type(c.floating) == "boolean")
c.floating = true
@ -79,4 +77,87 @@ end
end
}
local has_error
table.insert(steps, function()
-- Make sure there is no extra callbacks that causes double screen changes
-- (regress #1028 #1052)
client.connect_signal("property::screen",function(c)
if c.class and c.class ~= "screen"..c.screen.index then
has_error = table.concat {
"An unexpected screen change did occur\n",
debug.traceback(),"\n",
c.class, " Screen: ",c.screen.index
}
end
end)
return true
end)
local multi_screen_steps = {}
-- Add a test client on each screen.
table.insert(multi_screen_steps, function()
has_error = false
for i=1, screen.count() do
local s = screen[i]
test_client("screen"..i, nil, {
screen = s,
})
end
return true
end)
-- Test if the clients have been placed on the right screen
table.insert(multi_screen_steps, function()
if #client.get() ~= screen.count() then return end
if has_error then print(has_error) end
assert(not has_error)
for i=1, screen.count() do
local s = screen[i]
-- Just in case
for _, t in ipairs(s.selected_tags) do
assert(t.screen == s)
end
local t = s.selected_tag
assert(t and t.screen == s)
-- In case the next step has failed, find what went wrong
for _, c in ipairs(t:clients()) do
-- This is _supposed_ to be impossible, but can be forced by buggy
-- lua code.
if c.screen ~= t.screen then
local tags = c:tags()
for _, t2 in ipairs(tags) do
assert(t2.screen == c.screen)
end
assert(false)
end
assert(c.screen.index == i)
end
-- Check that the right client is placed, the loop above will trigger
-- asserts already, but in case the tag has more than 1 client and
-- everything is messed up, those asserts are still necessary.
assert(#t:clients() == 1)
assert(t:clients()[1].class == "screen"..i)
assert(t:clients()[1].screen == s)
-- Make sure it stays where it belongs
awful.placement.maximize(t:clients()[1])
assert(t:clients()[1] and t:clients()[1].screen == t.screen)
end
return true
end)
require("_multi_screen")(steps, multi_screen_steps)
require("_runner").run_steps(steps)

View File

@ -11,6 +11,13 @@ local tests = {}
local tb_height = awful.util.round(beautiful.get_font_height() * 1.5)
-- local border_width = beautiful.border_width
-- Detect "manage" race conditions
local real_apply = awful.rules.apply
function awful.rules.apply(c)
assert(#c:tags() == 0)
return real_apply(c)
end
local function test_rule(rule)
rule.rule = rule.rule or {}

View File

@ -128,4 +128,117 @@ end
end
}
local multi_screen_steps = {}
-- Clear the state (this step will be called once per disposition)
table.insert(multi_screen_steps, function()
if #client.get() == 0 then
-- Remove all existing tags created by the rc.lua callback
for s in screen do
while #s.tags > 0 do
local t = s.tags[1]
-- Test #1051 against regressions
assert(t.index == 1)
assert(t.screen == s)
t:delete()
assert(not t.activated)
assert(t.screen == nil)
end
end
return true
end
for _, c in ipairs(client.get()) do
c:kill()
end
end)
-- Add a bunch of tags on each screens.
table.insert(multi_screen_steps, function()
for i=1, screen.count() do
local s = screen[i]
-- Add 5 tags per screen
for idx=1, 5 do
awful.tag.add("S"..i..":"..idx,{screen = s})
end
end
-- Check if everything adds up
for i=1, screen.count() do
local tags = screen[i].tags
assert(#tags == 5)
for k, t in ipairs(tags) do
assert(t.index == k)
assert(t.name == "S"..i..":"..k)
end
end
-- Select a tag on each screen
for i=1, screen.count() do
screen[i].tags[1].selected = true
end
return true
end)
local function check_tag_indexes()
for s in screen do
for i, t in ipairs(s.tags) do
assert(t.index == i)
assert(t.screen == s)
end
end
end
-- Move tags across screens (without clients)
table.insert(multi_screen_steps, function()
-- This screen disposition cannot be used for this test, move on
if screen.count() < 2 then return true end
local s1, s2 = screen[1], screen[2]
local old_count1, old_count2 = #s1.tags, #s2.tags
local t = awful.tag.add("TEST", {screen=s1})
-- Check the current state
assert( t.index == old_count1+1 )
assert( t.screen == s1 )
assert( #s1.tags == old_count1+1 )
assert( s1.tags[old_count1+1] == t )
assert( #s2.tags == old_count2 )
-- Move to another index
local new_index = 3
t.index = new_index
assert(t.index == new_index)
check_tag_indexes()
-- Move the tag to screen 2
t.screen = s2
assert(t.screen == s2 )
assert(#s1.tags == old_count1 )
assert(#s2.tags == old_count2+1 )
assert( s2.tags[old_count2+1] == t )
assert( t.index == old_count2+1 )
check_tag_indexes()
-- Move to another index
t.index = new_index
assert(t.index == new_index)
check_tag_indexes()
-- Delete it
t:delete()
check_tag_indexes()
return true
end)
require("_multi_screen")(steps, multi_screen_steps)
require("_runner").run_steps(steps)