-
Notifications
You must be signed in to change notification settings - Fork 115
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
fix: Used GdkMonitor to set default window size #1743
base: main
Are you sure you want to change the base?
Conversation
Everyone contributing to this PR have now signed the CLA. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for your contribution!
We've talked about this previously and encountered some issues with multi monitor setups - see comments.
I'll have a look at the CLA check and try to find out why it's failing again!
GdkDisplay* display = gdk_display_get_default(); | ||
GdkMonitor* monitor = gdk_display_get_primary_monitor(display); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will cause some problems with multi monitor setups (where typically the primary monitor is the largest one, so the window will be too big when the app is opened on another one).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried something similar before, but ran into issues getting the correct monitor. You can take a look at my old branch here if you want to have a go at that :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, sure i'll check this out
gint default_width = screen_width * 0.8; | ||
gint default_height = screen_height * 0.8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think always setting the width and height to 80% of what's available works well with the design. I think we initially discussed automatically setting the window size to either 800x600 or 1280x800, depending on whether enough space is available or not.
@anasereijo did we consider showing an even smaller window as well?
I have made some changes, please do review it. |
|
||
|
||
// Ensure the window size is within the predefined sizes | ||
if (default_width < min_width) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be default_width < max_width
here, so that we show an 800x600 window if the screen is smaller than 1280x800?
// Ensure the window size is within the predefined sizes | ||
if (default_width < min_width) { | ||
default_width = min_width; | ||
} else if (default_width > max_width) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the change from my previous comment, this would need to be >=
} | ||
|
||
GdkDisplay* display = gdk_window_get_display(gdk_window); | ||
GdkMonitor* monitor = gdk_display_get_monitor_at_window(display, gdk_window); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With my current setup (laptop + external monitor) this always returns the (smaller) laptop screen. @sergio-costas do you know what the problem is here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a little test program, and it happens to me too. I tried using g_signal_connect_after()
for "realize" signal, but didn't work. Neither worked using "map" nor "map-event". But if I ask the data "manually" with a button, when the window is fully shown, then it works...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW: also tested "screen-changed" signal, but no dice.
made some changes now it's detecting screen width and height of current active window by using function GdkMonitor* monitor = gdk_display_get_monitor_at_point(display, x, y); Tested on Laptop(1366x768) and a external monitor(1920x1080) below are screenshots : Known issue [TODO] :
|
gdk_device_get_position(pointer, nullptr, &x, &y); | ||
|
||
// Get the monitor at the cursor position | ||
GdkMonitor* monitor = gdk_display_get_monitor_at_point(display, x, y); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not reliable: if the user moves the pointer in the right moment, it will fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if user opens(clicks) app in external monitor and then moves cursor to different screen(let's say laptop) before app is opened then the app will open in the laptop screen and size will adjust according to the display where cursor is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's sensitive to race conditions: If the mouse cursor crosses the boundary after the window has been mapped but before the code gets the position, we can have a wrong value. I personally think that we should use gdk_display_get_monitor_at_window.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay that is possible , the issue with gdk_display_get_monitor_at_window is that it is setting the screen_width and screen_height of the primary display & opens app on primary display not the one the user is interacting with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but as I commented, in the second MAP event, the data returned is the right one, so I think that the solution is to get the current size in the MAP event, compare it with the last one to see if it has changed (the first time, of course, you must consider it as changed), and if changed, resize the window. Also, in my opinion, it would be even better to just launch an idle task from the MAP callback if the size has changed, and in that idle task do the size change, to avoid doing the change inside the MAP event processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay i'll try that 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't make it work other than current solution, i think we sould just open the window in primary display and let the user move and resize, if user want to move to external monitor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My fault: it was the draw
signal, not the map
one. Anyway, here is my test code. I tested it and it works fine.
#include "my_application.h"
#define APPLICATION_FLAGS G_APPLICATION_NON_UNIQUE
struct _MyApplication {
GtkApplication parent_instance;
};
G_DEFINE_TYPE(MyApplication, my_application, GTK_TYPE_APPLICATION)
gboolean set_new_size(gpointer user_data) {
// assigning it to a g_autofree to ensure that it is automagically
// freed at the end of the function
g_autofree GdkRectangle *geometry = user_data;
g_print("New geometry detected: %d, %d, %d, %d\n", geometry->x, geometry->y, geometry->width, geometry->height);
// Ensures that this function isn't called again. It must be called only once per change.
return G_SOURCE_REMOVE;
}
void on_window_realize(GtkWidget* widget, gpointer user_data) {
static int x = 0, y = 0, width = 0, height = 0;
GdkWindow* gdk_window = gtk_widget_get_window(widget);
if (gdk_window == NULL) {
g_print("No Gdk window\n");
return;
}
GdkDisplay* display = gdk_window_get_display(gdk_window);
GdkMonitor* monitor = gdk_display_get_monitor_at_window(display, gdk_window);
g_autofree GdkRectangle *geometry = g_malloc(sizeof(GdkRectangle));
gdk_monitor_get_geometry(monitor, geometry);
g_print("Called geometry callback with geometry: %d, %d, %d, %d\n", geometry->x, geometry->y, geometry->width, geometry->height);
// If the geometry has changed since the last signal, launch an idle task to update the size of the window
if ((geometry->x != x) || (geometry->y != y) || (geometry->width != width) || (geometry->height != height)) {
x = geometry->x;
y = geometry->y;
width = geometry->width;
height = geometry->height;
g_idle_add(set_new_size, g_steal_pointer(&geometry));
}
}
static void button_clicked(GtkButton *button, gpointer data) {
g_print("Button clicked\n");
on_window_realize(GTK_WIDGET(data), NULL);
}
static gboolean draw_event(GtkWidget *self, cairo_t *cr, gpointer data) {
g_print("MAP event\n");
on_window_realize(GTK_WIDGET(data), NULL);
return TRUE;
}
// Implements GApplication::activate.
static void my_application_activate(GApplication* application) {
MyApplication* self = MY_APPLICATION(application);
GtkWindow* window = GTK_WINDOW(gtk_application_window_new(GTK_APPLICATION(application)));
gtk_window_set_application(window, GTK_APPLICATION(application));
GtkButton *button = GTK_BUTTON(gtk_button_new_with_label("Check monitor"));
gtk_container_add(GTK_CONTAINER(window), GTK_WIDGET(button));
g_signal_connect(G_OBJECT(button), "clicked", G_CALLBACK(button_clicked), window);
g_signal_connect_after(window, "realize", G_CALLBACK(on_window_realize), NULL);
// Pass the window in the data pointer
g_signal_connect_after(window, "draw", G_CALLBACK(draw_event), window);
gtk_widget_show_all(GTK_WIDGET(window));
}
// Implements GObject::dispose.
static void my_application_dispose(GObject* object) {
MyApplication* self = MY_APPLICATION(object);
G_OBJECT_CLASS(my_application_parent_class)->dispose(object);
}
static void my_application_class_init(MyApplicationClass* klass) {
G_APPLICATION_CLASS(klass)->activate = my_application_activate;
G_OBJECT_CLASS(klass)->dispose = my_application_dispose;
}
static void my_application_init(MyApplication* self) {}
MyApplication* my_application_new() {
return MY_APPLICATION(g_object_new(my_application_get_type(),
"application-id", "com.rastersoft.testapp", "flags",
APPLICATION_FLAGS, NULL));
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Huzaifa-code are you planning to continue working on this?
Just commenting again because I accidentally unsubscribed |
This PR updates the application to use GdkMonitor for retrieving the screen size. This change ensures that the default window size is calculated based on the actual screen dimensions, providing a more adaptive and consistent user experience.
Changes:
Image :