[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/3] Include all clients in _NET_CLIENT_LIST_STACKING



Hi,

On Mon, Nov 19, 2012 at 03:01:12PM +0100, Hans-Peter Deifel wrote:
> On Mo, Nov 19 2012, Thorsten Wißmann wrote:
> > On Sun, Nov 18, 2012 at 07:21:54PM +0100, Hans-Peter Deifel wrote:
> > > Also include clients that are not in the current stack. Now,
> > > _NET_CLIENT_LIST_STACKING should contain exactly the same windows as
> > > _NET_CLIENT_LIST.
> >
> > > +static Window* ewmh_merge_stacking_list(Window *buf, int count, int *new_count) {
> > > +    int last = count;
> > > +
> > > +    buf = g_renew(Window, buf, count + g_window_count);
> >
> > If ..LIST_STACKING contains exactly the same windows as ..LIST, then why
> > don't you realloc the buf size directly to g_window_count? (Of course
> > you additionally should check how many windows are finally added to
> > the buf).
> 
> That was a bit of defensive programming on my part. That way, it even
> works if the input isn't as expected (for example duplicate windows in
> buf).
>
> But yeah, you could also just check if you're overflowing the buffer.

I implemented it by creating a buf of size g_window_count. Then I use
stack_to_window_buf() which does the buf size checking internally.

> > > +    for (int i = 0; i < g_window_count; i++) {
> > > +        if (array_find(buf, count, sizeof(Window), g_windows + i) < 0) {
> > > +            buf[last++] = g_windows[i];
> > > +        }
> > > +    }
> >
> > That's OK for now. If there are n clients, that needs O(n²) run-time,
> > but a O(n*m) way (with m monitors) isn't difficult (and not long) to
> > implement, because you know which tags are visible on monitors and which
> > aren't (You can check it with find_monitor_with_tag() in O(m)). I'll fix
> > that in near (or far) future.
> 
> Sure. The constant factor is small and the numbers likely too, so I
> didn't bother to find out how to look up tags in the current stack.

You can look at the code now; it uses some hlwm internal functions. For
n < 50 no difference should be noticed, but imo it is cleaner.

By the way: array_find() probably can be replaced by the POSIX function
lsearch(3), which just is a bit slower (because of the compare function
pointer). I added the array_find() when I didn't know about lsearch (and
bsearch and...).

Regards,
Thorsten