Fix CGlyphRunResource::GetDWriteRenderingMode to choose correct DWriteRenderingMode under ClearType

[rladuca]

Fixes #2025

[vatsan-madhavan]

~Do we still support Windows Server 2008 R2? ~

Never mind - GetRecommendedRenderingMode is supported there anyway.

[rladuca]

Do we still support Windows Server 2008 R2?

Can you explain what that has to do with this change? We will only choose symmetric if DWrite itself chooses symmetric. It's not obvious, but DWRITE_RENDERING_MODE_NATURAL_SYMMETRIC and DWRITE_RENDERING_MODE_CLEARTYPE_NATURAL_SYMMETRIC are the same. As in, they have the exact same value in the enumeration.

[vatsan-madhavan]

Can you explain what that has to do with this change?

I rushed into the question thinking it was new call and may not be supported on some platforms; and then realized it has been used before all along.

[rladuca]

Can you explain what that has to do with this change?

I rushed into the question thinking it was new call and may not be supported on some platforms; and then realized it has been used before all along.

All good, there are other things I am looking into that actually do have a Win8+ requirement. But those would have to be a separate PR and would be much more extensive and invasive. If that actually becomes a thing we need to have a discussion about what we're willing to change in the rendering process to fully support them. I'll file a different issue altogether.

[vatsan-madhavan]

LGTM; low pri - can you add some comments about how a sample app might exercise each of the branches in GetDWriteRenderingMode (including the last dangling else with just an assert)? It would be helpful when writing tests for this, I think (or debugging problems...)

[rladuca]

Sure, I can add that.

[rladuca]

I am going to wait on merging this anyway until I get some more info on how effective this change will be.

[SamBent]

Your code calls DWrite and ignores the result in many cases. I'd prefer only calling DWrite if there's a chance of using the result, to avoid needless perf and risk. It should be easy enough to do, starting with changing old line 970-1 to
if ( (textRenderingMode == Grayscale) ||
((textRenderingMode == ClearType && IsDisplayMeasured()))

[rladuca]

I did a run of all text DRTs and microsuites and everything passed. I'm suspicious that we don't have certain parts of this coverage ported for .NET Core. Before I merge this I am going to make the equivalent changes to .NET Framework and get a full pass done there.

[vatsan-madhavan]

@rladuca changing title to WIP to prevent accidental merge.

[rladuca]

Full test run is back and everything looks good, merging.