[Bf-committers] [Proposal] CMake patch for VC2012 build

[Brecht Van Lommel]

Comments inline.

On Tue, May 14, 2013 at 10:59 AM, Jürgen Herrmann <shadowrom at me.com> wrote:
> brecht asked me to post the patch for CMake VS2012 here before committing:
> And here it is ;)
> Any comments or suggetions?
>
> ---- SNIP ----
>
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt    (revision 56785)
> +++ CMakeLists.txt    (working copy)
> @@ -157,7 +157,12 @@
>  option(WITH_BOOL_COMPAT "Continue defining \"TRUE\" and \"FALSE\" until these can be replaced with \"true\" and \"false\" from stdbool.h" ON)
>  mark_as_advanced(WITH_BOOL_COMPAT)
>
> +if(MSVC11)
> +    option(WITH_AVX_CPU " --WARNING! EXPERIMENTAL-- Build for newer CPUs with AVX extensions. Doesn't work on older CPUs!" OFF)
> +    mark_as_advanced(WITH_AVX_CPU)
> +endif()

The description can just use normal capitalization an doesn't need
exclamation marks and --.

> +
>  # (unix defaults to OpenMP On)
>  if((UNIX AND NOT APPLE) OR (MINGW))
>      set(PLATFORM_DEFAULT ON)
> @@ -948,12 +953,25 @@
>              set(LIBDIR ${CMAKE_SOURCE_DIR}/../lib/mingw64)
>          endif()
>      else()
> +        #default dir, just in case ;)
>          set(LIBDIR ${CMAKE_SOURCE_DIR}/../lib/windows)

This line can just be removed.

>          # Setup 64bit and 64bit windows systems
>          if(CMAKE_CL_64)
>              message("64 bit compiler detected.")

Messages should have a category, STATUS in this case.

> -            set(LIBDIR ${CMAKE_SOURCE_DIR}/../lib/win64)
> +            if(MSVC11)
> +                message("Visual C++ 2012 detected.")
> +                set(LIBDIR ${CMAKE_SOURCE_DIR}/../lib/win64_vc11)
> +            else()
> +                set(LIBDIR ${CMAKE_SOURCE_DIR}/../lib/win64)
> +            endif()
> +        else()
> +        # Setup 32bit windows systems

Indent one more level.

> +            if(MSVC11)
> +                message("Visual C++ 2012 detected.")
> +                set(LIBDIR ${CMAKE_SOURCE_DIR}/../lib/windows_vc11)
> +            else()
> +                set(LIBDIR ${CMAKE_SOURCE_DIR}/../lib/windows)
> +            endif()
>          endif()
>      endif()
>
> @@ -968,7 +986,12 @@
>      if(WITH_OPENAL)
>          set(OPENAL ${LIBDIR}/openal)
>          set(OPENAL_INCLUDE_DIR ${OPENAL}/include)
> -        set(OPENAL_LIBRARY wrap_oal)
> +        # MSVC 11 libs ware with OpenAL-Soft and don't bring wrap_oal.lib
> +        if(MSVC11)
> +            set(OPENAL_LIBRARY openal32)
> +        else()
> +            set(OPENAL_LIBRARY wrap_oal)
> +        endif()
>          set(OPENAL_LIBPATH ${OPENAL}/lib)
>      endif()

Last I checked 3D sound was broken for OpenAL-Soft, which is why we
don't use it. Can you test if it is fixed now?

> @@ -1017,8 +1040,23 @@
>      if(MSVC)
>          set(PLATFORM_LINKLIBS ws2_32 vfw32 winmm kernel32 user32 gdi32 comdlg32 advapi32 shfolder shell32 ole32 oleaut32 uuid psapi)
>
> +        #MSVC11 SDL is not hard linked to dxguid.lib
> +        if(MSVC11)
> +            set(PLATFORM_LINKLIBS ${PLATFORM_LINKLIBS} dxguid)
> +        endif()
> +

Can you make this if(MSVC11 AND WITH_SDL)?

Also nitpicking, please add a space after the # for comments here and
in other places, for consistency

>          add_definitions(/D_CRT_NONSTDC_NO_DEPRECATE /D_CRT_SECURE_NO_DEPRECATE /D_SCL_SECURE_NO_DEPRECATE /D_CONSOLE /D_LIB)
>
> +        #MSVC11 needs _ALLOW_KEYWORD_MACROS to build
> +        if(MSVC11)
> +            add_definitions(/D_ALLOW_KEYWORD_MACROS)
> +            #Experimental feature compile with /arch:AVX MSVC11 ONLY
> +            if(WITH_AVX_CPU)
> +                message("compiling with /arch:AVX")

Don't think this message is needed, can't really start doing this for
every option.

> +                add_definitions(/arch:AVX)
> +            endif()
> +        endif()
> +
>          set(CMAKE_CXX_FLAGS "/nologo /J /Gd /EHsc" CACHE STRING "MSVC MT C++ flags " FORCE)
>          set(CMAKE_C_FLAGS   "/nologo /J /Gd"       CACHE STRING "MSVC MT C++ flags " FORCE)
>
> @@ -1186,8 +1235,12 @@
>          if(WITH_BOOST)
>              set(BOOST ${LIBDIR}/boost)
>              set(BOOST_INCLUDE_DIR ${BOOST}/include)
> -            if(MSVC10)
> -                set(BOOST_LIBPATH ${BOOST}/vc2010/lib)
> +            if(MSVC11)
> +                set(BOOST_LIBPATH ${BOOST}/lib)
> +                set(BOOST_POSTFIX "vc110-mt-s-1_53.lib")
> +                set(BOOST_DEBUG_POSTFIX "vc110-mt-sgd-1_53.lib")
> +            elseif(MSVC10)
> +                set(BOOST_LIBPATH ${BOOST}/vc2012/lib)

Why did this change to 2012, it should remain 2010?

>                  set(BOOST_POSTFIX "vc100-mt-s-1_49.lib")
>                  set(BOOST_DEBUG_POSTFIX "vc100-mt-sgd-1_49.lib")
>              else()
> @@ -1236,7 +1289,6 @@
>              set(OPENCOLORIO_DEFINITIONS)
>          endif()
>
> -
>          set(PLATFORM_LINKFLAGS "/SUBSYSTEM:CONSOLE /STACK:2097152 /INCREMENTAL:NO /NODEFAULTLIB:msvcrt.lib /NODEFAULTLIB:msvcmrt.lib /NODEFAULTLIB:msvcurt.lib /NODEFAULTLIB:msvcrtd.lib")
>
>          # MSVC only, Mingw doesnt need