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

Request: User selectable button colors #26

Closed
f-l-y-b-o-y opened this issue Oct 22, 2024 · 16 comments · Fixed by #28
Closed

Request: User selectable button colors #26

f-l-y-b-o-y opened this issue Oct 22, 2024 · 16 comments · Fixed by #28

Comments

@f-l-y-b-o-y
Copy link

With the default (black) color and dark theme buttons are almost invisible. I modified deco-theme.cpp line 210 to draw the the lines in white and looks great for now, almost matching my GTK Adwaita-Dark. It would be awesome for users to be able to assign the color in wayfire.ini (other useful options: width of a stroke and space between icons).

241022_03-49-38

@soreau
Copy link
Owner

soreau commented Oct 23, 2024

Here's a WIP patch. If it works, we might be able to consider some variant of it. The main thing that could use attention is the line width of the minimize and maximize buttons don't always match the close button, apparently because close uses diagonal lines while the others use straight ones.

@soreau
Copy link
Owner

soreau commented Oct 23, 2024

This patch adds the three options you requested. Apply it with curl https://termbin.com/d9lf | git apply from the pixdecor source directory.

@f-l-y-b-o-y
Copy link
Author

Oh man that was quick! I just tried to apply that patch, but I'm getting an error (see screenshot). I deleted and cloned the git again and tried on a fresh one with the same result...
241023_02-50-03

@soreau
Copy link
Owner

soreau commented Oct 23, 2024

This is because I made the patch against the track-wlroots-0.18 branch. You would have to apply it manually for the master branch.

@f-l-y-b-o-y
Copy link
Author

f-l-y-b-o-y commented Oct 23, 2024

Got it. Compiled just fine and works very well! Thank you. Btw I see what you mean with the width of the close button, small detail, I can live with that.

Also I think this should be the default window decoration plugin for Wayfire!

241023_12-11-34

@f-l-y-b-o-y
Copy link
Author

I've found an issue with that patch, and it's not visual but in performance. There's a significant lag and "choppiness " while resizing windows. The master branch build is perfectly smooth when resizing windows.

@soreau
Copy link
Owner

soreau commented Oct 23, 2024

Thanks for the report. I think it's due to the added resize() call which is a bit of a hack to get the buttons to refresh immediately when options are changed.

@soreau
Copy link
Owner

soreau commented Oct 23, 2024

I've found an issue with that patch, and it's not visual but in performance. There's a significant lag and "choppiness " while resizing windows. The master branch build is perfectly smooth when resizing windows.

This version should fix that particular issue.

This wayfire pull request might interest you as well.

@f-l-y-b-o-y
Copy link
Author

Can confirm the latest patch fixed that resizing lag. It is as smooth as the master branch. Thanks again!

@soreau
Copy link
Owner

soreau commented Oct 24, 2024

Maybe the disconnect between the button line widths isn't such a sin, I'll consider adding the patch. I almost think we should render the buttons with GL directly instead of cairo, but this would require further developments.

@BowDown097
Copy link
Contributor

Is this patch available somewhere still? I don't see it anywhere in the repo, and the termbin links no longer exist.

@soreau
Copy link
Owner

soreau commented Dec 13, 2024

Is this patch available somewhere still? I don't see it anywhere in the repo, and the termbin links no longer exist.

Hi, I have updated the temporary link. For what it's worth, pixdecor patches are being held up by the fact that wayfire track-wlroots branch is still not merged. Please ping @ammen99 for more information.

@BowDown097
Copy link
Contributor

Thanks. Side question for @soreau or @ammen99, if I were to contribute code to the repo at this point, should I base it on the wlroots 0.18 or master branch, or just wait? I've added title font customization, addressing #13, and also made the title respect the text-scaling-factor gsetting.

@soreau
Copy link
Owner

soreau commented Dec 13, 2024

Thanks. Side question for @soreau or @ammen99, if I were to contribute code to the repo at this point, should I base it on the wlroots 0.18 or master branch, or just wait? I've added title font customization, addressing #13, and also made the title respect the text-scaling-factor gsetting.

You can submit a PR at any time to the master branch, but I'm not pushing anything to the master branch until track-wlroots-0.18 is merged, which depends on wayfire track-wlroots branch being merged. I don't want to push anything to track-wlroots-0.18 pixdecor branch that is unrelated to tracking wlroots. So there's a chance you might have to rebase the PR in the future. The other option is to wait, as you said. This might make more sense.

@BowDown097
Copy link
Contributor

BowDown097 commented Dec 14, 2024

Well here's a patch containing my changes for now. Been using it all day and haven't ran into any issues, except for:
- Dynamic changing of the font size being a bit sloppy, requiring interacting with a window for the change to properly apply.
- The titlebar rendering weird if you make the font size very small or very large, but I'm pretty sure that issue existed already.
No idea where to begin with addressing those, and they're too minor for me to consider looking into them immediately.

@soreau
Copy link
Owner

soreau commented Dec 14, 2024

The dynamic changing is probably related to an optimization, where the title is only updated when key properties are changed. See i.e. https://github.com/soreau/pixdecor/blob/master/src/deco-subsurface.cpp#L66-L70

I'm not sure about the 'weird' titlebar rendering if font size is small or large, though I haven't tested your changes nor looked into them in any depth.

Another thing is, pixdecor prefers wayfire settings to gsettings but for this particular case, I think it might make sense, because the setting is presumably integrated with gtk.

And since you have the patch, please go ahead and make a PR against master so we don't forget about it. 👍

soreau added a commit that referenced this issue Dec 25, 2024
This allows buttons to be a png image or different color, along with spacing and line thickness options.

Fixes #26.
soreau added a commit that referenced this issue Dec 25, 2024
This allows buttons to be a png image or different color, along with spacing and line thickness options.

Fixes #26.
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 a pull request may close this issue.

3 participants