[DWrite] Factory2.TranslateColorGlyphRun should handle DWRITE_E_NOCOLOR.

[Jayonas]

The auto-generated implementation of Factory2.TranslateColorGlyphRun understandably uses the default errror handling behavior, which is understandably to wrap the error HRESULT in an exception and throw it. Unfortunately, in this particular case, I think that turns out to be a bad idea and would be worth creating a hand-written implementation for.

The problem is that DWrite returns an "error" code in the case where TranslateColorGlyphRun simply has no work to do because the glyph run in question is just a plain monochrome glyph run rather than a multi-color one. This is a problem because calling TranslateColorGlyphRun and checking for that result is the only way that I could find to even detect color glyph runs.

That means if you want to support color glyph runs you need to call this method every time you draw a glyph run and check for that result. That's fine in the native case, but when that result gets turned into an exception for the managed case, suddenly the typical usage (non-color glyph runs) requires throwing and catching/ignoring an exception every time you draw a glyph run. Drawing things is typically a relatively hot code path, so that exception can easily have a pretty big perf impact.

I suggest that a hand-written implementation of TranslateColorGlyphRun should just return null in the case where the glyph run doesn't use color. I think that makes good sense because the return type is ColorGlyphRunEnumerator, and for non-color glyph runs there is no need for such an instance, so you just get null instead.

(Note that I'm basing this off my usage of SharpDX 3.1.1, which I realize is out of date, but it also doesn't look like much has happened in the DWrite code since then so I think it's likely that this is still an issue.)

[mrvux]

Returning null is not such a good idea (since then we don't know what is the source of the error), instead it's better to modify the code generator to work in a Try API style, as per this issue for example https://github.com/sharpdx/SharpDX/issues/804

So we get a TryTranslateColorGlyphRun which returns a Result struct and has out ColorGlyphRunEnumerator parameter, TranslateColorGlyphRun still performs the same task and returns exception if result is incorrect.

I'll look to add this one soon as well, doesn't take too much time normally.

[Jayonas]

I disagree about it being bad to return null. It would mean exactly one thing: the glyph run isn't color. Any other error could continue to throw an exception as expected. Unless you're talking about callers just not knowing what a null result means? It's true that it wouldn't necessarily be super obvious to callers without documentation, though it seems like a note in the function's doc comments would suffice for that?

Having a Try variant would certainly allow callers to deal with the issue, but I don't think it's quite as elegant since it will require callers to know which error code to check for and manually throw an exception for any other code in the same way that SharpDX would. That said, I do like the idea of Try variants being a general pattern across the API that would allow more raw access to the native library when needed.

On further thought, maybe an ideal situation would be if both suggestions were eventually implemented. I think mine is a better abstraction and fits better with managed paradigms, but yours is part of a general system that could be helpful in many places where better abstractions aren't available/feasible since we're wrapping low-level native libraries, after all.

Anyway, I definitely don't want to get in the way of someone with enough time and familiarity implementing a solution, even if it's not the one I'd pick first. Thanks a lot for taking a look and potentially spending some time on it. :)

[mrvux]

This adds Try support https://github.com/sharpdx/SharpDX/commit/e3889153c411f9f3861bf9831ea9ed57d2efb04b

[Jayonas]

I've recently updated to SharpDX 4.2.0 and was able to remove my workaround for this issue (custom native implementation) in favor of calling your new Try-style API and manually checking the error code. Thanks!