search

fyne-io / systray

a cross platfrom Go library to place an icon and menu in the notification area
Apache License 2.0
227 stars 42 forks source link

Add support for auto-scaling images #9

Closed dhaavi closed 1 year ago

dhaavi commented 2 years ago

The idea behind this PR is that different implementations need different optimal image sizes and not all tray implementations are good at scaling the images. The new public variable allows for developers to quickly adapt the library to their target distro, or configure their preferred size via config without having to ship different binaries with different image sizes for different distros. It is not meant as a highly exposed feature, but as a small helpful addition for those who care to read the docs.

dhaavi commented 2 years ago

Ah, yes, I've meant the variable to be empty from the get go, but committed the testing value... As stated, the idea is that this is an advanced option - opt-in only.

andydotxyz commented 2 years ago

Thanks for this, very sensible, the "opt in" approach works well.

However exposing a public global like this leads to unavoidable race conditions. Would it be OK to match the other setters (like SetIcon) so we can handle that complexity behind a method call if required?

dhaavi commented 2 years ago

Thinking about this more, I am unsure what the best approach is.

Ideally, icons would not be scaled on every call, but once by the user of the library. So, maybe we could instead provide a utility function that just does the scaling and you would supply the result to SetIcon?

If scaling is only needed once, users could also just do systray.SetIcon(systray.ScaleIcon(iconData, 32)).

andydotxyz commented 2 years ago

That is a good plan. Then again, it is probably easier for the developer to scale in their code because they have the source data. If we do it we have to decode, scale, encode then it can be set. Perhaps it is a documentation issue?

dhaavi commented 2 years ago

Yes, I have now also done it in my code directly. I agree the best way forward is to give an example how to easily scale the images.

You can find an example for scaling a PNG icon here: https://github.com/safing/portmaster-ui/commit/5008392a8f756bf0428a6732144bb07b540d4748

andydotxyz commented 1 year ago

As discussed this is probably a documentation issue, so I have opened #31 in place of this proposed change.