GetOrCreate allows constructing invalid Win2D effect wrappers

[Sergio0694]

Overview

This is arguably a niche case, but the Win2D ICanvasFactoryNative::GetOrCreate method allows constructing WinRT effect wrappers that are in an invalid state. Specifically, with a realization device set to a D2D device that's incompatible with the wrapped D2D effect. The crux of the issue is that in the CanvasEffect constructor being invoked by the TryCreateEffect factory method, there is no validation logic to ensure that the input CanvasDevice and ID2D1Effect objects could possible be usable together. That is, here:

Win2D/winrt/lib/effects/CanvasEffect.cpp

Lines 32 to 38 in 1848f90

	// Wrapping around an existing D2D resource?
	if (effect)
	{
	auto d2dDevice = As<ICanvasDeviceInternal>(device)->GetD2DDevice();
	m_realizationDevice.Set(d2dDevice.Get(), device);
	}

You can see that if there is a wrapped effect (which will always be the case when this constructor is invoked by TryCreateEffect, Win2D will retrieve the D2D device from the input CanvasDevice, and save that pair as realization device. The problem though is that there's nothing preventing the input D2D effect from having been created from a completely different CanvasDevice object! This paired with the fact that every CanvasDevice object calls CreateD2DFactory upon construction and uses that, means that it's very possible to accidentally construct a WinRT effect wrapper around a device tied to a different factory, which would explode upon drawing 💥

Reproduction

Here's a full minimal repro that illustrates the issue:

// Create CanvasDevice #1 (this will internally create the D2D factory #1)
auto canvasDevice1 = ref new CanvasDevice();

// Create an effect (this will be in the unrealized state, not tied to any device)
auto colorEffect1 = ref new ColorSourceEffect();

ComPtr<ID2D1Image> imageAbi1;

// Realize the effect on CanvasDevice #1. This will cause the effect to be realized, and to return the D2D effect
// with the same D2D factory #1 as parent as the CanvasDevice (and hence D2D device) it was created from.
ThrowIfFailed(As<ABI::Microsoft::Graphics::Canvas::ICanvasResourceWrapperNative>(colorEffect1)->GetNativeResource(
    As<ABI::Microsoft::Graphics::Canvas::ICanvasDevice>(canvasDevice1).Get(),
    0,
    IID_PPV_ARGS(&imageAbi1)));

// Create CanvasDevice #2 (this will internally use a different D2D factory #2)
auto canvasDevice2 = ref new CanvasDevice();

ComPtr<ID2D1Image> imageAbi2;

// Realize the effect on CanvasDevice #2. From GetD2DImage, this effect will see the realization device does not
// match, so it will unrealize and re-realize on the new device. That is, the returned D2D image will now be tied
// to the new D2D device retrieved from CanvasDevice #2, and as such it will be parented to D2D factory #2.
ThrowIfFailed(As<ABI::Microsoft::Graphics::Canvas::ICanvasResourceWrapperNative>(colorEffect1)->GetNativeResource(
    As<ABI::Microsoft::Graphics::Canvas::ICanvasDevice>(canvasDevice2).Get(),
    0,
    IID_PPV_ARGS(&imageAbi2)));

// At this point: the D2D effect retrieved from the color effect is "orphaned". That is, it no longer has an associated
// wrapper, as its parent Win2D effect has unrealized (ie. discarded it) and re-realized on another device. We can now
// call GetOrCreate to ask Win2D to create a new wrapper for this D2D effect then. Here's the issue:
//   - We're passing CanvasDevice #2 as the realization device (tied to D2D factory #2)
//   - We're wrapping the D2D effect from the effect created from CanvasDevice #1
// Win2D doesn't validate this, so we get back an effect in an inconsistent state. It wraps a D2D effect from a given
// device and factory, but at the same time it also thinks it's actually realized on another device and factory.
auto colorEffect2 = GetOrCreate<ColorSourceEffect>(canvasDevice2, imageAbi1.Get());

// Create some drawing session from CanvasDevice #2
auto canvasRenderTarget = ref new CanvasRenderTarget(canvasDevice2, 128, 128, 96);
auto canvasDrawingSession = canvasRenderTarget->CreateDrawingSession();

// Try to draw the effect. From GetD2D image, the effect will run the usual realization logic, but it will see that
// the input device (CanvasDevice #2, from the drawing session) is the same the device it thinks it's realized on.
// So it will skip the unrealization, and will just return the existing wrapped D2D effect, which is invalid 💥
canvasDrawingSession->DrawImage(colorEffect2);

As expected, you get this:

As you can see, the D2D debug layer is triggered and just stops this. In Release, or if you just disabled the debug layer, you get 0x88990012, ie. D2DERR_WRONG_FACTORY: Objects used together must be created from the same factory instance..

Of course, for the same reason, if you actually try creating a render target and drawing session from canvasDevice2 in the example above, that works fine (because the effect will see the device doesn't match and unrealize and re-realize). Of course this makes no sense though, because it means the only case where this ends up working is if you try drawing the effect you just asked Win2D to wrap around a given CanvasDevice, on a different CanvasDevice. Not to mention this also just ends up discarding that D2D effect entirely.

Proposed solution

The solution here would be to add the missing checks in that CanvasEffect constructor, so that if you do accidentally pass an invalid device/effect pair, you get the failure immediately upon constructing the wrapper, rather that at some unspecified point later on when you're trying to draw. This should also make investigating errors in this area simpler.

What we can do is to get the ID2DFactory from the input effect and the input device, and check that they're the same. This would be sufficient to avoid this issue in the general case of invalid factories, which is 100% not valid and won't work (as shown in the example).

This doesn't technically make invalid states here completely impossible, as there's technically a chance that you could have a device that has the same factory as an effect, but the device is actually wrapping a different DXGI adapter, which would cause issues (using an effect created from a D2D device tied to a different DXGI device can work, if the underlying adapter is the same and it was created from the same D2D factory). But, there's no D2D API to check if this is the case (and there's no way to get the D2D device from a D2D effect, so we can't check this ourselves either), so the best we can do for this is just to add documentation and a warning.

[Sergio0694]

This has been fixed in the internal repo, fix will be included in the next preview for both UWP and WASDK 🙂