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

fix: Used GdkMonitor to set default window size #1743

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Huzaifa-code
Copy link

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:

  • Fetch screen size using GdkMonitor.
  • Calculate default window size as 80% of the screen size.
  • Ensure the minimum window size is at least 852x652 pixels.
  • Set geometry hints and default window size based on calculated dimensions.

Image :
Screenshot from 2024-07-14 17-54-18

Copy link

github-actions bot commented Jul 14, 2024

Everyone contributing to this PR have now signed the CLA. Thanks!

@Huzaifa-code Huzaifa-code reopened this Jul 14, 2024
@Huzaifa-code Huzaifa-code reopened this Jul 14, 2024
@Huzaifa-code Huzaifa-code reopened this Jul 14, 2024
@Huzaifa-code Huzaifa-code reopened this Jul 14, 2024
@d-loose d-loose enabled auto-merge July 14, 2024 21:00
@d-loose d-loose disabled auto-merge July 14, 2024 21:00
@spydon spydon changed the title FIX: Used GdkMonitor to set default window size fix: Used GdkMonitor to set default window size Jul 15, 2024
@Huzaifa-code
Copy link
Author

Can someone help me in signing CLA ( Contributor license agreement ) ,
what should i enter in (i am individual contributor) - Please add the Canonical Project Manager or contact = input,
if enter all the data and submit - it shows :
image

but when i check its not getting signed up :
image

this is my first contribution to ubuntu.

Copy link
Member

@d-loose d-loose left a 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!

Comment on lines 39 to 40
GdkDisplay* display = gdk_display_get_default();
GdkMonitor* monitor = gdk_display_get_primary_monitor(display);
Copy link
Member

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).

Copy link
Member

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 :)

Copy link
Author

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

Comment on lines 47 to 48
gint default_width = screen_width * 0.8;
gint default_height = screen_height * 0.8;
Copy link
Member

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?

@Huzaifa-code
Copy link
Author

I have made some changes, please do review it.



// Ensure the window size is within the predefined sizes
if (default_width < min_width) {
Copy link
Member

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) {
Copy link
Member

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);
Copy link
Member

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?

Copy link
Contributor

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...

Copy link
Contributor

Choose a reason for hiding this comment

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

Also tested the "draw" event... maybe a solution is to connect to it and, every time the screen resolution changes, update the values...

imagen

Copy link
Contributor

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.

@Huzaifa-code
Copy link
Author

Huzaifa-code commented Jul 17, 2024

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);
and passing x,y - as current cursor position.

Tested on Laptop(1366x768) and a external monitor(1920x1080)

below are screenshots :

Screenshot from 2024-07-17 23-47-09

Screenshot from 2024-07-17 23-47-34

Screenshot from 2024-07-17 23-49-11

Screenshot from 2024-07-17 23-49-25

Known issue [TODO] :

  • in external monitor screen size(1920x1080) is detected and default width and height are set to 1280 and 800 respectively, still window is opening in fullscreen

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);
Copy link
Contributor

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.

Copy link
Author

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

Copy link
Contributor

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.

Copy link
Author

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

Copy link
Contributor

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.

Copy link
Author

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 👍

Copy link
Author

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.

Copy link
Contributor

@sergio-costas sergio-costas Jul 22, 2024

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));
}

Copy link
Collaborator

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?

@Huzaifa-code
Copy link
Author

All Done

fixed windows size opening in fullscreen in external monitor, had to change max_width and max_height if set max_width=1280 or greater and max_height=800 it is opening in fullscreen. so currently set max_width=1080 and max_height=700
tested on external monitor - 1920x1080

Screenshot from 2024-07-18 13-17-32

Screenshot from 2024-07-18 13-17-11

@sergio-costas
Copy link
Contributor

Just commenting again because I accidentally unsubscribed

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

Successfully merging this pull request may close these issues.

4 participants