Replace ImageExtensions.Save.tt with Source Generator

[CollinAlpert]

Prerequisites

• [x] I have written a descriptive pull-request title
• [x] I have verified that there are no overlapping pull-requests open
• [x] I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules :cop:.
• [ ] I have provided test coverage for my change (where applicable)

Description

This PR removes the ImageExtensions.Save T4 template and replaces it with a Source Generator. The generator (and all future generators) is placed in the ImageSharp.Generators project, since generators need to be in a separate project than the code they generate for. I have also added <LangVersion>10</LangVersion> to the generator, in order to use features like file-scoped namespaces and such. It is completely safe to use a language version as high as the project which consumes the generator.

Everything still builds and it is still possible to navigate to source for the extension methods the generator replaces. Let me know what you think!

[CLAassistant]

All committers have signed the CLA.

[CollinAlpert]

I see the build is failing due to the fact that the ImageSharp.Generators project does not have a net6.0 target. Where is this limitation coming from? SGs must target netstandard2.0.

[JimBobSquarePants]

Urgh......

So we explicitly tell dotnet build which target framework we want to build against during CI. This lets us identify JIT issues which we have an uncanny ability to uncover.

I wonder why the compiler doesn't multi target?

[CollinAlpert]

I see! I will try to multi target the SG later and see what happens. Should work fine.

[JimBobSquarePants]

I actually don't think it will because our build process isolates each target within any multi target declarations. Not quite sure what to do. 🤔

[CollinAlpert]

That's fine, the generator will build and work correctly under .NET 6 as well. It just won't show up in IntelliSense, which doesn't matter for a CI build. This is why it is discouraged to use a higher version than netstandard2.0.

[Sergio0694]

I don't understand the point of this PR. T4 templates are better suited for this purpose. Source generators are not a replacement for T4 generators and they're not inherently better, they're just meant for different use cases, and this doesn't look like one.

[CollinAlpert]

@Sergio0694 This PR was meant as a demonstration for the ImageSharp maintainers so they could evaluate the maintenance costs of abandoning T4 templates.

[Sergio0694]

Right, but I'm saying there's just no reason to do so at all. T4 templates are the right tool for this. Not to mention, using source generators with a local project reference is unsupported, and has broken tooling. If you clean the repository (eg. git clean -fdx) and open the solution, IntelliSense will be completely broken until you rebuild the source generator and close and reopen VS again. And even without this, using a source generator is just more maintenance effort for literally no gain here.

[antonfirsov]

We should only merge this if we are absolutely sure this is the way forward and we are ready to migrate our entire T4 logic to source generators, especially the massive usage around PixelFormats:

• PixelFormats/PixelImplementations/PixelOperations.Generated<TPixel>.tt
• PixelFormats/PixelImplementations/PixelOperations/Generated

TBH I was always assuming that we should switch to source generators at some point, since it's "the modern tool" for generating more source code based on existing sources, which is how our use case looks like (or should actually look like): generate chore code for all the pixel types in the repo automatically, generate Load/Save overloads based on existing overloads and/or some other hints etc.

they're just meant for different use cases

@Sergio0694 can you elaborate what do you mean by this considering our use-cases?

Not to mention, using source generators with a local project reference is unsupported, and has broken tooling.

I guess ... it's a show stopper for the adaption then? Though I really believed source generators are in better shape already. How do other projects deal with these problems? Or they don't have them?

[JimBobSquarePants]

Very curious to hear more expert opinion here. This is all very new to me!

[Sergio0694]

"can you elaborate what do you mean by this considering our use-cases?"

The advantage of source generators is that they can dynamically generate code based on what code you're using in your own project, but that really only makes sense when there's some kind of introspection being done on said code from the generator end. Say, you're using System.Text.Json, and the generator is inspecting your data models to generate serialization code for you based on those models. In your case instead, there's no introspection being done, and everything is just done in a generator initialization step. That is literally just a T4 template but with extra steps, and it's not really recommended because it just adds way more complexity for no benefit. Not to mention that, currently, there's really no proper tooling support for locally referenced generators, so working on them is a bit of a pain because IntelliSense is pretty much always broken.

The point is: source generators are not meant to replace T4 templates, they're just meant for different scenarios. If you need to generate some fixed templated code, then a T4 template is a perfectly reasonable, and much simpler, choice. Eg. I'm heavily using T4 templates in ComputeSharp too to generate all the HLSL primitive projection types for C#, not generators.

In your case, eg. in those pixel operations files that Anton linked, that setup looks perfectly reasonable. If you wanted to make it more compact, you could just use an array of all pixel format types, and generate all the partial types in a single file 🙂

[antonfirsov]

In your case instead, there's no introspection being done

This is how we implemented stuff with T4, but in theory it would be more robust if we could enumerate & inspect the existing pixel types, and existing Image API-s in the generator instead -- at least this is how I imagined it before @Sergio0694 came to ruin my naïve world 😆

If I get the whole picture right, you basically say: use Source Generator if you really need a reusable tool to generate more code based on existing code in a flexible manner. Use T4 if you want to generate code for a single project you control, otherwise you'll just end up adding even more complexity and maintenance burden.

I think we should stick with T4 then? Or are any counter-opinions in the community which are based on experience with bigger projects?

[CollinAlpert]

Yeah, I definitely agree that SGs are meant for scenarios where more of the code is actually analyzed/introspected to generate the result. I was just under the impression that the tooling around T4 templates (across all operating systems and IDEs) is also not great and that this would cancel out the Source Generators' shortcomings.

[Sergio0694]

"Use T4 if you want to generate code for a single project you control, otherwise you'll just end up adding even more complexity and maintenance burden."

I would say that in some cases it might make sense to still use a source generator for internal use, as long as the benefit is worth the extra work, eg. if you have lots of different uses on random types where the introspection makes sense due to the high variety of scenarios that are supported, but even then it's something you should think about on a case by case basis.

One rule of thumb I can definitely give you though is: if your generator is only using RegisterPostInitializationOutput, then that's really just T4 with extra steps 😄