Skip to content

Make screen option of icon boxes work again #1230

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

Draft
bitstreamout wants to merge 1 commit into
base: main
Choose a base branch
from
+17 −9
Draft

Make screen option of icon boxes work again #1230

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
20 changes: 13 additions & 7 deletions fvwm/icons.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1786,18 +1786,22 @@ void AutoPlaceIcon(
icon_boxes_ptr = NULL; /* init */
while(do_all_iconboxes(t, &icon_boxes_ptr))
{
struct monitor *m;
if (loc_ok == True)
{
/* leave for loop */
break;
}
/* get the screen dimensions for the icon box */
FScreenGetScrRect(fscr, FSCREEN_CURRENT,
&ref.x, &ref.y, &ref.width, &ref.height);
dim[1].screen_offset = ref.y;
dim[1].screen_dimension = ref.height;
dim[2].screen_offset = ref.x;
dim[2].screen_dimension = ref.width;
m = monitor_resolve_name(icon_boxes_ptr->IconScreen);
if (m == NULL)
{
m = monitor_get_current();
}
dim[1].screen_offset = ref.y = m->si->y;
dim[1].screen_dimension = ref.height = m->si->h;
dim[2].screen_offset = ref.x = m->si->x;
dim[2].screen_dimension = ref.width = m->si->w;
/* y amount */
dim[1].step = icon_boxes_ptr->IconGrid[1];
/* init start from */
Expand Down Expand Up @@ -1940,7 +1944,7 @@ void AutoPlaceIcon(
}

/* this may be a good location */
if (FScreenIsRectangleOnScreen(fscr, FSCREEN_XYPOS, &ref))
if (IsRectangleOnThisPage(m, &ref, t->Desk))
Comment on lines -1943 to +1947
Copy link
Copy Markdown
Member

@ThomasAdam ThomasAdam Sep 16, 2025

Choose a reason for hiding this comment

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

Why have you changed from FScreenIsRectangleOnScreen() to IsRectangleOnThisPage()?

Copy link
Copy Markdown
Author

@bitstreamout bitstreamout Sep 17, 2025

Choose a reason for hiding this comment

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

As otherwise the defined IconBox of the user does not match but that of the global_icon_box_ptr

    while(do_all_iconboxes(t, &icon_boxes_ptr))
    {
      struct monitor *m;
fprintf(stderr, "FFFF: %s %d\n", icon_boxes_ptr->IconScreen, loc_ok);
      if (loc_ok == True)

leads to

loaded [0]: /suse/werner/.fvwm/config
FFFF: DVI-1 0
FFFF: (null) 0

on stderr output of fvwm3 (here with a not set global_icon_box_ptr->IconScreen)

Copy link
Copy Markdown
Member

@ThomasAdam ThomasAdam Sep 17, 2025

Choose a reason for hiding this comment

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

So?

My point is that this is an entirely different function call you're changing. Why?

Copy link
Copy Markdown
Author

@bitstreamout bitstreamout Sep 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What is your proposal to make the users IconBox win here. Using NULL for fscreen_scr_arg *arg in FScreenIsRectangleOnScreen() does not help

Copy link
Copy Markdown
Member

@ThomasAdam ThomasAdam Sep 17, 2025

Choose a reason for hiding this comment

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

What is your proposal to make the users IconBox win here. Using NULL for fscreen_scr_arg *arg in FScreenIsRectangleOnScreen() does not help

What I'm asking, is why you changed from FScreenIsRectangleOnScreen() to IsRectangleOnThisPage() -- it's a completely different function.

Copy link
Copy Markdown
Author

@bitstreamout bitstreamout Sep 17, 2025

Choose a reason for hiding this comment

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

compare with my current test in #1230 (comment)

Copy link
Copy Markdown
Member

@ThomasAdam ThomasAdam Sep 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We're going round in circles, @bitstreamout

What I'm saying about FScreenIsRectangleOnScreen() is it's only used in two places.

Rather than write all this additional crap in icons.c, we should change FScreenIsRectangleOnScreen()'s implementation. We already have both "rectangles" to compare, so we should stop it from looking up monitors, etc., and just pass the relevant information in.

As a consequence ef doing this, we will need to update conditionals.c as well.

Would you prefer if I did this?

Copy link
Copy Markdown
Author

@bitstreamout bitstreamout Sep 17, 2025

Choose a reason for hiding this comment

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

Would you prefer if I did this?

Indeed before I do the next crap ;)

Copy link
Copy Markdown
Author

@bitstreamout bitstreamout Sep 17, 2025

Choose a reason for hiding this comment

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

Maybe it is enough to use FSCREEN_GLOBAL with FScreenIsRectangleOnScreen() as DVI-1 has an y offset of 1920 and the global screen has twice the size aka 3840

{
loc_ok = True;
}
Expand Down Expand Up @@ -2035,6 +2039,8 @@ do_all_iconboxes(FvwmWindow *t, icon_boxes **icon_boxes_ptr)
global_icon_box_ptr->IconGrid[0] = 80;
global_icon_box_ptr->IconGrid[1] = 80;
global_icon_box_ptr->IconFlags = ICONFILLHRZ;
global_icon_box_ptr->IconScreen = "p";
global_icon_box_ptr->do_free_screen = 0;
Copy link
Copy Markdown
Member

@ThomasAdam ThomasAdam Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why add do_free_screen here?

Copy link
Copy Markdown
Author

@bitstreamout bitstreamout Sep 17, 2025

Choose a reason for hiding this comment

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

Simply to be sure that the static string is not freed ... even it free_icon_boxes() will currently not reach this.

Copy link
Copy Markdown
Member

@ThomasAdam ThomasAdam Sep 17, 2025

Choose a reason for hiding this comment

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

Is an unnecessary addition.

Copy link
Copy Markdown
Author

@bitstreamout bitstreamout Sep 17, 2025

Choose a reason for hiding this comment

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

Is an unnecessary addition.

How to resolve the NULL for the IconScreen of the last IconBox seen from do_all_iconboxes() in the loop?

}
if (*icon_boxes_ptr == NULL)
{
Expand Down
6 changes: 4 additions & 2 deletions fvwm/style.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1964,7 +1964,7 @@ static char *style_parse_icon_box_style(

/* otherwise try to parse the icon box */
IconBoxes = fxcalloc(1, sizeof(icon_boxes));
IconBoxes->IconScreen = "global";
IconBoxes->IconScreen = "g";
/* init grid x */
IconBoxes->IconGrid[0] = 3;
/* init grid y */
Expand All @@ -1976,7 +1976,9 @@ static char *style_parse_icon_box_style(
is_screen_given = True;
option = PeekToken(rest, &rest); /* skip screen */
option = PeekToken(rest, &rest); /* get the screen spec */
IconBoxes->IconScreen = option;
/* GetIntegerArguments() below removes screen */
IconBoxes->IconScreen = fxstrdup(option);
IconBoxes->do_free_screen = 1;
}

/* try for 4 numbers x y x y */
Expand Down