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.

Sign up for GitHub

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

Add output selection #60

Closed
wants to merge 1 commit into from
Closed

Add output selection #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions include/slurp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,22 @@ struct slurp_state {
uint32_t border_weight;
bool display_dimensions;
bool single_point;
bool output_selection;
struct wl_list boxes; // slurp_box::link

struct slurp_box result;

struct {
struct slurp_box selection;
struct slurp_output *output;
} result;
};

struct slurp_output {
struct wl_output *wl_output;
struct slurp_state *state;
struct wl_list link; // slurp_state::outputs

char* name;

struct slurp_box geometry;
struct slurp_box logical_geometry;
int32_t scale;
Expand Down
83 changes: 63 additions & 20 deletions main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#define _POSIX_C_SOURCE 2
#define _POSIX_C_SOURCE 200809L

#include <errno.h>
#include <stdio.h>
Expand Down Expand Up @@ -169,12 +169,26 @@ static void handle_selection_start(struct slurp_seat *seat,
struct slurp_state *state = seat->state;
current_selection->has_selection = true;

if (state->single_point) {
state->result.x = current_selection->x;
state->result.y = current_selection->y;
state->result.width = state->result.height = 1;
if (state->single_point || state->output_selection) {

struct slurp_output *output;
wl_list_for_each(output, &state->outputs, link) {
if(in_box(&output->logical_geometry, current_selection->x, current_selection->y)) {
break;
}
}
state->result.output = output;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to set this for a rectangular selection as well? You do run into the problem of what to do if there are multiple outputs, but the simplest think would be just to pick a point, like the top left, or center of the rectangle, and use that to pick the "best" output.

Copy link

Choose a reason for hiding this comment

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

Slurp can take a predefined set of rectangles as input and limit possible selections to those rectangles (you can see this if you use the output selection example in the README).

I think the correct approach is to implement it this way, where there's no option to select anything other than the predefined rectangles which occupy each output entirely, and chosen by the rectangle the mouse is inside of.

Copy link

Choose a reason for hiding this comment

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

Looks like you can just use add_choice_box like is done here https://github.com/emersion/slurp/blob/master/main.c#L761

Copy link
Collaborator

Choose a reason for hiding this comment

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

limit possible selections to those rectangles

That isn't quite right. At least from me trying it out in slurp 1.2.0, it seems it just creates predefined boxes you can select with a click, but it doesn't stop you from dragging your own box.

Copy link

Choose a reason for hiding this comment

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

Hmmm, that sounds like a bug in the intended behavior, but @emersion would have to weigh in.

Copy link
Owner

Choose a reason for hiding this comment

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

No, this is intentional: the input boxes are just hints, the user can ignore the hint. I'd be fine with adding an option to prevent that.

Copy link

Choose a reason for hiding this comment

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

Glad I asked.


if(state->output_selection) {
state->result.selection = output->logical_geometry;
} else {
state->result.selection.x = current_selection->x;
state->result.selection.y = current_selection->y;
state->result.selection.width = state->result.selection.height = 1;
}

state->running = false;
} else {
} else {
current_selection->anchor_x = current_selection->x;
current_selection->anchor_y = current_selection->y;
}
Expand All @@ -183,11 +197,11 @@ static void handle_selection_start(struct slurp_seat *seat,
static void handle_selection_end(struct slurp_seat *seat,
struct slurp_selection *current_selection) {
struct slurp_state *state = seat->state;
if (state->single_point) {
if (state->single_point || state->output_selection) {
return;
}
if (current_selection->has_selection) {
state->result = current_selection->selection;
state->result.selection = current_selection->selection;
}
state->running = false;
}
Expand Down Expand Up @@ -450,11 +464,17 @@ static void xdg_output_handle_logical_size(void *data,
output->logical_geometry.height = height;
}

static void xdg_output_handle_name(void *data,
struct zxdg_output_v1 *xdg_output, const char *name) {
struct slurp_output *output = data;
output->name = strdup(name);
}

static const struct zxdg_output_v1_listener xdg_output_listener = {
.logical_position = xdg_output_handle_logical_position,
.logical_size = xdg_output_handle_logical_size,
.done = noop,
.name = noop,
.name = xdg_output_handle_name,
.description = noop,
};

Expand All @@ -468,6 +488,8 @@ static void create_output(struct slurp_state *state,
output->wl_output = wl_output;
output->state = state;
output->scale = 1;
output->name = strdup("<unknown>"); // just to simplify the logic when destroying outputs

wl_list_insert(&state->outputs, &output->link);

wl_output_add_listener(wl_output, &output_listener, output);
Expand Down Expand Up @@ -613,7 +635,7 @@ static void handle_global(void *data, struct wl_registry *registry,
create_output(state, wl_output);
} else if (strcmp(interface, zxdg_output_manager_v1_interface.name) == 0) {
state->xdg_output_manager = wl_registry_bind(registry, name,
&zxdg_output_manager_v1_interface, 1);
&zxdg_output_manager_v1_interface, 2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may break if the compositor only supports version 1. If that is a concern, I think MIN(2, version) would be better.

Copy link
Contributor Author

@vesim987 vesim987 Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think the -o feature w/o outputs names is kinda useless. But that is a question to @emersion, if we want to keep compatibility with version 1.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it's fine to bump the minimum required version to 2.

}
}

Expand All @@ -632,7 +654,8 @@ static const char usage[] =
" -s #rrggbbaa Set selection color.\n"
" -w n Set border weight.\n"
" -f s Set output format.\n"
" -p Select a single point.\n";
" -p Select a single point.\n"
" -o Output selection.\n";

uint32_t parse_color(const char *color) {
if (color[0] == '#') {
Expand All @@ -652,7 +675,7 @@ uint32_t parse_color(const char *color) {
return res;
}

static void print_formatted_result(const struct slurp_box *result,
static void print_formatted_result(const struct slurp_state *state,
const char *format) {
for (size_t i = 0; format[i] != '\0'; i++) {
char c = format[i];
Expand All @@ -662,16 +685,23 @@ static void print_formatted_result(const struct slurp_box *result,
i++; // Skip the next character (x, y, w or h)
switch (next) {
case 'x':
printf("%u", result->x);
printf("%u", state->result.selection.x);
continue;
case 'y':
printf("%u", result->y);
printf("%u", state->result.selection.y);
continue;
case 'w':
printf("%u", result->width);
printf("%u", state->result.selection.width);
continue;
case 'h':
printf("%u", result->height);
printf("%u", state->result.selection.height);
continue;
case 'o':
if(state->result.output == NULL) {
printf("<unknown>");
} else {
printf("%s", state->result.output->name);
}
continue;
default:
// If no case was executed, revert i back - we don't need to
Expand Down Expand Up @@ -708,7 +738,7 @@ int main(int argc, char *argv[]) {

int opt;
char *format = "%x,%y %wx%h";
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the -o option is used, should a different default format be used that includes the output name (or maybe is only the output name)?

while ((opt = getopt(argc, argv, "hdb:c:s:w:pf:")) != -1) {
while ((opt = getopt(argc, argv, "hdob:c:s:w:pf:")) != -1) {
switch (opt) {
case 'h':
printf("%s", usage);
Expand All @@ -728,6 +758,9 @@ int main(int argc, char *argv[]) {
case 'f':
format = optarg;
break;
case 'o':
state.output_selection = true;
break;
case 'w': {
errno = 0;
char *endptr;
Expand All @@ -748,7 +781,7 @@ int main(int argc, char *argv[]) {
}

wl_list_init(&state.boxes);
if (!isatty(STDIN_FILENO) && !state.single_point) {
if (!isatty(STDIN_FILENO) && !(state.single_point || state.output_selection)) {
struct slurp_box in_box = {0};
while (fscanf(stdin, "%d,%d %dx%d\n", &in_box.x, &in_box.y,
&in_box.width, &in_box.height) == 4) {
Expand Down Expand Up @@ -861,6 +894,13 @@ int main(int argc, char *argv[]) {
// second roundtrip for xdg-output
wl_display_roundtrip(state.display);

if(state.output_selection) {
struct slurp_output *box_output;
wl_list_for_each(box_output, &state.outputs, link) {
add_choice_box(&state, &box_output->logical_geometry);
}
}

struct slurp_seat *seat;
wl_list_for_each(seat, &state.seats, link) {
seat->cursor_surface =
Expand All @@ -872,8 +912,11 @@ int main(int argc, char *argv[]) {
// This space intentionally left blank
}

print_formatted_result(&state, format);

struct slurp_output *output_tmp;
wl_list_for_each_safe(output, output_tmp, &state.outputs, link) {
free(output->name);
destroy_output(output);
}
struct slurp_seat *seat_tmp;
Expand All @@ -900,11 +943,11 @@ int main(int argc, char *argv[]) {
free(box);
}

if (state.result.width == 0 && state.result.height == 0) {
if (state.result.selection.width == 0 && state.result.selection.height == 0) {
fprintf(stderr, "selection cancelled\n");
return EXIT_FAILURE;
}

print_formatted_result(&state.result, format);

return EXIT_SUCCESS;
}