question

TonyKalf-0700 avatar image
0 Votes"
TonyKalf-0700 asked TonyKalf-0700 edited

Wrong method sequence in ID2D1SvgElement API.

Hello,
As we are working on this API, we noticed that the implemented methods ID2D1SvgElement API (d2d1svg.h) are in the wrong sequence.
The right sequence of the methods, leaving out the comment and inline methods should be:

     interface DX_DECLARE_INTERFACE("ac7b67a6-183e-49c1-a823-0ebe40b0db29") ID2D1SvgElement  : public ID2D1Resource
     {
            
         STDMETHOD_(void, GetDocument)(
             _Outptr_result_maybenull_ ID2D1SvgDocument **document 
             ) PURE;
            
         STDMETHOD(GetTagName)(
             _Out_writes_(nameCount) PWSTR name,
             UINT32 nameCount 
             ) PURE;
            
         STDMETHOD_(UINT32, GetTagNameLength)(
             ) PURE;
            
         STDMETHOD_(BOOL, IsTextContent)(
             ) PURE;
            
         STDMETHOD_(void, GetParent)(
             _Outptr_result_maybenull_ ID2D1SvgElement **parent 
             ) PURE;
            
         STDMETHOD_(BOOL, HasChildren)(
             ) PURE;
            
         STDMETHOD_(void, GetFirstChild)(
             _Outptr_result_maybenull_ ID2D1SvgElement **child 
             ) PURE;
            
         STDMETHOD_(void, GetLastChild)(
             _Outptr_result_maybenull_ ID2D1SvgElement **child 
             ) PURE;
            
         STDMETHOD(GetPreviousChild)(
             _In_ ID2D1SvgElement *referenceChild,
             _COM_Outptr_result_maybenull_ ID2D1SvgElement **previousChild 
             ) PURE;
            
         STDMETHOD(GetNextChild)(
             _In_ ID2D1SvgElement *referenceChild,
             _COM_Outptr_result_maybenull_ ID2D1SvgElement **nextChild 
             ) PURE;
            
         STDMETHOD(InsertChildBefore)(
             _In_ ID2D1SvgElement *newChild,
             _In_opt_ ID2D1SvgElement *referenceChild = NULL 
             ) PURE;
            
        STDMETHOD(AppendChild)(
             _In_ ID2D1SvgElement *newChild 
             ) PURE;
            
         STDMETHOD(ReplaceChild)(
             _In_ ID2D1SvgElement *newChild,
             _In_ ID2D1SvgElement *oldChild 
             ) PURE;
            
         STDMETHOD(RemoveChild)(
             _In_ ID2D1SvgElement *oldChild 
             ) PURE;
            
         STDMETHOD(CreateChild)(
             _In_ PCWSTR tagName,
             _COM_Outptr_ ID2D1SvgElement **newChild 
             ) PURE;
            
         STDMETHOD_(BOOL, IsAttributeSpecified)(
             _In_ PCWSTR name,
             _Out_opt_ BOOL *inherited = NULL 
             ) PURE;
            
         STDMETHOD_(UINT32, GetSpecifiedAttributeCount)(
             ) PURE;
            
         STDMETHOD(GetSpecifiedAttributeName)(
             UINT32 index,
             _Out_writes_(nameCount) PWSTR name,
             UINT32 nameCount,
             _Out_opt_ BOOL *inherited = NULL 
             ) PURE;
            
         STDMETHOD(GetSpecifiedAttributeNameLength)(
             UINT32 index,
             _Out_ UINT32 *nameLength,
             _Out_opt_ BOOL *inherited = NULL 
             ) PURE;
            
         STDMETHOD(RemoveAttribute)(
             _In_ PCWSTR name 
             ) PURE;
            
         STDMETHOD(SetTextValue)(
             _In_reads_(nameCount) CONST WCHAR *name,
             UINT32 nameCount 
             ) PURE;
            
         STDMETHOD(GetTextValue)(
             _Out_writes_(nameCount) PWSTR name,
             UINT32 nameCount 
             ) PURE;
            
         STDMETHOD_(UINT32, GetTextValueLength)(
             ) PURE;
                
         /* this section should be in this sequence /*
            
         STDMETHOD(SetAttributeValue)(
             _In_ PCWSTR name,
             _In_ ID2D1SvgAttribute *value 
             ) PURE;
            
         STDMETHOD(SetAttributeValue)(
             _In_ PCWSTR name,
             D2D1_SVG_ATTRIBUTE_POD_TYPE type,
             _In_reads_bytes_(valueSizeInBytes) CONST void *value,
             UINT32 valueSizeInBytes 
             ) PURE;
            
         STDMETHOD(SetAttributeValue)(
             _In_ PCWSTR name,
             D2D1_SVG_ATTRIBUTE_STRING_TYPE type,
             _In_ PCWSTR value 
             ) PURE;
            
         STDMETHOD(GetAttributeValue)(
             _In_ PCWSTR name,
             _In_ REFIID riid,
             _COM_Outptr_result_maybenull_ void **value 
             ) PURE;
            
         STDMETHOD(GetAttributeValue)(
             _In_ PCWSTR name,
             D2D1_SVG_ATTRIBUTE_POD_TYPE type,
             _Out_writes_bytes_(valueSizeInBytes) void *value,
             UINT32 valueSizeInBytes 
             ) PURE;
            
         STDMETHOD(GetAttributeValue)(
             _In_ PCWSTR name,
             D2D1_SVG_ATTRIBUTE_STRING_TYPE type,
             _Out_writes_(valueCount) PWSTR value,
             UINT32 valueCount 
             ) PURE;
            
         STDMETHOD(GetAttributeValueLength)(
             _In_ PCWSTR name,
             D2D1_SVG_ATTRIBUTE_STRING_TYPE type,
             _Out_ UINT32 *valueLength 
             ) PURE;
                
     }; // interface ID2D1SvgElement

windows-api-general
· 9
5 |1600 characters needed characters left characters exceeded

Up to 10 attachments (including images) can be used with a maximum of 3.0 MiB each and 30.0 MiB total.

Your presented sequence are different with the official source code's (D2d1svg.h ). But I am not sure why the official sequence is wrong sequence. Could you provide more information about your concern?

0 Votes 0 ·

@RitaHan-MSFT

We checked against (32bit) D2D1.dll version 10.0.18362.900 date june 15 2020.

 .text:10019144 offset D2DSvgElement::SetAttributeValue(ushort const *,ID2D1SvgAttribute *)
 .text:10019148 offset D2DSvgElement::SetAttributeValue(ushort const *,D2D1_SVG_ATTRIBUTE_POD_TYPE,void const *,uint)
 .text:1001914C offset D2DSvgElement::SetAttributeValue(ushort const *,D2D1_SVG_ATTRIBUTE_STRING_TYPE,ushort const *)
 .text:10019150 offset D2DSvgElement::GetAttributeValue(ushort const *,_GUID const &,void * *)
 .text:10019154 offset D2DSvgElement::GetAttributeValue(ushort const *,D2D1_SVG_ATTRIBUTE_POD_TYPE,void *,uint)
 .text:10019158 offset D2DSvgElement::GetAttributeValue(ushort const *,D2D1_SVG_ATTRIBUTE_STRING_TYPE,ushort *,uint)

Further you can read about the investigation here and here on Github



0 Votes 0 ·

I'll consult the related engineer for helping on this issue. And update here if there is any progress.

1 Vote 1 ·
Show more comments
FeiXue-MSFT avatar image
0 Votes"
FeiXue-MSFT answered FeiXue-MSFT commented

Here is the code:


   hr = element->GetAttributeValue(L"fill", &paint);
         //DX::ThrowIfFailed(element->GetAttributeValue(L"fill", &paint));
      /*   element->GetAttributeValue(L"fill", D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR, &color, sizeof(D2D1_COLOR_F));
         element->SetAttributeValue(L"fill", D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR, &newColor, sizeof(D2D1_COLOR_F));
         element->GetAttributeValue(L"fill", D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR, &afterSetColor, sizeof(D2D1_COLOR_F));*/
    
         if (SUCCEEDED(hr))
         {
             hr = element->SetAttributeValue(L"fill", D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR, &newColor, sizeof(D2D1_COLOR_F));
         }
    
         if (SUCCEEDED(hr))
         {
             OutputDebugStringA("SetAttributeValue OK.\n");
    
             hr = element->GetAttributeValue(L"fill", D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR, &afterSetColor, sizeof(D2D1_COLOR_F));
         }
         else
         {
             OutputDebugStringA("SetAttributeValue failed.\n");
         }
    
         if (SUCCEEDED(hr))
         {
             OutputDebugStringA("GetAttributeValue OK.\n");
         }
         else
             OutputDebugStringA("GetAttributeValue failed.\n");
    
            
         if (FAILED(hr))
         {
             //wprintf_s(L"Failed. 0x%X error.\n", hr);
             DX::ThrowIfFailed(hr);
         }
· 3
5 |1600 characters needed characters left characters exceeded

Up to 10 attachments (including images) can be used with a maximum of 3.0 MiB each and 30.0 MiB total.

Thank you very much!

The code is doing what it should do.
So, this excludes an error in header file.

Thank you, I should send you some flowers!

0 Votes 0 ·

@FeiXue-MSFT

Ok, I tested this in Delphi and VS
With the corrected sequence (see VTable image), the Delphi compiler will not raise an exception and works as expected.
An error will not be raised if GetAttributeValue is called, before the SetAttributeValue method, while the VS compiler does.
Throwing an exception in C++ or in Delphi should not be possible, because an interface implemented in a DLL should not behave like that, instead it will return a HResult value.

So, I still believe something is wrong with this interface declaration in the headerfile.

I tested on other interfaces with overloads, and they work fine.

Regards, Tony.


0 Votes 0 ·

@TonyKalf-0700, Thanks for the detailed information. However, due to the issue is complex, I suggest you contacting Microsoft support to raise an incident so that our engineer could work closely with him to identify the root cause and resolve this issue as soon as possible.
If the support engineer determines that the issue is the result of a bug the service request will be a no-charge case and you won't be charged.
Please visit the below link, and click 'Contact us' then choose Windows desktop application development to submit an incident.
https://developer.microsoft.com/en-us/windows/support/

0 Votes 0 ·
TonyKalf-0700 avatar image
0 Votes"
TonyKalf-0700 answered TonyKalf-0700 edited

@FeiXue-MSFT
Because a reply on your reply doesn't work anymore on this site, I'll reply this way:

It would be handy to add code the appropiate way like putting it in a codeblock with comments, instead of attaching a screenshot with code that costs me time to figure out from where this code is originated.
I'll test your suggestion asap.
Never the less, the interface vtable does not corresponds with the API header.
As agreed more than a decade ago Microsoft headerfiles should be compatible with other languages that includes correct interface method sequences (vtables), method naming conventions and naming parameters or offer alternatives for semantics.

Thanks, Tony.

UPDATE

I tested D2DSvgImage, and not surprisingly after 2 times calling GetAttributeValue and SetAttributeValue the debugger throws this exception in VS2019:

Exception thrown at 0x00007FFFC866A719 in D2DSvgImage.exe: Microsoft C++ exception: Platform::InvalidArgumentException ^ at memory location 0x000000C759FFCA08. HRESULT:0x80070057 The parameter is incorrect.

WinRT information: The parameter is incorrect.

This is exactly what Delphi is returning when executing this code.

I altered the code a bit to check return values:

 void D2DSvgImageRenderer::RecolorSubtree(ID2D1SvgElement* element, D2D1_COLOR_F newColor)
 {
     // Check if this SVG element has a "fill" element explicitly specified or inherited.
     if (element->IsAttributeSpecified(L"fill"))
     {
         // Retrieve the value of this element's "fill" attribute, as a paint object.
         ComPtr<ID2D1SvgPaint> paint;
           
         HRESULT hr = S_OK;
    
         D2D1_COLOR_F color;
         D2D1_COLOR_F newColor;
         D2D1_COLOR_F afterSetColor;
    
         newColor.r = 50;
         newColor.g = 100;
    
         hr = element->GetAttributeValue(L"fill", &paint);        
            
         //DX::ThrowIfFailed(element->GetAttributeValue(L"fill", &paint));
           
         if (SUCCEEDED(hr))
         {
             hr = element->GetAttributeValue(L"fill", D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR, &color, sizeof(D2D1_COLOR_F));
             if (SUCCEEDED(hr))
             {
                 hr = element->SetAttributeValue(L"fill", D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR, &newColor, sizeof(D2D1_COLOR_F));
             }
    
             if (SUCCEEDED(hr))
             {
                 hr = element->GetAttributeValue(L"fill", D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR, &afterSetColor, sizeof(D2D1_COLOR_F));
             }
         }
    
         if (FAILED(hr))
         {
             //wprintf_s(L"Failed. 0x%X error.\n", hr);
             DX::ThrowIfFailed(hr);
         }
    
    
         // Check the type of paint object that was set. There are different types of
         // paints, such as plain colors, URLs, or the 'none' type. For this app we
         // only want to modify fills that were set to a specific color.
         D2D1_SVG_PAINT_TYPE paintType = paint->GetPaintType();

17660-ss-codesample-001.jpg



ss-codesample-001.jpg (184.8 KiB)
· 2
5 |1600 characters needed characters left characters exceeded

Up to 10 attachments (including images) can be used with a maximum of 3.0 MiB each and 30.0 MiB total.

@TonyKalf-0700, Thanks for this detailed info. There are some element's fill value are none in the drawing.svg. So this exception expected when you try to get its value to D2D1_SVG_ATTRIBUTE_POD_TYPE_COLOR. You can modify the code, and make the SetAttributeValue function call before GetAttributeValue, then the exception will disappear. Here is the code for your reference(due to the length limitation, try to post another post for the code, and need some time for moderator approving), and please feel free to let me know if still have any concern.



0 Votes 0 ·

@FeiXue-MSFT

What about this?

19178-vtable-dump-svg-d2d1dll-64bit.jpg

D2D1.dll (64bit)

For what I see is a sequence that matches Delphi - but not Msvc, that relies on the original method sequence in d2d1svg.h -. However, when looking at the details I see (not in this screenshot) mixed compiler directives (__stdcall and __fastcall) Now that is a bit weird. For what I learned, this is not a good practice. Especially concerning winapi calls that should be stdcall. Now, I know the Delphi compiler has been changed to a modified opensource CLang compiler.
And Microsoft is intending to support this compiler too, but, not at the time this header was written and D2D1.dll was compiled I think.
So the conclusion is: The MS headerfile works for Msvc but not for Delphi.
Delphi works when the Vtable as shown sequenced will be altered to the sequence ID2D1SvgElement following the D2D1.dll (See screenshot).

0 Votes 0 ·
TonyKalf-0700 avatar image
0 Votes"
TonyKalf-0700 answered

So the end conclusion is:

The definition of the ID2D1SvgElement API (d2d1svg.h) is in wrong sequence.
The sample to check this, presented in this conversation, shows the same flaw when running the sample multiple times where it throws an exception instead of returning a HResult.
The flaw still exists in SDK 10.0.22000.0
Thank you.

5 |1600 characters needed characters left characters exceeded

Up to 10 attachments (including images) can be used with a maximum of 3.0 MiB each and 30.0 MiB total.