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

Added: Support for coloured vector icons. #1681

Merged
merged 10 commits into from
Jun 25, 2024
Merged

Added: Support for coloured vector icons. #1681

merged 10 commits into from
Jun 25, 2024

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Jun 25, 2024

Fixes #1666

This adds support for coloured icons that can have a ViewBox and thus match design.

Not much more needs to be said.

I also wound up doing some additional research for more efficiently loading vectors in the future; including some alternative file formats, how to integrate, etc. but I decided to relegate carrying out that need into my own personal time; as for the purposes of the project, what we have is sufficiently good enough.


Misc Note: Doing this also exposed to me just how inefficient our use of icons in some regards are.
Give for example Projectanker icons.

These:

  1. Load SVG from embedded resource
  2. Parse it using uncompiled regex
  3. Create StreamGeometry etc. from the path from regex.

And we repeat this for every icon we load, for example, every individual tree row on AdvancedInstaller.

In addition, changing a colour on a Projectanker icon repeats the entire same process, from scratch. oof, when technically one could just change the brush.

In any case, my implementation reuses the data when possible, so the actual icon controls are cheap to create.

@Sewer56 Sewer56 added the Dev UI label Jun 25, 2024
@Sewer56 Sewer56 requested a review from a team June 25, 2024 06:58
@Sewer56 Sewer56 self-assigned this Jun 25, 2024
@Sewer56 Sewer56 marked this pull request as ready for review June 25, 2024 07:19
@erri120 erri120 requested review from erri120 and Al12rs June 25, 2024 07:31
Copy link
Member

@erri120 erri120 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we're doing DIY SVG rendering, the Avalonia Svg control does all of this already. I thought we'd just update our usage of the control with the viewbox and color.

@Sewer56
Copy link
Member Author

Sewer56 commented Jun 25, 2024

I'm not sure why we're doing DIY SVG rendering, the Avalonia Svg control does all of this already. I thought we'd just update our usage of the control with the viewbox and color.

I tried variants of

else if (change.Property == ForegroundProperty && Content is Avalonia.Svg.Skia.Svg svg)
{
    if (change.NewValue is not SolidColorBrush brush) return;
    var css = $".Black {{ fill: {brush.ToHexColour()}; }}";
    // ToHexColour() I made myself as an extension
    Avalonia.Svg.Skia.Svg.SetCss(svg, css);
}

Unfortunately I found that the CSS styling that's part of the Avalonia SVG package applied only to styling classes.

i.e. For every SVG we download, we'd need to manually add a class to each path so we can then style it.
And likewise, strip properties like fill for each path.

Then, at runtime, to change the colour once we got there, we'd convert the brush into hex, make a string out of it. Then the Svg library would pick up that string, have to do a class lookup, create a new brush, etc. It felt very silly and inefficient at that point. If we ever needed to animate with colour transitions; that would also be very far from ideal.

src/NexusMods.Icons/IconValue.cs Outdated Show resolved Hide resolved
src/NexusMods.Icons/UnifiedIcon.cs Outdated Show resolved Hide resolved
src/NexusMods.Icons/SimpleVector/SimpleVectorIconImage.cs Outdated Show resolved Hide resolved
src/NexusMods.Icons/SimpleVector/SimpleVectorIconImage.cs Outdated Show resolved Hide resolved
@erri120 erri120 removed the Dev UI label Jun 25, 2024
Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No extra comments from me.
For development efficiency, it would be nice if we didn't have to cherry pick out parts of the SVG to put in the constructor, and just be able to paste the entire thing. But it is probably not worth the time investment to implement such a thing to save a couple of minutes on every icon add.

@Sewer56 Sewer56 requested a review from erri120 June 25, 2024 09:22
@Sewer56 Sewer56 merged commit ad01530 into main Jun 25, 2024
4 of 11 checks passed
@erri120 erri120 deleted the icon-colouring branch July 1, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Bug: SVG Icons in App Cannot be Recoloured
3 participants