> +#ifdef _MSC_VER
> +#undef max
> +#endif
>  void AnnotLine::generateLineAppearance()
>  {

It would be better to define NOMINMAX before including windows.h as documented in http://support.microsoft.com/kb/143208

The long term solution would be to not include windows.h in our header files. All windows specific code should factored out into separate .cc files.

Created attachment 91276 [details] [review]
patch for poppler compilation with Qt5 frontend on Win8 with VS 2012

Here is the patch with the suggested correction. I would also recommend to replace all _WIN32 with _MSC_VER which is more generic than looking for _WIN32 only

(In reply to comment #2)
> Created attachment 91276 [details] [review] [review]
> patch for poppler compilation with Qt5 frontend on Win8 with VS 2012

The #define NOMINAX should go in each header file that include windows.h. Otherwise we will have the same problem the next time someone adds a max or min to another file.

> Here is the patch with the suggested correction. I would also recommend to
> replace all _WIN32 with _MSC_VER which is more generic than looking for
> _WIN32 only

_WIN32 is defined when compiling for the windows platform. _MSC_VER is defined when compiling with VC++. If compiling with MinGW (either natively or cross compiling) _WIN32 will be defined but not _MSC_VER.

Looking at your patch the you should be using _WIN32 as it is all windows platform specific stuff (generally anything related to windows.h). _MSC_VER is only used to handle the differences between VC++ and GCC.

Created attachment 91288 [details] [review]
patch for poppler compilation with Qt5 frontend on Win8 with VS 2012

here is the new path where all _MSC_VER have been replaced by _WIN32. I cannot define NOMINMAX in each header where windows.h is included as in some sources min and max macros are expected.

Can you please mark the old patches as obsolete when posting new ones? That really helps when one goes to the "Attachments" table to see what one needs to apply.

I am guessing you need the same in qt4?

This code seems to have been working for a while. Why are these changes now needed?

On Windows with Visual Studio toWCharArray is not found at link time as Qt has its own wchar_t, so I am wondering if poppler has been tested with Visual Studio. I have used only Qt5.2, not sure about Qt4, but I can make some tests if you request.

I'd appreciate if you could try the qt4 side since it has the same code and thus it'd seem it would have the same issues.

Also any idea if this will affect the mingw users?

(In reply to comment #4)
> I cannot define NOMINMAX in each header where windows.h is included as in some
> sources min and max macros are expected.

Can they be replaced with std::min/std::max?

Created attachment 91324 [details] [review]
patch for poppler compilation with Qt5 frontend on Win8 with VS 2012

poppler-qt4 test needs to replace unistd.h with Windows.h in order to have Sleep function

So the wchar_t thing doesn't happen in qt4? Interesting. 

About the minmax thing, how does windows.h end up included in there? Seems to me it's via GooMutex. Maybe we can narrow the include in GooMutex to not be windows.h but winbase.h?

Otherwise I'm with Adrian, we should put the define after each include of windows.h and if we are using max() change it to std::max

Created attachment 91359 [details] [review]
patch for poppler compilation with Qt5 or Qt4 frontend on Win8 with VS 2012

This new patch defines NOMINMAX above each include of windows.h, some includes are removed and max is replaced by std::max

This patch looks good to me.

Commited, thanks!