On January 30th, 2014, 12:30 p.m. CET, Martin Gräßlin wrote:

I tried to create a test application demonstrating the behavior using the values from https://bugzilla.gnome.org/attachment.cgi?id=193285 but with pure xcb, KWin/5 does not show this problem.

It would probably help if I could see the xprop of the affected windows. Unfortunately I don't have any gtk development environment set up, so I cannot just test myself.

For reference the test code I used:
#include <stdlib.h>
#include <xcb/xcb.h>
#include <xcb/xcb_icccm.h>

int main(int, char **)
{
    int screenNumber;
    xcb_connection_t *c = xcb_connect(nullptr, &screenNumber);

    auto getScreen = [=]() {
        const xcb_setup_t *setup = xcb_get_setup(c);
        auto it = xcb_setup_roots_iterator (setup);
        for (int i = 0; i < screenNumber; ++i) {
            xcb_screen_next(&it);
        }
        return it.data;
    };
    xcb_screen_t *screen = getScreen();

    xcb_window_t w = xcb_generate_id(c);
    const uint32_t values[2] = {screen->white_pixel, XCB_EVENT_MASK_BUTTON_PRESS | XCB_EVENT_MASK_BUTTON_RELEASE};

    xcb_create_window(c, 0, w, screen->root, 0, 0, 80, 36, 0, XCB_WINDOW_CLASS_INPUT_OUTPUT,
                      screen->root_visual, XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK, values);

    // set the normal hints
    xcb_size_hints_t hints;
    hints.flags = XCB_ICCCM_SIZE_HINT_P_MIN_SIZE | XCB_ICCCM_SIZE_HINT_BASE_SIZE | XCB_ICCCM_SIZE_HINT_P_RESIZE_INC;
    hints.min_width = 8;
    hints.min_height = 15;
    hints.base_width = 0;
    hints.base_height = 0;
    hints.width_inc = 8;
    hints.height_inc = 15;
    xcb_icccm_set_wm_normal_hints(c, w, &hints);

    // and map the window
    xcb_map_window(c, w);
    xcb_flush(c);

    while (xcb_generic_event_t *event = xcb_wait_for_event(c)) {
        bool exit = false;
        if ((event->response_type & ~0x80) == XCB_BUTTON_RELEASE) {
            exit = true;
        }
        free(event);
        if (exit) {
            break;
        }
    }

    xcb_disconnect(c);
    return 0;
}

On January 30th, 2014, 12:49 p.m. CET, Thomas Lübking wrote:

The Problem is that the gnome terminal class (also powers emacs and gvim - famous bugs ;-) tries to play WM and "fixes" the size set by the WM.
The KWin code is ultra-twisted and the comments do not match exactly what is done.
Also the code seems to stem from fvwm -> KWM -> KWin =)

Atm, the code seems correct (while reads wrong)
KWin merges base_width into min_width and uses min_width as base_width - this seems due to the different demands on aspect ratio calculation, where the claim is:

// According to ICCCM 4.1.2.3 PMinSize should be a fallback for PBaseSize for size increments,
// but not for aspect ratio.

I could not verify this statement, but *that* is what causes the divergence and needs to be sorted out.

On January 30th, 2014, 1:04 p.m. CET, Marek Marczykowski-Górecki wrote:

The problematic case is when application provides both min size and base size - then KDE uses only min size for ResizeInc handling and ignores base size. The code:
    if (xSizeHint.flags & PBaseSize) {
        // PBaseSize is a fallback for PMinSize according to ICCCM 4.1.2.3
        // The other way around PMinSize is not a complete fallback for PBaseSize,
        // so that's not handled here.
        if (!(xSizeHint.flags & PMinSize)) {
            xSizeHint.min_width = xSizeHint.base_width;
            xSizeHint.min_height = xSizeHint.base_height;
        }
(...)
    int basew_inc = xSizeHint.min_width; // see getWmNormalHints()
    int baseh_inc = xSizeHint.min_height;
    w = int((w - basew_inc) / width_inc) * width_inc + basew_inc;
    h = int((h - baseh_inc) / height_inc) * height_inc + baseh_inc;


Aspect ratio code is ok (which uses directly xSizeHint.base_*), only ResizeInc handling is wrong.

Yes, that gnome terminal "behaviour" is also a bug, but separate one, not the only one. 

BTW hints of affected window:
WM_NORMAL_HINTS(WM_SIZE_HINTS):
		program specified minimum size: 356 by 104
		program specified resize increment: 9 by 18
		program specified base size: 15 by 64
		window gravity: NorthWest

On January 30th, 2014, 1:26 p.m. CET, Martin Gräßlin wrote:

@Thomas: I just verified and the comment is correct

I just tried with the values from the provided size hints and the problem is not reproducable. Thus the shrinking is related to client "playing WM".

On January 30th, 2014, 1:42 p.m. CET, Marek Marczykowski-Górecki wrote:

Yes, only in combination with clients "playing with WM" (which is separate bug, gnome-terminal this time) it causes window shrink, in all other cases, window simply will gets wrong size. Every sane application should simply accept those sizes, but still - there are inconsistent with hints because of bug, not because of some real need of WM.

For example window with:
WM_NORMAL_HINTS(WM_SIZE_HINTS):
		program specified minimum size: 352 by 104
		program specified resize increment: 9 by 18
		program specified base size: 15 by 64

Can get size 775x320, which is (352 + 47*9) x (104 + 12*18), but not in form of (15 + i*9) x (64 + j*18).

To reproduce this bug, simply install and run gnome-terminal under KDE, it doesn't require whole Gnome installed.

On January 30th, 2014, 2:57 p.m. CET, Martin Gräßlin wrote:

ok I'm convinced ;-) I have my test-application in a state that it shows the problem and switching to base_width and base_height fixes the problem.

On January 30th, 2014, 3:26 p.m. CET, Thomas Lübking wrote:

> The problematic case is when application provides both min size and base size...
Yupp, it's rather that we merge, not how we merge.

As a minor nitpick, i'd suggest to twist logics, since PBaseSize will be the irregular case.

On January 30th, 2014, 3:36 p.m. CET, Martin Gräßlin wrote:

I pushed my test-app into a new tests/ directory in kwin. See http://commits.kde.org/kde-workspace/1569723cc3b04881bd4de9cce82e222d8b34c989

Congratulations: you got me to start adding small test applications to verify that our behavior is correct \o/

On January 30th, 2014, 6:01 p.m. CET, Marek Marczykowski-Górecki wrote:

@Thomas: surprisingly many applications specifies base size (as 0x0). If you want to save some CPU cycles, its better to check if PResizeInc flag is specified (or if width_inc != 1 || height_inc != 1) - the current code always calculates new w/h regardless of this flag.

On January 30th, 2014, 7 p.m. CET, Thomas Lübking wrote:

> surprisingly many applications specifies base size (as 0x0)
The only one i found is mplayer (which apparently has a 4/4 min_size - makes me wonder whether we should ignore explicit 0x0 base_size), which of course has a fixed aspect ratio ...
No Qt, nor chromium nor gimp (about the gtk+ apps on my desktop) does here. Gtk3 thing?

On January 31st, 2014, 4:25 a.m. CET, Marek Marczykowski-Górecki wrote:

Perhaps Gtk3. All randomly chosen gnome3 applications (gnome-dvb-control, gnome-power-statistics, gnome-calculator) set 0x0 base_size and some positive min_size, without settings any *_inc nor aspect ratio.

Ignoring 0x0 base size sounds wrong to me - IIUC it is there to prevent fallback to min size in case of PResizeInc used. So again, if you want to save some CPU cycles, IMO the better option is to calculate new size (based on *_inc, base_*, etc) only if PResizeInc is specified - similary as in case of PAspect. I can prepare a patch for this (here, or create new request?).

I don't think it's necessary to try to save CPU cycles. To me it doesn't matter whether the check is the one way around or the other. So I'd say the current version is OK: ShipIt(TM)

Do you have commit rights or should we push it?

- Martin

On January 30th, 2014, 6:56 a.m. CET, Marek Marczykowski-Górecki wrote:

Use base_width/base_height if provided by the application and fallback to min_width/min_height only if not (according to ICCCM 4.1.2.3).
Previously always min_width/min_height was used (if provided), even if base_width/base_height was also provided.

This fixes long standing bug with shrinking gnome-terminal window. Haven't found it in KDE Bugs, but described here: https://bugzilla.redhat.com/show_bug.cgi?id=707664

Tested patched kde-workspace-4.10.5 (Fedora 18), I don't any significant changes until now.
Patched version allows normal gnome-terminal use, previously it was shrinking to minimal size.