Skip to content

[feat (issue #6389)] make tauri icon support SVGs#6444

Merged
lucasfernog merged 13 commits intotauri-apps:devfrom
fetzsav:patch-1
Nov 7, 2023
Merged

[feat (issue #6389)] make tauri icon support SVGs#6444
lucasfernog merged 13 commits intotauri-apps:devfrom
fetzsav:patch-1

Conversation

@fetzsav
Copy link
Copy Markdown
Contributor

@fetzsav fetzsav commented Mar 14, 2023

What kind of change does this PR introduce?

  • Bugfix
  • [ x] Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • [x ] No

Checklist

  • [x ] When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • [x ] A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • [ x] I have added a convincing reason for adding this feature, if necessary

Other information

This allows .SVG files to be passed into the CLI's icon module. This was a request in the issues tab

@fetzsav fetzsav requested a review from a team as a code owner March 14, 2023 00:19
@fetzsav fetzsav changed the title Update icon.rs [feat] make tauri icon support SVGs Mar 14, 2023
@fetzsav fetzsav changed the title [feat] make tauri icon support SVGs [feat (issue #6389)] make tauri icon support SVGs Mar 14, 2023
@lorenzolewis
Copy link
Copy Markdown
Member

lorenzolewis commented Mar 14, 2023

This is an awesome addition! @amrbashir maybe this is something we could use downstream in CTA later as well to reduce the size even more?

Edit: Would require a few more changes to the actual core bits of Tauri that pull in the rendered icons I realise, but still cool to move closer to that possibility!

@amrbashir
Copy link
Copy Markdown
Member

amrbashir commented Mar 14, 2023

Cool! If this ends up getting merged, and tauri changes its logic to depend on a single svg icon and generate the others on the fly, it will save us a couple of hunderd kilobytes

@fetzsav
Copy link
Copy Markdown
Contributor Author

fetzsav commented Mar 14, 2023

As per discussion in the Discord, there are a couple points worth checking before merge.

  1. Resizing of the SVG. Currently, we are not utilizing the SVG, as we are converting it to rasterized and then performing the image resize. I’ll look into this when I get home today.

  2. Extra C dependency from nsvg package. Per Discord discussion, resvg was suggested. Just let me know if you guys would prefer to use a different crate.

In response to the previous comments… are you guys talking about saving extra disk space? So, if an SVG is used, the PNGs arent saved to disk? Just some clarification here would help thanks!

@lorenzolewis
Copy link
Copy Markdown
Member

@fetzsav we were just having a slight sidebar for CTA (create-tauri-app) for a few ideas of how this could help get us closer to shrinking the templates down there in the future. You'd still need to generate the raster images to use in icons, this PR just helps us get closer to using SVGs to generate those raster images and then in theory in the future we could use that functionality to optimise CTA.

tl;dr no impact to this, just some enhancements we could get closer to downstream with this PR

@lucasfernog
Copy link
Copy Markdown
Member

I think this feature should target the v2 release, v1.3 is pretty much ready and the idea is to focus 100% on v2 when that release is out.

@lucasfernog
Copy link
Copy Markdown
Member

I tried to use resvg but couldn't get the resize to work. Can you review @FabianLars ?

@FabianLars
Copy link
Copy Markdown
Member

I switched back to resvg because nsvg failed to resize my very first test icon 😂 (it worked with other test files, resvg could handle all i tested). Resizing should work, but i know what issue you were talking about.

Also the ios bg color is now applied after resizing to make sure it's the svg renderer resizing it and not image-rs. Seemed to work on my end but i have no ios setup to test this rn.

FabianLars
FabianLars previously approved these changes Nov 6, 2023
@lucasfernog lucasfernog merged commit 50f7ccb into tauri-apps:dev Nov 7, 2023
size: u32,
file_path: &Path,
ios_color: Option<Rgba<u8>>,
bg_color: Option<Rgba<u8>>,
Copy link
Copy Markdown
Member

@FabianLars FabianLars Nov 7, 2023

Choose a reason for hiding this comment

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

i swear i named it this way first but reverted it for no reason 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🔎 In audit

Development

Successfully merging this pull request may close these issues.

5 participants