Repository navigation
Improvements to the CppWinrt code generator. - #350
Conversation
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Fix the alphabetization of these. #Closed
|
|
||
| public override string TypeVector4 { get; } = "float4"; | ||
|
|
||
| public override string TypeMatrix3x2 { get; } = "float3x2"; |
There was a problem hiding this comment.
Alphabetize #Closed
|
|
||
| public override string Vector4(Vector4 value) => $"new Vector4({Float(value.X)}, {Float(value.Y)}, {Float(value.Z)}, {Float(value.W)})"; | ||
|
|
||
| public override string CanvasFigureLoop(CanvasFigureLoop value) |
There was a problem hiding this comment.
Alphabetize #Closed
|
|
||
| public override string TypeVector4 { get; } = "float4"; | ||
|
|
||
| public override string TypeMatrix3x2 { get; } = "float3x2"; |
There was a problem hiding this comment.
Alphabetize
#Closed
|
|
||
| namespace Microsoft.Toolkit.Uwp.UI.Lottie.UIData.CodeGen.Cx | ||
| { | ||
| sealed class CppNamespaceListBuilder |
There was a problem hiding this comment.
CppNamespaceListBuilder [](start = 17, length = 23)
This class is identical in both the Cx and Cppwinrt namespaces. I know this PR is about separating the codegen for CX and C++/WinRT, but can this class be shared? #Closed
There was a problem hiding this comment.
It could, but as you say, this PR separates the code generators on purpose. Any shared behavior needs to be tested on both CX and cppwinrt, which doubles the maintenance cost for any shared code.
My dream is that one day the CX code goes away completely anyway.
In reply to: 489611120 [](ancestors = 489611120)
| #if PUBLIC_UIDataCodeGen | ||
| public | ||
| #endif | ||
| sealed class CppwinrtInstantiatorGenerator : InstantiatorGeneratorBase |
There was a problem hiding this comment.
CppwinrtInstantiatorGenerator [](start = 17, length = 29)
What about this file was modified besides changing directories? It's not easy to tell what was changed from Codeflow, but the file is not shown as just a "moved" file either. #Closed
There was a problem hiding this comment.
Consider it all brand new code. We really didn't support cppwinrt before... it was just some hacks to modify the cx output to do what one particular project needed, but it was never right.
In reply to: 489612853 [](ancestors = 489612853)
|
builder.WriteLine("#include |
||
|
|
||
|
builder.WriteLine("#include |
||
|
builder.WriteLine("#include |
There was a problem hiding this comment.
winrt/Windows.UI.Composition.h [](start = 41, length = 30)
Should it check whether it is WinUi3 before including this file? #Closed
There was a problem hiding this comment.
Good point. BTW, I don't know whether Winui3 works for any of this anyway.
In reply to: 489617240 [](ancestors = 489617240)
| : $"{NormalizedNamespace}.{UnqualifiedName}"; | ||
|
|
||
|
/// |
||
| /// A non-language-specific name that can be used for display |
There was a problem hiding this comment.
name [](start = 36, length = 4)
Nit: "namespace"? #Closed
| builder.WriteLine("#else"); | ||
|
builder.WriteLine("#include |
||
| builder.WriteLine("#endif"); | ||
|
builder.WriteLine("#include |
There was a problem hiding this comment.
[](start = 44, length = 36)
Curious what is the diff between "Windows.Graphics.Effects.Interop.h" and "windows.graphics.effects.interop.h" #Closed
There was a problem hiding this comment.
| protected override void WriteCanvasGeometryCombinationFactory(CodeBuilder builder, CanvasGeometry.Combination obj, string typeName, string fieldName) | ||
| { | ||
| builder.WriteLine($"{typeName} result;"); | ||
| builder.WriteLine("ID2D1Geometry *geoA = nullptr, *geoB = nullptr;"); |
There was a problem hiding this comment.
ID2D1Geometry * [](start = 31, length = 15)
ComPtr like the other ones in this block? #Closed
There was a problem hiding this comment.
This is one of the geometry cases that is not handled yet. Anything using raw raw pointers or ComPtr in the cppwinrt generator is just copied from cx and will be fixed later.
In reply to: 489622758 [](ancestors = 489622758)
| UINT*, | ||
| ::ABI::Windows::Graphics::Effects::GRAPHICS_EFFECT_PROPERTY_MAPPING*) override | ||
| { | ||
| return E_INVALIDARG; |
There was a problem hiding this comment.
E_INVALIDARG [](start = 19, length = 12)
E_NOTIMPL #Closed
There was a problem hiding this comment.
It's implemented. There are no named property mappings, so any name you give us is invalid.
In reply to: 489633599 [](ancestors = 489633599)
|
|
||
| // Write out each namespace using. | ||
| foreach (var n in namespaces) | ||
| // Add fields and proeprty declarations for each of the theme properties. |
There was a problem hiding this comment.
proeprty [](start = 29, length = 9)
"property" #Closed
| } | ||
|
|
||
| builder.Class.Public.WriteLine($"winrt::{_animatedVisualTypeName} TryCreateAnimatedVisual("); | ||
| builder.Class.Public.Indent(); |
There was a problem hiding this comment.
builder.Class.Public.Indent(); [](start = 12, length = 30)
Missing an Unindent() below. #Closed
3 participants
Up until now, the Cppwinrt code generator was a hack on top of the CX code generator that was enough to get certain projects unblocked but was not anything close to a general solution. With this change I am separating the CX and Cppwinrt code generators, removing all of the hacks that switched between CX and cppwinrt, and making the output work with the cppwinrt tools.
As a result, we now output .IDL files for each Lottie file in addition to the .h and .cpp files. The generated code is designed to work with the code generated by cppwinrt.exe from the IDL.
This change does not enable cppwinrt for all Lotties - there is some work still do do with some geometries, and getting this thing tested is going to take a long time, but it is much closer to the dream than what we had before, and fixing it to deal with issues we uncover will be a lot simpler.