feat(unity): Add support packing multiple type sprite

[Creta5164]

This PR improves builder in Unity package that makes to supporting Multiple type Sprite Mode in Texture import settings.

This is an improvement to address the current Unity implementation, which treats all Sprites as a single texture and therefore cannot individually pack Sprite resources if Texture's Sprite Mode type is Multiple.

Based on the issue discussion, this should be able to be done by default without any options, so I'm making sure that packing happens naturally internally without requiring any options.

Fixes #47

[Creta5164]

Hey @elringus, looks like test requires update license, can you take a look about it?

[Creta5164]

Performace regression

Looks like this implementation kinda slow due change from array to list, I'll do fix it for performance.

Edit: Maybe not, I need to investigate about this later.

Edit: I tried to reduce EnsureReadable in SourceLoader (https://github.com/elringus/sprite-dicing/pull/48/commits/50a4360cdf03ab52ed73f5c4cc1390189747d16a) but it seems still slow for huge amount of framed sprites.

Edit: I tried to preflight texture's color data before building native texture data(https://github.com/elringus/sprite-dicing/pull/48/commits/cab7b2652b461bccee246c397651f32f5d32b844), it's now considerably faster then previous implementations before delivering sprites data to native area! It was something I could have easily missed, previously it spent 25 minutes long for it and it now finishes under 10 minutes!

[Creta5164]

Exception regression

I fixed it from IEnumerable to IReadOnlyList of AtlasBuilder.CollectSourceSprites due lost pointer of SerializedObject while packing process, it occured when packing triggers reimport assets.

Edit: Hmm, sometimes it throws exception in UpdateCompressionRatio too due lost of SerializedProperty.

This is most likely caused by the asset importer. Please hold off on merging this PR as I need to fix this issue before it gets merged.

[elringus]

The failing workloads should now be fixed on main. Please rebase the PR.

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 100.00%. Comparing base (bdc7861) to head (fb2038f). :warning: Report is 4 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main       #48      +/-   ##
===========================================
+ Coverage   98.73%   100.00%   +1.26%     
===========================================
  Files          19        18       -1     
  Lines         869       878       +9     
===========================================
+ Hits          858       878      +20     
+ Misses         11         0      -11     

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

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Creta5164]

I made this PR turn into draft due needs more fixing regressions.

[Creta5164]

Hey @elringus, since it's been 2 weeks, I'm pretty busy right now but I think I can take a look into in this weekend.

[Creta5164]

Guess since AtlasBuilder's SO is partially not managed area. I have to cache setting values (managed types) before starting build.

[Creta5164]

Hey @elringus It's been late from last change.

• Fixed generating sprites that has invalid file name characters (i.e. /, &, etc) while build, it'll replace to underscore. (_)
• I improved performance about generating huge amount of sprite assets, now it'll takes 7~8 minutes around to finish with 13k sprites, previously it takes over 30 minutes.

Also can you give some opinion about my last comment when you have some spare time?

[elringus]

Hey @elringus It's been late from last change.

* Fixed generating sprites that has invalid file name characters (i.e. `/`, `&`, etc) while build, it'll replace to underscore. (`_`)

* I improved performance about generating huge amount of sprite assets, now it'll takes 7~8 minutes around to finish with 13k sprites, previously it takes over 30 minutes.

Also can you give some opinion about my last comment when you have some spare time?

Hi, thanks for the improvements! I'm not sure I understand the part about the builder SO — could you please elaborate a bit?

[Creta5164]

I'm not sure I understand the part about the builder SO — could you please elaborate a bit?

@elringus Since EditorProperties does redirects it's SO's properties, this causes lost native reference while building atlas because importers are triggered to reimport them in specific condition. So I thinking about that these values should stored into managed area's memory while Unity working it's job.

[elringus]

Still not sure I get it. The editor props are atlas asset props – why would they invalidate during the build? That could only happen if the atlas asset is destroyed, which shouldn't happen during the build.

Let's keep this PR focused on adding support for the multiple sprite mode. Please create dedicated issues for other topics so we can discuss them in isolation.

[Creta5164]

@elringus I think this pr is ready to review, still there's room for improvement of performances (loading from texture) but I'll leave them for now due schedule issue.

[elringus]

It seems the editor tests are failing, likely because some class signatures have changed. We also need to add tests for the new mode to ensure full coverage.

I can either keep the PR open or merge into a branch and finish it when I have spare time.

[Creta5164]

@elringus I see, I'll fix these signatures problem of test codes and adding new multiple type sprite sheets too, when I have some spare time nether...

[Creta5164]

I figured out why I've didn't noticed test codes, it's outside of package, gonna try some cover up these test code.

[Creta5164]

Looks like pretty long way to go for resolving these test codes...

[elringus]

Yeah, the Unity tests are quite fragile. This is a legacy from v1, where everything was implemented in C#, and such granular tests were justified, but that's no longer the case. Ideally, we'd make the Unity tests more integration/black-box. Will look into this when I have time. Thank you for the contribution! I'll handle the rest of this.

[Creta5164]

@elringus Hey but I can send to you my changes at least like below, I'll commit this if you okay to take cover with it.

And also it seems we should take these test code into package area too, I've been working on it with symlink directory so I have to manually move them into package area for testing with it.

[elringus]

Sure, you can continue committing to this PR.

[Creta5164]

@elringus Okay, here you go! You can probably try below git command to continue modify on my branch.

git remote add cretapark https://github.com/Creta5164/sprite-dicing.git

I've been turned on Allow edits by maintainer so you can directly push my branch at here.

[Creta5164]

Oh guess I have to rebase first

[Creta5164]

@elringus Okay so hope you like it, and want to say thanks for making this package!

I can able to reduce 1.22GB memory usage of our game with this package, thank you.