Windows: fewer windows.h includes from platform.h

[aras-p]

Description

Using almost any OIIO header (e.g. imageio.h) includes platform.h, which on Windows includes <windows.h>. That IMHO is "not ideal at all", primarily because windows headers are fairly terrible in what they hijack as preprocessor symbols, that often clash with sensible things used by other code.

The most famous ones being them defining min and max, but there are many others. Now, I tried removing inclusion of <windows.h> from public OIIO headers, and that almost works, except for the usage of YieldProcessor() from an inline function in thread.h, which is hard to remove in a clean way.

So instead of that, here's a much smaller proposal: in addition to defining NOMINMAX (salvages min and max symbols) and WIN32_LEAN_AND_MEAN / VC_EXTRALEAN (salvages small symbol), also define NOGDI, which saves ERROR and GetObject among others.

• ERROR allows code to use both OIIO and google/gtest library. Case in point: Blender, which up until now always used modified OIIO headers (adding NOGDI just like here).
• GetObject is just a possibly common symbol; and having it not hijacked to either GetObjectA or GetObjectW by windows headers sounds useful.

Tests

N/A

Checklist:

• I have read the contribution guidelines.
• If this is more extensive than a small change to existing code, I
  have previously submitted a Contributor License Agreement
  (individual, and if there is any way my
  employers might think my programming belongs to them, then also
  corporate).
• I have updated the documentation, if applicable.
• I have ensured that the change is tested somewhere in the testsuite
  (adding new test cases if necessary).
• My code follows the prevailing code style of this project.

[lgritz]

Seems to me that YieldProcessor (git says that line in the source dates from 10 years ago, before we could count on C++11) could now be replaced by std::this_thread::yield(). Does that really let us remove windows.h from platform.h?

[aras-p]

They are not the same; OIIO already has yield() (give up the rest of my timeslice) and pause() ("I'm inside a busy loop and imma let you know in case you want to adjust power or whatever"), and the confusingly named Windows YieldProcessor is the latter.

YieldProcessor could be "re-implemented" without including the rest of 100 thousand lines of windows headers, something along the lines of (typing out of my head):

#if defined(_M_AMD64)
#    define YieldProcessor _mm_pause
#elif defined(_M_ARM64) || defined(_M_HYBRID_X86_ARM64)
__forceinline void YieldProcessor(void)
{
    __dmb(_ARM64_BARRIER_ISHST);
    __yield();
}
#elif defined(_M_ARM)
__forceinline void YieldProcessor(void)
{
    __dmb(_ARM_BARRIER_ISHST);
    __yield();
}
#else
__forceinline void YieldProcessor(void)
{
    _asm pause
}
#endif

But yes, I think that would allow removal of windows header inclusions from public OIIO headers, and they would only have to be included in a handful of implementation .cpp files. This might be considered a trivial "compilation breaking" change (but not linking/binary ABI change), if someone's code needed windows headers, did not explicitly include them and only got them indirectly via OIIO.

[lgritz]

In theory, API backwards compatibility is supposed to break only for major releases (2.x -> 3.0, but not 2.4 -> 2.5). But this is a grey area, and in similar situations I've rationalized "API compatibility" as meaning specifically making OIIO API calls and the presence of OIIO API headers. If an OIIO header incidentally includes (non-OIIO) foo.h, and then stops doing so, and that breaks user code because they also need something from foo.h but neglected to include it themselves in their module, I'm inclined to call that user error. I accepted that rationale when I was weeding openexr/imath headers from our public APIs.

So I'm ok with a small rewrite that removes windows.h from some or all of our public headers as a 2.5 feature (i.e., to put it in master now), but am still leery about backporting it to 2.4 now that it's released. I guess the way I navigate the grey area I described in the paragraph above is to allow it between minor releases but not from patch to patch within a minor release.

This present PR looks fine and we should backport it to 2.4. Then it would be great to have a follow-up that did a better job eliminating windows.h, but maybe just for master (2.5) and not backport that change.

[aras-p]

That sounds like a good plan indeed! I can do a 2.5-only followup with a larger windows.h removal later on.