[WIP] Mutter 3.36.9 rebase #601
Conversation
|
Well that it's a really big and amazing goals. I also think this is the only way to go if Cinnamon want to survive in long terms. The most important thing now is probably fix the build, because this change need to be tested a lot and that means request the help of all possible people. It probably will require a blog post about the introduction of this changes and the instruction of how to compile and install it from source or a repository to install this branch for test purpose only. About the windows tilling I think it's good to take a look at problems with the pop-shell implementation and probably evaluate his own code instead of the usage of the shelltiling extension. As Clutter is internally now and at it have a lot of changes, should be good think about also make a review of the GNOME updates in St, to evaluate what should be needed to add. Think about Wayland in this point should be just crazy, so i think is good not take anything about Wayland in account for now, just only X11. I know @mtwebster invested a lot of time to rebuild and unify as possible the GNOME keybinding mechanisms some years ago. It should be also a point to take in consideration, as currently this change was reverted with the introduction of the new Mutter code base. I think that all changes that involve the Cinnamon cjs code need to be address as possible with the introduction of a code hijacked like this. This should be done instead of try to fix all occurrence, because fix all occurrences can be done in new iterations subsequently if needed and then remove the hijacked code. Any feature that should added to muffin only, should be introduced as an extension of the existing code and not as an override of it, to ensure a more soft update in the future. Should now be also more easy detect where the desktop can be needed an improved thanks to the GNOME plataform profiling initiative https://blogs.gnome.org/chergert/2020/03/14/how-to-use-sysprof-to/ as it should work now in Cinnamon. Of course all that was my opinion, but the truth is that it makes me want to install Cinnamon and see closely all the evolution of this development in progress. Cinnamon should be really much faster after the change. |
|
@clefebvre thanks for your big works, I taken only a fast look for now and some small things:
|
clefebvre
force-pushed
the
muffin-5.2-test
branch
from
0e55d00
to
8dbf865
Compare
|
@clefebvre why you removed man page (instead rename it)? if you don't want it is remained debian/muffin.postinst that use it to fix |
|
Hi Fantu, We don't really use or expose muffin (the binary) to users. It's only used by us to troubleshoot issues and we never had a man page for it before. |
There is a man page for muffin, see https://github.com/linuxmint/muffin/tree/master/doc/man |
|
From a maintainer perspective, even if it's unused if there is a binary there should be a man page although building three muffin binary itself can be disabled |
|
Still very ALPHA/broken, but it builds and installs fine now. |
|
@clefebvre thanks, take a look to linuxmint/cinnamon#10377 to make cinnamon part of this task building also in a clean environments, even if the real cause of the issue is e3b579e (merge of -dev without add all -dev deps) edit: Can you keep libmuffin-dev package with development thing separated (as should be) and with all correct deps please? (add -dev deps is not good, better keep -dev package separated as should be to avoid install of many packages not needed for major of users) done here (Fantu@77b90b0) if you want cherry-pick and done also this other small commits: Fantu@2ec9296 Fantu@4441a90 |
|
I added to my commit also the missed breaks/replace in -dev package to make working also upgrade (sorry for mistake) |
|
@clefebvre thanks for readd of the -dev package, is missed the cinnamon part, here updated build-deps/deps and meson dep: linuxmint/cinnamon#10385 for people that want do a fast test I prepared a ppa, I tested on mint 20.2, added linux mint nightly build ppa and my ppa @mtwebster @clefebvre I did a fast test, I had 2 crash on cinnamon-settings for missed schema one with "workspace-cycle" and one "tile-hud-threshold" (probably already know them), out of settings and know bugs no crash and the performances seems better |
|
@Fantu thanks, I wouldn't worry about csettings just yet. There are so many bugs with the core desktop still, one thing at a time :) |
|
I updated my ppa with latest muffin/cinnamon fixes and did another fast test (only in a vm for now) I don't have time now to do a check about it if related to muffin rebase and if I can do a fix, I posted if can be useful, if you need a specific test for help or something else try to tell and I'll do if I'll have enough time, for now I'll try at least to update ppa for anyone want test it in easy/fast way |
|
@Fantu I think some of those are coming from an issue with the backgrounds. When I first start I get a black background and those errors. But if I restart cinnamon, I get a proper background and those errors seem to go away. |
|
Well i installed this branch over an updated Mint Uma virtual machine (with VirtualBox) using this repository: https://launchpad.net/~kelebek333/+archive/ubuntu/muffin-3.36rebase-test So, @Fantu, maybe is better that you make your repository private, to not introduce more things to take on account in a user report. This because I think that to have two different source, form where install this could be a little complicate to be handled. Point apart, latest version of Uma is broken in VirtualBox running with accelerate graphics (acceleration is working well with this branch). I can reproduced the bug "Laggy category change in application menu (vector box, need to fix st-polygon build)" and it remember me an already problem that i faced with the cinnamon menu, running in mutter some time ago. Please see: https://bugzilla.gnome.org/show_bug.cgi?id=778158 as probably it could help. Anyway also if this fix is apply, sure get a focus chain when we only need the next visible actor it's not a good idea. Also it's not a good idea simulate a focus change as currently is doing the cinnamon menu, because this is not friendly with the accessibility. It's preferable really move the focus and avoid all that at once and then fix orca to be used for users with "different abilities". |
|
@lestcape uma have already daily build packages by default? I saw that kelebek333 ppa don't depend on dailybuild and don't have full packages updated from git in it, on my ppa I keep daily build as dep for rebuild only cinnamon and muffin for this task (like when this will be merged and also because cinnamon also include other commits for next version) and I wrote instructions for use it on a post above (now I'll also include in ppa description to avoid mistake). and about my additional commits I did only few small things to make cinnamon build and very small packaging improvements awaiting that will be merged, should be nothing that change the tests, or I'm missing something? edit: about my vm (with kvm) don't going to fallback anymore in latest test with rebase (with latest cinnamon commits of latest 2 days) |
|
Seem to be that the _keepMaskActive is getting a wrong value and is jumping between true and false because the VectorMask was disabled and enabled several times during the category-box motion event in a direction that this should not happen, causing a lot of changes in the actor reactivity, what seem to cause also a theme-node update for all category buttons. @Fantu what i said was just a suggestion... It's fine to me if you think differently.
I think it depend. At less apparently is updated with this branch otherwise I would not have talked about it. Sorry if im wrong. Edit: I see now what you said and why, so, I understand what you do now. If i want to use your repository i also will need the daily-build repository, but not if i use the kelebek333 repo, because it include a copy of the other dependencies from the daily-build, but in an static way, that need to be updated manually. I don't know what approach should be better for test this, but as they are that way we can not ensure they will produced the same result for same test, because other the daily-build package can change in the middle and affect the result of the test. To be honest I didn't know how your repository would work with only two packages. Now I understand that the daily-build had to be added before.This was my bad, and the reason of why i selected the other repo.
I don't know what could be the difference, that is the point. I only saw that kelebek333 is handled more packages that what i will get if i used your repository and the duplicate effort is not a good thing. That was all. I ask only to you, because i don' t know who is kelebek333... |
|
This is my proposition to fix the vector-box _keepMaskActive jumping issue: The reason of why _keepMaskActive is jumping between true and false is because the current code is enabled the VectorMask without to really check if we need to enable it. To check that, we first need to have a the information of the vector_mask (this.vector_mask_info) to then check it, so the _categoryMotionEvent function should be convert to something like this: _categoryMotionEvent(actor, event) {
// Always keep the mask engaged - motion-events on the category buttons
// trigger this.
if (this.vector_update_loop == 0) {
if (!this.vector_mask_info) {
this.vector_mask_info = this._getNewVectorInfo(actor);
}
if (this._keepMaskActive()) {
this._enableVectorMask(actor);
}
}
return Clutter.EVENT_PROPAGATE;
}Unfortunately, this is not enough, because there are another problem. The problem is that we are using the broken function calc_angle to finding our angle direction. The broken part of the function is here: if (x == 0 || y == 0) {
return 0;
}because in our cartesian system references when y == 0 the angle is not always 0 because depend of the sign of x it can be 0 or 180 degrees. The same occurs when x == 0 the angle is not 0 is instead 90 or -90 degrees depending of the y value, if is positive or negative. So to fix that we need to replace the calc_angle function with this one: function calc_angle(x, y) {
let r = Math.atan2(y, x) * (180 / Math.PI);
return r;
}Please note that Math.atan2 is giving us the right value automatically, so the right thing to do here is not do anything more. Here, the problem of the menu was gone, but i think that we also can add some improvements. For example: log(`${this.vector_mask_info.top_angle.toFixed()} <---${angle.toFixed()}---> ${this.vector_mask_info.bottom_angle.toFixed()} - Continue? ${ret}`);and this not get us an easy way to know if the angle is negative or positive because the separator <--- have the same character of the negative sign. Another improve is just to add a gap to our angle to ensure that the corner cases do not activate the vectorbox. I think that 10 degrees is enough. So, we can then check if our angle is in range in this way instead: ret = angle <= this.vector_mask_info.top_angle + 10 &&
angle >= this.vector_mask_info.bottom_angle - 10; |
|
Please note this another bug: The Edit: void
meta_display_keybinding_action_invoke_by_code (MetaDisplay *display,
unsigned int keycode,
unsigned long mask)
{
MetaKeyBindingManager *keys = &display->key_binding_manager;
xkb_keycode_t code = (xkb_keycode_t) keycode;
MetaResolvedKeyCombo resolved_combo = { &code, 1 };
resolved_combo.mask = mask_from_event_params (keys, mask);
MetaKeyBinding *binding = get_keybinding (keys, resolved_combo);
if (binding)
invoke_handler (display, binding->handler, null, null, binding);
}Another possibility should be also add the action and the handler to the internal _bindings object of the KeybindingManager: this._bindings[name] = settings;
this._bindings[name].action = ret;
this._bindings[name].handler = handler;And then in Main instead of: if (action == Meta.KeyBindingAction.CUSTOM) {
global.display.keybinding_action_invoke_by_code(keyCode, modifierState);
}We can do: if (action > Meta.KeyBindingAction.LAST) {
for (let name in keybindingManager._bindings) {
if (keybindingManager._bindings[name] && keybindingManager._bindings[name].action == action) {
keybindingManager._bindings[name].handler();
break;
}
}
} |
mtwebster
force-pushed
the
muffin-5.2-test
branch
from
7aea0d8
to
6a2abe8
Compare
mtwebster
force-pushed
the
muffin-5.2-test
branch
from
ae009ab
to
a3c6bf4
Compare
|
Thank you @clefebvre for your continued maintenance of Mint/Cinnamon! I'm a multiple time $ contributor to the project. Not sure if this is the appropriate venue for this, but I just wanted to cast my vote on 'resize threshold' setting. I do use and appreciate this setting. I find it especially useful with low-precision laptop touchpads. |
Fractional scaling should be picked up ok, but base scale is dropped - muffin can figure out where this should be - maybe we'll revisit it. This hasn't been tested against complex, multi-monitor, multi- scale configurations, but should work fine.
Currently this is 48px by default which is triggered too easily just moving windows normally around the screen. Change it so that it's only triggered when moving the mouse right against the top edge.
Instead of it being based on the shake-loose threshold, it's traditionally been 1px wide on extreme edges and 5px on shared edges. This restores that behavior.
- remove unused build options, set remaining ones to our preferred defaults. - clean up debian/rules some more
compositor so they can be managed properly with the background. Add a way to control the desklet container level. This wasn't an issue before, as the desklet container wasn't part of the window group, but now it is so we need to handle it during stack updating.
d/rules: restore armel and armhf default_driver change
keyboard device is used. ref: https://gitlab.gnome.org/GNOME/mutter/-/issues/398 New muffin uses xinput2 instead of xlib for doing key grabs, and is incredibly slow at it in comparison. NewKeyboardNotify can be sent when using auxillary mouse buttons or the volume encoder on a keyboard. It ends up causing a very noticeable UI freeze when it happens. This seems to only affect nvidia.
style classes and elements changing. Muffin only uses a very narrow set of theme elements, and they chain from the ssd window background changing. This doesn't happen when we switch from Mint-Y to Mint-Y-Blue, for example, as they all have the same background.
display: Add helper functions for translating between logical and xinerama monitor numbers. Monitor 0 is traditionally the primary monitor. Logical monitors don't work this way - if you change the primary, its number stays the same. Fortunately the underlying x number is still available. This may eventually be complicated by monitors that allow multiple logical inputs (tiling), but I have none to test with.
mtwebster
force-pushed
the
muffin-5.2-test
branch
from
c3b6bf6
to
559c421
Compare
|
Nice! @Fantu you said you were going to adjust the hardening in Debian? |
|
@ItzSwirlz as you can see debian/rules of muffin rebase there is already all hardening enabled (same of mutter), now that code are improved with the rebase and on tests I did for now I not found issue related so will be applied also to muffin 5.4 debian package |
|
About window corner tiling, you probably want to wait for GNOME: https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2512#note_1523744 instead of make it in js with a bad performance... |
|
Also you probably want to fork mutter again to a point where you can add this: https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1441 Have 60 FPS instead of 30 FPS in x11 seem to be as very good improve. |
needed:
Some transition notes (of interest to xlet devs mainly):
https://docs.google.com/spreadsheets/d/1AHbuhi5bnlN_O0JP0O-rmgbN50IBHqZm74w6A1TL8Nk/edit?usp=sharing
This is just an experiment.
Loss of functionality
The following features would be lost and not brought back:
The following features would need to be brought back:
Regressions
Existing wm and desktop settings- mostly restored.Titlebar menu (removed in GNOME 3.13.2 GNOME/mutter@8640982)- using clutter menu for nowBugs
Pros
More than 3 keyboard layouts at once.- postponed for nowRequire adaptation
Filenames differences with 5.0
gir1.2-meta-muffin-0.0
libmuffin0
muffin-common
muffin
muffin-dbg