Implement subsurfaces

[ammen99]

Should resolve #22

This is my work on subsurfaces inside WLC. I haven't tested them a lot(the only compositor tested is orbment), but gedit and nautlilus work properly now(and they use subsurfaces). If there is anything which doesn't work, please tell me, I'll do my best to fix it.

What this pull request does is rendering the subsurfaces of a surface and also modify pointer focus functions to support focusing of subsurfaces.

Things that still need to be done(aka TODO list):

1. Implement proper handling of synchronized subsurfaces - DONE
2. Implement subsurface::place_above and subsurface::place_below - DONE, but untested

[Cloudef]

If you haven't seen it already https://github.com/Cloudef/wlc/blob/master/CONTRIBUTING.rst this file goes over the general code styling quite well as FYI. But this code overall looks good.

[crondog]

I just tested this with weston-subsurfaces and the pointer disappears when going inside the subsurfaces (the 2 small squares)

[ammen99]

Yes, you're right. I'll investigate that.

[ammen99]

The last commit should resolve the disappearing cursor. Can you confirm it?

[crondog]

The pointer is now visible. However the subsurfaces don't appear until I put the pointer inside the application.

[Cloudef]

The pointer is now visible. However the subsurfaces don't appear until I put the pointer inside the application.

The code probably needs wlc_output_schedule_repaint when surface is added. It might be better to do this in wlc_surface_add_child_surface(parent, child) where output from parent is copied to child and that output is also scheduled.

[ammen99]

Possible reason for this bug is that weston-subsurfaces switches between synchronized and desynchronized mode, which is still to be implemented(but I don't know for sure).

[Cloudef]

This is starting to look quite good. I'm pretty sure the subsurfaces not appearing is due to the render loop not doing a cycle when it should (e.g. they only appear when application triggers new render cycle (moving pointer inside)). Though I may be wrong too.

Anyhow, I should give it spin too later.

[ammen99]

My latest commit is from the account rilix04, this is because my sister was on the pc yesterday, her account was still logged in.

[ammen99]

Well, for the weston-subsurfaces bug: for me(I'm using orbment) I get different bug - the subsurfaces aren't placed correctly(their positions don't get commited) until I either resize the window(when nested compositor) or I switch the focus. I mean, when weston-subsurfaces start, it has some geometry. Orbment changes it to half the screen(I have a terminal opened before), the main surface gets changed, but the subsurfaces remain on their old positions. @Cloudef do you have idea what happens during these actions that causes weston-subsurfaces to commit the surfaces? This might show what's wrong.

[ammen99]

I forgot to mention that this bug has nothing to do with synchronized or desynchronized subsurfaces. I tried directly committing position, which shows synchronization is irrelevant for this bug.

[Cloudef]

What does the spec say about this. If the parent surface gets commit would the subsurfaces need to be commited as well by compositor?

[ammen99]

According to the spec, if subsurface is synchronized, then when parent is commited subsurface is commited also. Maybe I should try to implement synchronization when I have more time and see what happens then.

[ammen99]

I implemented subsurface synchronization. This fixes the problem with weston-subsurfaces - at least on my computer.

[crondog]

Nice work. Weston-subsurfaces appears to be working correctly now. Don't forget you still need to clean up the stray printf and the if statements

[ammen99]

I implemented subsurface_place_above/below requests, but do you know any program which uses these? I just have no way to test them.

[Cloudef]

@ammen99 Not even the weston test programs? Could always write one yourself I guess. I think we could start merging this PR though, so we get later fixes / additions in smaller chunks.

[ammen99]

I did your suggestions. In weston's tests there is one with such request - but it doesn't run(it shows a lot of errors), as do all other tests.

[ammen99]

Is there something that still needs to be done or I should squash my commits into one to merge?

[Cloudef]

Squash into one, and I will do final checkups that there is nothing obvious and merge it then.

[Cloudef]

The changes in wlc_pointer_focus for transforming mouse coordinates (in case view geometry != client geometry) cause slight breakage. Pointer coordinates are wrong in programs. E.g. Launch weston-resizor, and try resize the window from bottom handle, you can't do it, but you can do it from bit more above.

I'm guessing from reading code, that this happens because the transformation does not handle correctly main surface (binded to view). Main surface coordinates are relative, and starts from 0,0. The calculation was changed in favor of subsurfaces which have different rules (?).

[ammen99]

I fixed the bug. It turned out that the problem were maximized views, which now should be handled correctly. Please write if there are any other known bugs.

[ammen99]

I didn't quite get what you mean. Moving the code to wlc_view_commit state definitely breaks pointer. The thing that I couldn't figure out before was why in configure_view() the parameter g is different from what I get when I call wlc_view_get_bounds()?

[ammen99]

I hope this finally fixes the problem. It was simply because coordinate_scale must be updated not only when view is commited, but when surface->size changes(when commiting a buffer).

[ammen99]

Maybe we should add a new function for surfaces. something like wlc_surface_update_coordinate_scale()?

[Cloudef]

Yeah, that would be good idea. I think this should work inside view_commit too though, if not. There may be some bigger problem somewhere. I'll look into it too later.

[Cloudef]

I'll look into why view_commit doesn't work for this now.

[Cloudef]

I sent you a bit ugly PR, as I by instict updated it against latest wlc as well. That should make the coordinate transform updates bit saner. I think it's okay for merge after this.

I'm wondering if the subsurfaces should be tight fitted with the main view when wlc forces it to requested geometry. It may be something to do in future, but for now that's not important. (It might rely on moving the wlc_view_get_bounds functionality to surface.c instead)

[ammen99]

With these changes the coordinate scale doesn't get updated properly if I don't move the window. When this happens though everything is OK. So we must either use surface_update_coordinate_transform or think of some other way to do this.

[Cloudef]

It's probably because of the surface being created after view commit. And the precondition in view commit causes the function to be no-op if there was no changes to view state. If this is the case wlc_surface_set_parent should set the initial transform to new child surface.

[ammen99]

I'm talking about the whole programs (try gedit for example). Maybe this should be done when surface size changes.

[ammen99]

@Cloudef Do you have idea if there is already some function which gets triggered when surface size changes or some other related event?

[Cloudef]

The view_commit_state is the function that applies new view size. For surfaces (not related to views) it's surface_commit (in surface.c).

[ammen99]

So, after all, we must update coordinate_scale in each surface_commit, because we can't know when surface size changes. I will re-add the function as I see no other way. Tell me if I'm wrong.

[ammen99]

Is there something that still needs to be done or I should squash for merging?

[Cloudef]

Squash, and I'll merge it.

[Cloudef]

That did not go right I guess :P

[ammen99]

Nope, it didn't. Sadly I cannot find a way to remove the merge using a rebase(it expands the commits)? Do you have idea why is this?

EDIT: fixed with some git branch -m magic

[Cloudef]

Yeah, you should not ever need merge in PR's, just rebase on top of upstream should do it.

[Cloudef]

Can you edit the commit message to be something more descriptive about this feature.

[ammen99]

Something like that?