search

nuttyartist / notes

Fast and beautiful note-taking app written in C++. Write down your thoughts.
https://notes-foss.com
Mozilla Public License 2.0
3.72k stars 326 forks source link

Linux: Restore code for frameless window resizing #492

Closed guihkx closed 1 year ago

guihkx commented 1 year ago

Linux users can not easily resize Notes' window if they have 'frameless mode' enabled (which is the default behavior).

This change restores this ability, which was present in v1.5.0, but was later removed in v2.0.0 by commit ddd79edf077158e0e62cc2b432f669e60a7d9bfb.

Fixes #483

Before:

https://user-images.githubusercontent.com/626206/220225135-ad2ba1c8-092b-4ab9-8646-dbc659a2f3bc.mp4


After:

https://user-images.githubusercontent.com/626206/220225150-ac97a46c-1e80-4b7e-8930-ab6c258ed64f.mp4

guihkx commented 1 year ago

Regarding the before / after captures, what happened to the shadow in the after version?

Good question... I'll try to investigate that once I get home.

guihkx commented 1 year ago

Ah, I think I got it.

The shadows are still there, they're just less intensive. On a complete white background, they stand out a bit better:

image

And I think that happens because we're making the window background translucent (with Qt::WA_TranslucentBackground).

If I don't set the translucent attribute, the shadows are much more intensive (though the window itself looks terrible because of the margins added to resize the window):

image

I couldn't find a way to set the background completely transparent, but in my opinion this does not look too bad. It's certainly better than not being able to resize the window...

nuttyartist commented 1 year ago

I tested the latest build and ~works very well~. Solves the problem I mentioned in #489.

nuttyartist commented 1 year ago

Oh well. It seems to work only on the top part of the window, not the rest. It is better tho from what's on dev. I'll try to create a video.

guihkx commented 1 year ago

Oh well. It seems to work only on the top part of the window, not the rest.

It doesn't match what I show on my second video, then?

I'll try to create a video.

Thanks, that will be helpful, I think.

nuttyartist commented 1 year ago

Oh well. It seems to work only on the top part of the window, not the rest.

It doesn't match what I show on my second video, then?

I'll try to create a video.

Thanks, that will be helpful, I think.

No... it seems to work only well on the top editor area. The other top areas only shows up when I press down. Do you think it's a problem with Elementary?

https://user-images.githubusercontent.com/16375940/220425795-d463cfb5-cf04-4718-bd26-5371c6949590.mp4

guihkx commented 1 year ago

Uh-oh. That looks unusual to me.

I'm not sure what if the problem is elementaryOS-specific (works well on Ubuntu 18.04 with Unity though, which is what elementaryOS 5 is based on).

I'll setup a VM to test it out.

guihkx commented 1 year ago

I just noticed something on your video that should be fixed (in a subsequent PR).

At around 0:16 seconds, you were able to resize the window there. But that's wrong, because you should never be able to resize the window while the cursor is hovering inside the window, only outside of it.

Anyway, I could not reproduce your issue on a live CD image of elementaryOS 5:

https://user-images.githubusercontent.com/626206/220441018-23f4ce2f-6a08-4235-b09c-d3fe0582bde1.mp4


But yeah, the blurry icon issue you mentioned in #489 is real.

guihkx commented 1 year ago

I'm thinking you could've had a different version of Notes already running when you tried to open the AppImage from this pull request, because the shadows around the window on your video look different from mine.

bjorn commented 1 year ago

I'm thinking you could've had a different version of Notes already running when you tried to open the AppImage from this pull request, because the shadows around the window on your video look different from mine.

This happens to me all the time as well when I try to test stuff, heh.

The shadows are still there, they're just less intensive. On a complete white background, they stand out a bit better:

In that case, could it be that we're rendering the shadows in gray rather than black? Because they look like they are lighter than the background in your second screen capture.

nuttyartist commented 1 year ago

I'm thinking you could've had a different version of Notes already running when you tried to open the AppImage from this pull request, because the shadows around the window on your video look different from mine.

Oh, maybe. I'll test again and report back.

In that case, could it be that we're rendering the shadows in gray rather than black? Because they look like they are lighter than the background in your second screen capture.

They definitely don't look good in that video. I will test now on my end...

nuttyartist commented 1 year ago

While it might be better than what we have on dev, it's still very inconsistent on my machine. I wonder if it's because my trackpad isn't very smooth. Here's a video:

https://user-images.githubusercontent.com/16375940/220625034-dacd368a-f247-44c7-a45d-d1f109babc2b.mp4

And I strongly prefer the previous shadow, is it possible to get it back?

nuttyartist commented 1 year ago

I'm thinking you could've had a different version of Notes already running when you tried to open the AppImage from this pull request, because the shadows around the window on your video look different from mine.

And apparently, it's because Hide to tray icon was on but elementary doesn't show up the icon in the tray area (I think it doesn't support it?) so I kept thinking the app was closed while it was still open until I disabled it.

guihkx commented 1 year ago

In that case, could it be that we're rendering the shadows in gray rather than black?

It could be, but I'm not really a graphics person and the code in gaussianDist() / fillRectWithGradient() scares me a little bit. 😅

Just keep in mind that the shadows have been like that since v1.0.0, all I'm doing here is restoring the code so we can resize the window again.

it's still very inconsistent on my machine

The Qt 5 build has some mouse-grabbing issues with startSystemMove() (I reported it in #472), so once you release the mouse button, the cursor gets "stuck" until you left click anywhere inside the window again.

I also just fixed the issue where you were able to resize the window while your mouse was inside of it.

Now you should only be able to resize it by its outer edges. Please test again once you can.

And apparently, it's because Hide to tray icon was on but elementary doesn't show up the icon in the tray area (I think it doesn't support it?) so I kept thinking the app was closed while it was still open until I disabled it.

Ohhh, right. Both GNOME and elementary don't support tray icons natively, so when we close the window, Notes "hides" itself and there's no way of restoring it because we don't have the system tray...

It should be an easy fix, I think: We probably just have to check if there's a working system tray with QSystemTrayIcon::isSystemTrayAvailable() when the user requests to close the app.

nuttyartist commented 1 year ago

It should be an easy fix, I think: We probably just have to check if there's a working system tray with QSystemTrayIcon::isSystemTrayAvailable() when the user requests to close the app.

Nice one.

Now you should only be able to resize it by its outer edges. Please test again once you can.

Sure I'll test now.

nuttyartist commented 1 year ago

The Qt 5 build has some mouse-grabbing issues with startSystemMove() (I reported it in https://github.com/nuttyartist/notes/issues/472), so once you release the mouse button, the cursor gets "stuck" until you left click anywhere inside the window again.

That's annoying. With Qt6 build it doesn't happen?

Anyway, tt seems to work like 2.0.0 (but better because the width of the panes doesn't change now on each resize).

So I approve this PR.

guihkx commented 1 year ago

That's annoying. With Qt6 build it doesn't happen?

That's correct, it's a Qt5-only issue on Linux (I think it's also specific to X11).

Anyway, I just rebased against dev so I can merge this branch manually (merging with GitHub's own tool removes my commit signature).

Thanks for testing, y'all!