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

Better shadows #67

Closed
wants to merge 0 commits into from
Closed

Better shadows #67

wants to merge 0 commits into from

Conversation

bertini97
Copy link

Hello. I was troubled by #32 so I attempted to fix it. This is what this PR does - every change has been done to match Adwaita:

  • Shadows are differentiated between active and inactive windows
  • Inactive window shadows are softer
  • Shadows are bigger and more blurred
  • Shadow width is big enough not to produce white artifacts (see Shadows vs adwaita shadows #32)
  • Shadow width is not accounted for title position

Here are the results for active and inactive windows:

Schermata del 2024-06-02 13-46-47
Schermata del 2024-06-02 13-46-59

@grulja
Copy link
Contributor

grulja commented Jun 2, 2024

Doesn't this make the resizable area around windows too big?

@bertini97
Copy link
Author

Doesn't this make the resizable area around windows too big?

I am unfamiliar with this. Is this the area that appears as transparent around the window in screenshots? Because Adwaita windows have a huge one.

@bertini97
Copy link
Author

I increased the shadow width to something that doesn't show the white artifact anymore. It's even bigger now. It doesn't give me any problem on gnome.

@grulja
Copy link
Contributor

grulja commented Jun 2, 2024

This is what I'm talking about:
Screenshot_20240602_151433

See that you can resize the window even when the mouse is quite far away from the right window border. I'm not sure making the shadows that wide is correct and the original 10px is perfectly fine. I think to fix the decorations we should implement them differently and avoid using qt_blurImage

@bertini97
Copy link
Author

I see what you mean. Indeed, the patch makes it huge. It's not a problem for me, but I understand not wanting it.

In the latest changes I brought the size down to 16px. I think it's a good compromise. Feel free to close this PR if you don't want this implemented at all, but please consider leaving the shadow on unfocused windows.

@grulja
Copy link
Contributor

grulja commented Jun 3, 2024

I see what you mean. Indeed, the patch makes it huge. It's not a problem for me, but I understand not wanting it.

In the latest changes I brought the size down to 16px. I think it's a good compromise. Feel free to close this PR if you don't want this implemented at all, but please consider leaving the shadow on unfocused windows.

Can you split this into more changes? The size change is something that needs to be compared to what GNOME does use so we are as most as close to their results. Same for the shadows for active/non-active windows as I'm not sure shadows are used there in such case. But your change to not include the shadows for titlebar text calculation is I think a good one.

@bertini97
Copy link
Author

Makes sense. I'll submit multiple PRs if that's ok.

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.

2 participants