Interpolation modes
Opened this issue · 5 comments
Direct2D 1.0 and 1.1 use different enums for image interpolation modes. See http://msdn.microsoft.com/en-us/library/windows/desktop/hh447004%28v=vs.85%29.aspx for 1.1 and http://msdn.microsoft.com/en-us/library/windows/desktop/dd368073%28v=vs.85%29.aspx for 1.0
The problem is, none of them perfectly match wxInterpolationQuality.
The current conversions can be found here: https://github.com/alexpana/wxWidgets/blob/SOC2014_DIRECT2D/src/msw/graphicsd2d.cpp#L656
Should they be kept as they are, or should we ignore values which don't have corresponding wxInterpolationQuality entry and return false for http://docs.wxwidgets.org/3.0/classwx_graphics_context.html#af6ecb449cb732632a8d9b5c285d01b91 ?
No, we shouldn't return false (OTOH there should be a wxFAIL_MSG("unknown value") after the switch). But, as usual, I think there is confusion between compile and run time checks here: the fact that we are being compiled with D2D 1.1 header does not guarantee that the program will run on a machine with D2D 1.1 support, so setting the interpolation quality to wxINTERPOLATION_BEST may well fail to work entirely instead of at least using D2D1_INTERPOLATION_MODE_DEFINITION_LINEAR. Or is this code only used with 1.1 anyhow?
Or is this code only used with 1.1 anyhow?
I have recently added a comment which documents the fact that these values are only used with 1.1 device contexts.
(OTOH there should be a wxFAIL_MSG("unknown value") after the switch).
I see. This is also mentioned in the coding guidelines. Unfortunately, the two versions are not totally equivalent. This is first caught by the compiler complaining "not all control paths return a value". Indeed, if one was to use a wrong static_cast, the second version would return some junk value.
I think there is some misunderstanding. You should keep both the switch and the return statement, but just add wxFAIL between them. This will take care -- at run-time, and not compile-time, but better late than never -- of compilers that don't check switch exhaustiveness (such as MSVC...).
The example provided in the coding guidelines did not have a "default" return statement, and that's what I used as a reference. I'll make sure to change every similar function.