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

double window_margin_width in tab #2133

Closed
Acelya-9028 opened this issue Nov 14, 2019 · 4 comments
Closed

double window_margin_width in tab #2133

Acelya-9028 opened this issue Nov 14, 2019 · 4 comments

Comments

@Acelya-9028
Copy link

Here are two screenshots, the first without tab and the second with.

without-tab
with-tab

As you can see it's not the same margin.

I have just tested with the git version and the result is the same (kitty 0.14.6 (c4c6ef799f) created by Kovid Goyal).
This behavior is the same with window_margin_width and single_window_margin_width.

It's weird, no?

@Acelya-9028 Acelya-9028 changed the title doubled window_margin_width in tab double window_margin_width in tab Nov 14, 2019
@Acelya-9028
Copy link
Author

Acelya-9028 commented Nov 14, 2019

I'm not a c dev but I've added logs and here are the results.

In the dpi_for_os_window_id func printf("dpi: %f, x: %f, y: %f\n", dpi, os_window->logical_dpi_x, os_window->logical_dpi_y);
In the pt_to_px func printf("pt: %f, dpi: %f \n", pt, dpi);

without tab:

dpi: 96.000000, x: 96.000000, y: 96.000000
pt: 100.000000, dpi: 96.000000

with tab:

dpi: 192.000000, x: 192.000000, y: 192.000000
pt: 100.000000, dpi: 192.000000

Now, I've stopped my investigation in the big create_os_window func kitty/glfw.c:622.

EDIT: I think the response is in the get_window_content_scale function, I look that tomorrow.

@kovidgoyal
Copy link
Owner

This is on what OS?

@Acelya-9028
Copy link
Author

Arch Linux with Sway (Wayland) as wm with a scale to 2.

@Acelya-9028
Copy link
Author

So, I have added more logs. Here is the diff:

diff --git a/glfw/wl_window.c b/glfw/wl_window.c
index 73261a9f..dfdefc9e 100644
--- a/glfw/wl_window.c
+++ b/glfw/wl_window.c
@@ -55,17 +55,25 @@ static bool checkScaleChange(_GLFWwindow* window)
     if (_glfw.wl.compositorVersion < 3)
         return false;
 
+    printf("[checkScaleChange] monitor count: %i\n", window->wl.monitorsCount);
+
     // Get the scale factor from the highest scale monitor.
     for (i = 0; i < window->wl.monitorsCount; ++i)
     {
         monitorScale = window->wl.monitors[i]->wl.scale;
+        printf("[checkScaleChange] monitor scale: %i\n", monitorScale);
+
         if (scale < monitorScale)
             scale = monitorScale;
     }
 
+    printf("[checkScaleChange] new scale: %i, old scale: %i\n", scale, window->wl.scale);
+
     // Only change the framebuffer size if the scale changed.
     if (scale != window->wl.scale)
     {
+        printf("[checkScaleChange] update scale: %i\n", scale);
+
         window->wl.scale = scale;
         wl_surface_set_buffer_scale(window->wl.surface, scale);
         return true;
@@ -138,6 +146,8 @@ clipboard_mime(void) {
 }
 
 static void dispatchChangesAfterConfigure(_GLFWwindow *window, int32_t width, int32_t height) {
+    printf("[dispatchChangesAfterConfigure]\n");
+
     if (width <= 0) width = window->wl.width;
     if (height <= 0) height = window->wl.height;
     bool size_changed = width != window->wl.width || height != window->wl.height;
@@ -383,6 +393,7 @@ static void handleEnter(void *data,
                         struct wl_surface *surface UNUSED,
                         struct wl_output *output)
 {
+    printf("[handleEnter]\n");
     _GLFWwindow* window = data;
     _GLFWmonitor* monitor = wl_output_get_user_data(output);
 
@@ -406,6 +417,7 @@ static void handleLeave(void *data,
                         struct wl_surface *surface UNUSED,
                         struct wl_output *output)
 {
+    printf("[handleLeave]\n");
     _GLFWwindow* window = data;
     _GLFWmonitor* monitor = wl_output_get_user_data(output);
     bool found;
@@ -470,6 +482,8 @@ static bool createSurface(_GLFWwindow* window,
 
     window->wl.width = wndconfig->width;
     window->wl.height = wndconfig->height;
+
+    printf("[createSurface] reset scale to 1\n");
     window->wl.scale = 1;
 
     if (!window->wl.transparent)
@@ -511,6 +525,8 @@ static void xdgToplevelHandleConfigure(void* data,
                                        int32_t height,
                                        struct wl_array* states)
 {
+    printf("[xdgToplevelHandleConfigure]\n");
+
     _GLFWwindow* window = data;
     float aspectRatio;
     float targetRatio;
diff --git a/kitty/glfw.c b/kitty/glfw.c
index e7f680eb..132f4f1b 100644
--- a/kitty/glfw.c
+++ b/kitty/glfw.c
@@ -39,6 +39,8 @@ static int min_width = 100, min_height = 100;
 
 void
 update_os_window_viewport(OSWindow *window, bool notify_boss) {
+    printf("\n[update_os_window_viewport] logical_dpi_x: %f, logical_dpi_y: %f\n", window->logical_dpi_x, window->logical_dpi_y);
+
     int w, h, fw, fh;
     glfwGetFramebufferSize(window->handle, &fw, &fh);
     glfwGetWindowSize(window->handle, &w, &h);
@@ -378,10 +380,16 @@ make_os_window_context_current(OSWindow *w) {
 
 static inline void
 get_window_content_scale(GLFWwindow *w, float *xscale, float *yscale, double *xdpi, double *ydpi) {
-    if (w) glfwGetWindowContentScale(w, xscale, yscale);
+    if (w) {
+        glfwGetWindowContentScale(w, xscale, yscale);
+        printf("[get_window_content_scale][window] xscale: %f, yscale: %f\n", *xscale, *yscale);
+    }
     else {
         GLFWmonitor *monitor = glfwGetPrimaryMonitor();
-        if (monitor) glfwGetMonitorContentScale(monitor, xscale, yscale);
+        if (monitor) {
+            glfwGetMonitorContentScale(monitor, xscale, yscale);
+            printf("[get_window_content_scale][monitor] xscale: %f, yscale: %f\n", *xscale, *yscale);
+        }
     }
     // check for zero, negative, NaN or excessive values of xscale/yscale
     if (*xscale <= 0 || *xscale != *xscale || *xscale >= 24) *xscale = 1.0;
@@ -549,10 +557,15 @@ create_os_window(PyObject UNUSED *self, PyObject *args) {
     if (!common_context) common_context = application_quit_canary;
 #endif
 
+    printf("\n[create_os_window] create temp window\n");
+
     GLFWwindow *temp_window = glfwCreateWindow(640, 480, "temp", NULL, common_context);
     if (temp_window == NULL) { fatal("Failed to create GLFW temp window! This usually happens because of old/broken OpenGL drivers. kitty requires working OpenGL 3.3 drivers."); }
     float xscale, yscale;
     double xdpi, ydpi;
+
+    printf("[create_os_window] get content scale\n");
+
     get_window_content_scale(temp_window, &xscale, &yscale, &xdpi, &ydpi);
     FONTS_DATA_HANDLE fonts_data = load_fonts_data(global_state.font_sz_in_pts, xdpi, ydpi);
     PyObject *ret = PyObject_CallFunction(get_window_size, "IIddff", fonts_data->cell_width, fonts_data->cell_height, fonts_data->logical_dpi_x, fonts_data->logical_dpi_y, xscale, yscale);
@@ -565,6 +578,9 @@ create_os_window(PyObject UNUSED *self, PyObject *args) {
     // is no startup notification in Wayland anyway. It amazes me that anyone
     // uses Wayland as anything other than a butt for jokes.
     if (global_state.is_wayland) glfwWindowHint(GLFW_VISIBLE, true);
+
+    printf("[create_os_window] create final window\n");
+
     GLFWwindow *glfw_window = glfwCreateWindow(width, height, title, NULL, temp_window);
     glfwDestroyWindow(temp_window); temp_window = NULL;
     if (glfw_window == NULL) { PyErr_SetString(PyExc_ValueError, "Failed to create GLFWwindow"); return NULL; }
@@ -773,6 +789,7 @@ dbus_user_notification_activated(uint32_t notification_id, const char* action) {
 
 static PyObject*
 glfw_init(PyObject UNUSED *self, PyObject *args) {
+    printf("\n[glfw_init]\n");
     const char* path;
     int debug_keyboard = 0;
     if (!PyArg_ParseTuple(args, "s|p", &path, &debug_keyboard)) return NULL;
diff --git a/kitty/tabs.py b/kitty/tabs.py
index c24e0039..237b85e7 100644
--- a/kitty/tabs.py
+++ b/kitty/tabs.py
@@ -50,6 +50,8 @@ class Tab:  # {{{
         self.margin_width, self.padding_width, self.single_window_margin_width = map(
             lambda x: pt_to_px(getattr(self.opts, x), self.os_window_id), (
                 'window_margin_width', 'window_padding_width', 'single_window_margin_width'))
+        print('[Tab.__init__]', getattr(self.opts, 'single_window_margin_width'), self.single_window_margin_width)
+
         self.name = getattr(session_tab, 'name', '')
         self.enabled_layouts = [x.lower() for x in getattr(session_tab, 'enabled_layouts', None) or self.opts.enabled_layouts]
         self.borders = Borders(self.os_window_id, self.id, self.opts, pt_to_px(self.opts.window_border_width, self.os_window_id), self.padding_width)

And here is the result:

❯ make && ./kitty/launcher/kitty

[glfw_init]
[get_window_content_scale][monitor] xscale: 2.000000, yscale: 2.000000

[create_os_window] create temp window
[createSurface] reset scale to 1
[create_os_window] get content scale
[get_window_content_scale][window] xscale: 1.000000, yscale: 1.000000
[create_os_window] create final window
[createSurface] reset scale to 1
[xdgToplevelHandleConfigure]
[dispatchChangesAfterConfigure]
[checkScaleChange] monitor count: 0 ### hum, why 0 monitor?
[checkScaleChange] new scale: 1, old scale: 1

[update_os_window_viewport] logical_dpi_x: 96.000000, logical_dpi_y: 96.000000
[get_window_content_scale][window] xscale: 1.000000, yscale: 1.000000
[Tab.__init__] 10.0 13 ### 10.0 is the margin value in the config and 13 is the conversion to pixel
[xdgToplevelHandleConfigure]
[dispatchChangesAfterConfigure]
[checkScaleChange] monitor count: 0
[checkScaleChange] new scale: 1, old scale: 1
[handleEnter]
[checkScaleChange] monitor count: 1  ### nice :+1:
[checkScaleChange] monitor scale: 2
[checkScaleChange] new scale: 2, old scale: 1
[checkScaleChange] update scale: 2

### action: open a tab

[update_os_window_viewport] logical_dpi_x: 96.000000, logical_dpi_y: 96.000000
[get_window_content_scale][window] xscale: 2.000000, yscale: 2.000000
[Tab.__init__] 10.0 27 ### 10.0 is the margin value in the config and 27 is the conversion to pixel, 27 is the good and the wanted value

I think that the error is that there is no monitor in the first checkScaleChange and no monitor is equal to the default scale, 1.

I have written a fix, but I'm not sure, I send a PR with explanations.

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

No branches or pull requests

2 participants