timmaffett / material_symbols_icons

Complete Flutter support for google's Material DesignMaterial Symbols Icons
Apache License 2.0
32 stars 5 forks source link

Symbols.text_selection_start icon disappearing in release build #8

Closed timmaffett closed 1 week ago

timmaffett commented 10 months ago

Hi Tim,

I hope you’re well and thank you for https://pub.dev/packages/material_symbols_icons!

We are currently trying it out for our app and everything looks fine in development, but noticed that when we use it in production it doesn’t show the icon.

Development:

image

Production:

image

Notice the first icon which uses the library has an error. Not sure what’s wrong as it’s a simple Icon(Symbols.text_selected) statement. The missing icon here look like this https://fonts.google.com/icons?selected=Material+Symbols+Outlined:text_select_start. Any ideas?

Thank you, Rach

timmaffett commented 10 months ago

Hi Rach,    I have created an official issue to track this here: https://github.com/timmaffett/material_symbols_icons/issues/8     Sorry for the delay in getting back to you, it was our independence day holiday here in the states.      Now this is a very puzzling problem that you are having - I have not seen any issue with icons disappearing from release builds.   Are all of the icons in the example you showed from the Symbols.XXXX material symbols icons library ?  (I am assuming so).   The first thing that I would try to narrow things down is try using the rounded or sharp variations.   Also in your email you mention it being Symbols.text_selected but it is the Symbols.text_select_start icon.  Was that a typo in the email ?   So perhaps try changing the symbol name to Symbols.text_select_start_sharp and see what happens, or Symbols.text_select_start_sharp. Here is the link to it in the demo web flutter icon previewer app.  This link shows all three variations.     https://timmaffett.github.io/material_symbols_icons/?query=text_select_start&iconSize=48&fontType=universal&fill=0&weight=400&grade=0&opticalSize=48

BTW, When using this preview app you can click on any icon to have its symbol name copied to the clipboard.      Any more information you can provide would be helpful.  One possible cause for what you are seeing is that the flutter icon tree shaking is not identifying that you are using the Symbols.text_select_start icon and then it removes it from the font during tree shaking.  I would suspect that this must be the cause but I have no idea how the flutter tree shaking is not detecting it's use..   If you could report what happens when you change to the rounded or sharp variation of that icon this would be helpful. (My gut feeling is that they should also appear missing in the --release version, because it must be something is 'hiding' the location of this symbols use from the flutter icon tree shaker, so it shakes the icon from the font).      What would help the most is if I could see an excerpt of the code where the icons are being used to make that toolbar.   (You could send code excerpts to my personal email, but other than that it may be useful to care on this conversation in the issue I linked to at the start) Thanks,   Tim

timmaffett commented 10 months ago

... and I just discovered that https://github.com/flutter/engine/pull/42598 has broken the link:

https://timmaffett.github.io/material_symbols_icons/?query=text_select_start&iconSize=48&fontType=universal&fill=0&weight=400&grade=0&opticalSize=48

from working correctly because the engine is stripping the query URL..

I will have to get this fixed.

r-ilagan commented 10 months ago

Hi Tim,

Thanks for getting back to me and no worries, I forgot it was July 4th.

To your questions: Yes, it was a typo, sorry. I meant to say Symbols.text_select_start. The other symbols below the broken one are from the Icons class. I'm assuming that it might be related to this problem of tree shaking you submitted a fix for a couple of months ago #125704. I've asked my team lead if we could have the next build have the --no-tree-shake-icons parameter and see if that fixes it. I'll also try and see if the other variations will result in having missing icons.

I'm following the fix engine#41592 now as well and hopefully it will be included in a minor version update soon.

Appreciate all the work you're doing, btw!

r-ilagan commented 10 months ago

I tried it on release mode and it renders for me (Symbol.text_select_start). This was with tree shaking as well. Hmm, not sure what's going on now

timmaffett commented 10 months ago

Hi Rach, I am confident that turning off tree shaking should resolve the problem, but there will be around a 18MB increase in build size.

It is possible that #125704 / engine/#41592 is causing the problem, but that primarily caused problems with font variation features like fill, weight, grade and optical size. If you are not using font variations for the icons then I would not expect this to cause a problem.

Another possible solution is to do the tree shaking on a master branch build and substitute the resulting material symbols icons fonts into your stable built release build. This allows you the space savings of the tree shaking while still building everything in the stable branch. Unfortunately without tree shaking the fonts are enormous, so the savings is something around 18MB down to a few thousand bytes.

---Oh, just saw your new email. I think perhaps the previous problem was caused by the tree shaker not seeing that symbol use somehow, or a corruption of the output font. I does not surprise me that it is working now, as that is what I would expect. Keep me posted if the problem pops up again, always happy to help!

-tim

On Wed, Jul 5, 2023 at 10:17 AM Rach Ilagan @.***> wrote:

Hi Tim,

Thanks for getting back to me and no worries, I forgot it was July 4th.

To your questions: Yes, it was a typo, sorry. I meant to say Symbols.text_select_start. The other symbols below the broken one are from the Icons class. I'm assuming that it might be related to this problem of tree shaking you submitted a fix for a couple of months ago #125704 https://github.com/flutter/flutter/issues/125704. I've asked my team lead if we could have the next build have the --no-tree-shake-icons parameter and see if that fixes it. I'll also try and see if the other variations will result in having missing icons.

I'm following the fix engine#41592 https://github.com/flutter/engine/pull/41592 now as well and hopefully it will be included in a minor version update soon.

Appreciate all the work you're doing, btw!

— Reply to this email directly, view it on GitHub https://github.com/timmaffett/material_symbols_icons/issues/8#issuecomment-1622172891, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3YUSJSKCGD4VR22SLKBG3XOWOULANCNFSM6AAAAAAZ7G6HZ4 . You are receiving this because you authored the thread.Message ID: @.***>

r-ilagan commented 10 months ago

Hi Tim,

Thank you for the suggestion. Unfortunately, we're unable to do any substitution with our current version control setup -- we use SVN. And turning off tree shaking is out of the question with an 18MB increase for a web app. I think we'll go with manually adding the SVGs as custom fonts for now until the fix arrives.

Thank you again!

timmaffett commented 10 months ago

Hi Rach, I would encourage you to try publishing the release version again and see if you can reproduce the problem with the icon disappearing. Tree shaking should work just fine for you guys if you are not using font variation features of the material symbols icons. Let me know.

-tim

timmaffett commented 1 week ago

I am closing this issue - feel free to re-open it if you see the problem again.