Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set panels/backgrounds' geometries correctly and don't render them ex… #743

Merged
merged 8 commits into from
Jul 14, 2016

Conversation

deklov
Copy link
Contributor

@deklov deklov commented Jul 6, 2016

…plicitly

Panels and backgrounds were explicitly rendered by calling wlc_surface_render
in handle_output_pre_render. Calling wlc_surface_render does not set the
surface's geometry (like wlc_view_set_geometry does). Sway did not call
wlc_view_set_geometry for panels/backgrounds, so wlc defaulted their geometry
to be at the origin. This is not correct for bars unless their location is top.

Furthermore, for a surface to receive pointer events, its mask has to be
set to visible. This causes wlc to render these surfaces, causing panels
and backgrounds to be rendered twice.

This commit makes panels and surfaces visible, sets the correct geometries
and removes the code that explicitly rendered them.

@deklov
Copy link
Contributor Author

deklov commented Jul 6, 2016

Mouse pointer looks like the X default and mouse events are received by the bar.

@ddevault
Copy link
Contributor

ddevault commented Jul 6, 2016

How well does this work with:

  • Output resizing (either via swaymsg output ... res ... or the x backend resizing the windows)
  • Z ordering when the layout is arranged, the bg should always be at the bottom and the bar should be on top of all tiled windows but below floating and fullscreened windows

@ddevault
Copy link
Contributor

ddevault commented Jul 9, 2016

Bump

@deklov
Copy link
Contributor Author

deklov commented Jul 9, 2016

Not very well unfortunately. I got so excited when the bar started getting mouse events that I forgot to test re-sizing and swaybg. I'll try to work on this some more maybe tonight or tomorrow.

@ddevault
Copy link
Contributor

ddevault commented Jul 9, 2016

No worries, take your time.

@deklov
Copy link
Contributor Author

deklov commented Jul 10, 2016

Think I got this working but before you merge to master could you please test it.

I wanted to focus on the bar first so I reverted some of my code related to background. I will come back to this later but just in case I don't, I don't want to leave half-done, non-working behind.

@deklov
Copy link
Contributor Author

deklov commented Jul 10, 2016

I forgot to commit something..

@deklov
Copy link
Contributor Author

deklov commented Jul 10, 2016

btw. I have not tested this on dual head since I only have one monitor at home.

@deklov
Copy link
Contributor Author

deklov commented Jul 10, 2016

One more thing. I didn't to anything to try to set the z ordering. Fullscreen does work, but that might be coincidental. Do we need to set it explicitly?

@ddevault
Copy link
Contributor

Not sure how it'd shake out over time, restarts, etc.

@deklov
Copy link
Contributor Author

deklov commented Jul 10, 2016

[handlers.c:521] Command 'restart' failed: Unknown/invalid command

Get this before any of my changes as well.

@ddevault
Copy link
Contributor

Sorry, I meant reload. Which causes the swaybg and swaybar processes to restart.

@deklov
Copy link
Contributor Author

deklov commented Jul 10, 2016

Reload seems to work just fine.

@deklov
Copy link
Contributor Author

deklov commented Jul 11, 2016

This last commits require the previous commits to be tested and I was to lazy to create a branch with only these commits and too lazy to figure out how to create a PR with only these new commits. Hope you don't mind that I amended them to this PR.

@@ -7,6 +7,10 @@
#include "bar/config.h"
#include "bar/ipc.h"

void ipc_send_workspace_command(const char *workspace_name) {
sway_log(L_DEBUG, "Clicked on window %s", workspace_name);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log message should be: "Clicked on workspace button %s".

@deklov
Copy link
Contributor Author

deklov commented Jul 11, 2016

Last piece of the puzzle. You can now change workspace by clicking on the bar.

snprintf(command, size, "workspace %s", workspace_name);

uint32_t reply_size;
ipc_single_command(swaybar.ipc_socketfd, IPC_COMMAND, command, &reply_size);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, memory leak forgot to free command.

Copy link
Contributor Author

@deklov deklov Jul 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And another bug, the last argument to ipc_single_command is both input and output... Interesting semantics.

@deklov
Copy link
Contributor Author

deklov commented Jul 11, 2016

Fixed the the memory leak and misuse of ipc_single_command.

@ddevault
Copy link
Contributor

How about changing workspaces with the scroll wheel?

@deklov
Copy link
Contributor Author

deklov commented Jul 11, 2016

I look into that once this PR has been merged. Can you please open a ticket with all the mouse related features you want to see for swaybar and assign it to me.

@ddevault
Copy link
Contributor

Sure.

@ddevault
Copy link
Contributor

Tested this and it doesn't work with multihead - all of the swaybars end up spawning on the same output. You can simulate multihead by running sway with the x backend and using WLC_OUTPUTS=n where n is the number of windows to spawn as virtual outputs.

@@ -25,6 +23,8 @@ struct panel_config {
enum desktop_shell_panel_position panel_position;
// used to determine if client is a panel
struct wl_client *client;
// wlc handle for this panel's surface, not set until panel is created
wlc_handle handle;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tab issues

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@deklov
Copy link
Contributor Author

deklov commented Jul 11, 2016

We know what output each panel should be on: https://github.com/SirCmpwn/sway/blob/master/sway/extensions.c#L88. Could it be as simple as telling wlc what output to put the panels on?

@ddevault
Copy link
Contributor

Perhaps!

@deklov deklov closed this Jul 11, 2016
@ddevault
Copy link
Contributor

Can you just push more commits to this branch instead of making a fourth pull request?

@ddevault ddevault reopened this Jul 11, 2016
@deklov
Copy link
Contributor Author

deklov commented Jul 12, 2016

Yes, I didn't mean to close this PR, must have done so by mistake.

@deklov
Copy link
Contributor Author

deklov commented Jul 12, 2016

The dual head problem was easy to fix: git grep -n 'wlc.*output' in sway's source tree, found wlc_view_set_outpu and added a call to it in handle_view_created. Patch on the way. It will take longer to fix the spelling and tab errors ;)

@deklov
Copy link
Contributor Author

deklov commented Jul 14, 2016

Bump. Can we please try and get this merged.

@ddevault
Copy link
Contributor

Can you sync up with master?

This reverts commit 99bda4a.

It turned out that code to handle swaybg as shell surface was broken so we don't
want to make swaybg a shell surface until this has been fixed.
This code had some issues. Remove it now so that we can start clean and fix
it later.
Panels were explicitly rendered by calling wlc_surface_render in
handle_output_pre_render. Calling wlc_surface_render does not set the
surface's geometry (like wlc_view_set_geometry does). Sway did not call
wlc_view_set_geometry for panels, so wlc defaulted their geometry to be at
the origin. This is not correct for bars unless their location is top.

Furthermore, for a surface to receive pointer events, its mask has to be
set to visible. This causes wlc to render these surfaces, causing panels
and backgrounds to be rendered twice.

This commit makes panels and surfaces visible, sets the correct geometries
and removes the code that explicitly rendered them.
Also remove some unnecessary strtup()s and rename a few variables and functions.
This commit does not do anything with this information other than logging it.
@deklov
Copy link
Contributor Author

deklov commented Jul 14, 2016

Done

@ddevault ddevault merged commit 6abbe04 into swaywm:master Jul 14, 2016
@ddevault
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants