dunst-project / dunst

Lightweight and customizable notification daemon
https://dunst-project.org
Other
4.42k stars 338 forks source link

Gradients #1286

Open bynect opened 4 months ago

bynect commented 4 months ago

This is the implementation of #1273.

At the moment I only implemented the gradient for highlight, but it can be easily applied also to background, foreground etc. I am using a cairo linear pattern which allows for any number of colors so the user is free.

TODO:

Example

img-1708384655

highlight = "#ffffff, #cc397b, #39ff8a"
codecov-commenter commented 4 months ago

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (7775775) 64.48% compared to head (4b44dfe) 64.35%.

Files Patch % Lines
src/draw.c 42.42% 19 Missing :warning:
src/dbus.c 25.00% 6 Missing :warning:
src/notification.c 40.00% 3 Missing :warning:
src/rules.c 0.00% 2 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1286 +/- ## ========================================== - Coverage 64.48% 64.35% -0.14% ========================================== Files 48 48 Lines 7918 7956 +38 ========================================== + Hits 5106 5120 +14 - Misses 2812 2836 +24 ``` | [Flag](https://app.codecov.io/gh/dunst-project/dunst/pull/1286/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dunst-project/dunst/pull/1286/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | `64.35% <37.50%> (-0.14%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bynect commented 4 months ago

@fwsmit gladly the ci passed, so let me know what do you think 👍🏻

fwsmit commented 4 months ago

What do you mean with delta correction? One thing that may be cool to add is an angle for the gradient. But already looks cool like this

bynect commented 4 months ago

What do you mean with delta correction? One thing that may be cool to add is an angle for the gradient. But already looks cool like this

Good idea. I was thinking of adding the possibility to provide a list of xy points for the gradient breaks. But just an angle is also fine.

Let me know if I should add gradient support for other color.

For the delta correction is something that's done for the foreground but I don't think it's needed

bynect commented 4 months ago

@fwsmit @zappolowski I don't particularly like the way dunst decodes color strings at every draw call, since it makes for more difficult validation and it is plain inefficient. I was thinking of storing in the config directly the color struct and parse once in the option_parser.

Is there something that prevents me from doing that (like protocols) or can I go ahead?

Obviously I would close this pr and make it again later (and better) to store gradients in the config

Also I would remove the fact that invalid colors are black and just error out on parsing

zappolowski commented 4 months ago

I had a similar idea in mind, but TBH I didn't have the time yet to fully dig through all the code and thus cannot foresee what the implications are. From quickly skimming through the Desktop Notification Specification I could not find anything which prevents this (e.g. sending color information along with the actual notification data).

Having said that, I think parsing should be done once when reading the configuration and not on every notification being created. So, go for it, but maybe do this refactoring first and then (on top of that) add the new gradient feature.

bynect commented 4 months ago

I had a similar idea in mind, but TBH I didn't have the time yet to fully dig through all the code and thus cannot foresee what the implications are. From quickly skimming through the Desktop Notification Specification I could not find anything which prevents this (e.g. sending color information along with the actual notification data).

Having said that, I think parsing should be done once when reading the configuration and not on every notification being created. So, go for it, but maybe do this refactoring first and then (on top of that) add the new gradient feature.

Yes that was the plan, and it will make the whole feature easier

bynect commented 3 months ago

I will rework this completely. At the moment my idea is to store a cairo_pattern_t in the gradient struct