Skip to content

Commit

Permalink
[gts] Move to official upstream sources (#25034)
Browse files Browse the repository at this point in the history
* [gts] Move to official upstream sources

* [gts] Rename src->SOURCE_PATH
  • Loading branch information
ras0219-msft authored Jun 7, 2022
1 parent 3b040ec commit 053c82f
Show file tree
Hide file tree
Showing 12 changed files with 124 additions and 322 deletions.
43 changes: 43 additions & 0 deletions ports/gts/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
cmake_minimum_required(VERSION 3.14)

project(gts C)

find_package(PkgConfig REQUIRED)
pkg_check_modules(GLIB2 glib-2.0 IMPORTED_TARGET)

if(WIN32)
add_definitions(-DNATIVE_WIN32 -D_USE_MATH_DEFINES)
else()
add_definitions(-DHAVE_FPU_CONTROL_H)
endif()

add_definitions(
-DGTS_COMPILATION
-DGTS_MAJOR_VERSION=0
-DGTS_MINOR_VERSION=7
-DGTS_MICRO_VERSION=6
-DGTS_INTERFACE_AGE=1
-DGTS_BINARY_AGE=1
-DGTS_VERSION=${GTS_MAJOR_VERSION}.${GTS_MINOR_VERSION}.${GTS_MICRO_VERSION}
)

file(GLOB src src/*.c src/gts.def)
list(REMOVE_ITEM src src/predicate_init.c)
add_library(gts ${src})
file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/config.h" "")
target_include_directories(gts PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/src>
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
$<INSTALL_INTERFACE:include>
)
target_link_libraries(gts PUBLIC PkgConfig::GLIB2)

set(prefix ?)
set(exec_prefix \${prefix})
set(libdir \${prefix}/lib)
set(includedir \${prefix}/include)
configure_file(gts.pc.in gts.pc @ONLY)

install(FILES src/gts.h src/gtsconfig.h DESTINATION include)
install(TARGETS gts)
install(FILES "${CMAKE_CURRENT_BINARY_DIR}/gts.pc" DESTINATION lib/pkgconfig)
28 changes: 0 additions & 28 deletions ports/gts/fix-M_PI-in-windows.patch

This file was deleted.

13 changes: 13 additions & 0 deletions ports/gts/fix-dllexport.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/src/gts.h b/src/gts.h
index 1b76915..ae1d888 100644
--- a/src/gts.h
+++ b/src/gts.h
@@ -38,7 +38,7 @@ extern "C" {
* we prefix variable declarations so they can
* properly get exported in windows dlls.
*/
-#ifdef NATIVE_WIN32
+#if 0

This comment has been minimized.

Copy link
@magjac

magjac Jun 30, 2024

This change causes an access violation on Windows when accessing gts_allow_floating_vertices which is not aligned on a 4-byte boundary. Why was this disabled with #if 0?

This comment has been minimized.

Copy link
@dg0yt

dg0yt Jul 1, 2024

Contributor

AFAICS this patch is applied only for static library linkage, and the changed line is meant to disable dllexport decorations. This doesn't seem wrong, and it doesn't seem to be connect to alignment at all.
Can you provide more information?

This comment has been minimized.

Copy link
@magjac

magjac Jul 1, 2024

Thanks for the fast response.

These are the commands I run:

./vcpkg/vcpkg.exe install --recurse --clean-after-build --host-triplet=x64-windows gts
./vcpkg/vcpkg.exe export --raw --output=gts-x64-vcpkg-export --host-triplet=x64-windows gts

and this is the relevant part of the generated header from ./gts-x64-vcpkg-export/installed/x64-windows/include/gts.h

/* Added based on glib.h by M J Loehr 01/01/01 */
/* GTS version.
 * we prefix variable declarations so they can
 * properly get exported in windows dlls.
 */
#if 0
#  ifdef GTS_COMPILATION
#    define GTS_C_VAR __declspec(dllexport)
#  else /* not GTS_COMPILATION */
#    define GTS_C_VAR extern __declspec(dllimport)
#  endif /* not GTS_COMPILATION */
#else /* not NATIVE_WIN32 */
#  define GTS_C_VAR extern
#endif /* not NATIVE_WIN32 */

So it seems the patch is applied even though I don't specify x64-windows-static.

I don't know how this causes gts_allow_floating_vertices to become incorrectly aligned. Just that it does.

I've instrumented my code with some debug printing showing that four bytes are allocated for gts_allow_floating_vertices, but that it isn't aligned on a 4-byte boundary. You can see the code, the debug prints and the exception here:

image

If I apply this patch:

diff --git a/vcpkg/installed/x64-windows/include/gts.h b/vcpkg/installed/x64-windows/include/gts.h
index bb03207..925ad0d 100644
--- a/vcpkg/installed/x64-windows/include/gts.h
+++ b/vcpkg/installed/x64-windows/include/gts.h
@@ -38,7 +38,7 @@ extern "C" {
  * we prefix variable declarations so they can
  * properly get exported in windows dlls.
  */
-#if 0
+#if 1
 #  ifdef GTS_COMPILATION
 #    define GTS_C_VAR __declspec(dllexport)
 #  else /* not GTS_COMPILATION */

the variables are all correctly aligned on 4-byte boundaries and program passes the place where the exception occurred and runs to completion. You can see this here:
image

This comment has been minimized.

Copy link
@dg0yt

dg0yt Jul 5, 2024

Contributor

I realize that your comment is on an old commit, but the problem might be related to recent changes, maybe #34673.
Can you help verify the version which introduced the problem?

This comment has been minimized.

Copy link
@magjac

magjac Jul 9, 2024

I haven't been able to run the test yet, but yes, if I check out the commit before the one that merged #34673, 27728bb I get:

#if 1
#  ifdef GTS_COMPILATION
#    define GTS_C_VAR extern __declspec(dllexport)
#  else /* not GTS_COMPILATION */
#    define GTS_C_VAR extern __declspec(dllimport)
#  endif /* not GTS_COMPILATION */
#else /* not NATIVE_WIN32 */
#  define GTS_C_VAR extern
#endif /* not NATIVE_WIN32 */

(which is the same as my workaround), while if I check out the one that merged #34673, 79be4be, I get:

#if 0
#  ifdef GTS_COMPILATION
#    define GTS_C_VAR __declspec(dllexport)
#  else /* not GTS_COMPILATION */
#    define GTS_C_VAR extern __declspec(dllimport)
#  endif /* not GTS_COMPILATION */
#else /* not NATIVE_WIN32 */
#  define GTS_C_VAR extern
#endif /* not NATIVE_WIN32 */

which is what causes me problems.

# ifdef GTS_COMPILATION
# define GTS_C_VAR __declspec(dllexport)
# else /* not GTS_COMPILATION */
28 changes: 0 additions & 28 deletions ports/gts/fix-pkgconfig.patch

This file was deleted.

54 changes: 0 additions & 54 deletions ports/gts/glib2.patch

This file was deleted.

39 changes: 22 additions & 17 deletions ports/gts/portfile.cmake
Original file line number Diff line number Diff line change
@@ -1,29 +1,34 @@
vcpkg_check_linkage(ONLY_DYNAMIC_LIBRARY ONLY_DYNAMIC_CRT)
if(VCPKG_LIBRARY_LINKAGE STREQUAL "static")
set(patches fix-dllexport.patch)
else()
set(patches "")
endif()

vcpkg_from_github(
vcpkg_from_sourceforge(
OUT_SOURCE_PATH SOURCE_PATH
REPO finetjul/gts
REF c4da61ae075f355d9ecc9f2d4767acf777f54c2b #0.7.6
SHA512 e53d11213c26cbda08ae62e6388aee0a14d2884de72268ad25d10a23e77baa53a2b1151c5cc7643b059ded82b8edf0da79144c3108949fdc515168cac13ffca9
HEAD_REF master
PATCHES
fix-M_PI-in-windows.patch
support-unix.patch
fix-pkgconfig.patch
glib2.patch
REPO gts/gts
REF 0.7.6
FILENAME gts-0.7.6.tar.gz
SHA512 645123b72dba3d04dad3c5d936d7e55947826be0fb25e84595368919b720deccddceb7c3b30865a5a40f2458254c2af793b7c014e6719cf07e7f8e6ff30890f8
PATCHES ${patches}
)

file(COPY "${CMAKE_CURRENT_LIST_DIR}/CMakeLists.txt" DESTINATION "${SOURCE_PATH}")
file(COPY "${CMAKE_CURRENT_LIST_DIR}/predicates_init.h" DESTINATION "${SOURCE_PATH}/src")

vcpkg_find_acquire_program(PKGCONFIG)
vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
vcpkg_cmake_configure(
SOURCE_PATH "${SOURCE_PATH}"
OPTIONS
-DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}
"-DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}"
)

vcpkg_install_cmake()
vcpkg_cmake_install()

file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/include)
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")

vcpkg_fixup_pkgconfig()
vcpkg_copy_pdbs()

file(INSTALL ${SOURCE_PATH}/COPYING DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT} RENAME copyright)
configure_file("${SOURCE_PATH}/COPYING" "${CURRENT_PACKAGES_DIR}/share/${PORT}/copyright" COPYONLY)
file(COPY "${CMAKE_CURRENT_LIST_DIR}/usage" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}")
26 changes: 26 additions & 0 deletions ports/gts/predicates_init.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* This file was generated automatically by predicates_init
*
* This file is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
* License as published by the Free Software Foundation; either
* version 2 of the License, or (at your option) any later version.
*
* This file is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
*/

static double splitter = 134217729.000000;
static double resulterrbound = 3.330669073875471e-16;
static double ccwerrboundA = 3.330669073875472e-16;
static double ccwerrboundB = 2.220446049250315e-16;
static double ccwerrboundC = 1.109335647967049e-31;
static double o3derrboundA = 7.771561172376103e-16;
static double o3derrboundB = 3.330669073875473e-16;
static double o3derrboundC = 3.204747427460364e-31;
static double iccerrboundA = 1.110223024625158e-15;
static double iccerrboundB = 4.440892098500632e-16;
static double iccerrboundC = 5.423418723394464e-31;
static double isperrboundA = 1.776356839400253e-15;
static double isperrboundB = 5.551115123125792e-16;
static double isperrboundC = 8.751425667295619e-31;
Loading

0 comments on commit 053c82f

Please sign in to comment.