(0018098)
Sean McBride (developer)
2009-10-18 22:12

I would *love* to see this too!

(0019051)
Marcus Hanwell (old account) (developer)
2010-01-07 19:22
edited on: 2010-01-07 19:24

Francois just pointed this bug out to me. I have done this to other apps in the past, and worked with KDE developers when it was being added to KDE. Today I checked in vtkABI.h, and a few days ago some changes to kwsys that add visibility support.

Locally I already have VTK compiled (just the libs - bindings were causing issues). In my mind it has benefits on start up time, I saw roughly a 37% reduction in the number of exported symbols for vtkCommon, and in helping Linux developers spot when they forget to export symbols until the Windows dashboard fails.

It is a fairly big change, but I hope to check in what I have soon. I will also be adding some default flags to GCC to increase warnings for Debug and RelWithDebInfo builds. Feedback on these additions would be appreciated.

(0019075)
Sean McBride (developer)
2010-01-08 17:57

I believe it will also help with dead code stripping. It could allow the linker to remove swaths of unused code, which could really reduce object code size in apps that use only small parts of VTK (and link to it statically).

(0019369)
Marcus Hanwell (old account) (developer)
2010-01-30 15:27

VTK CVS currently has some support for building the standard VTK kits with visibility support using >GCC 4.2 and Linux and Mac. Fixed a couple of tricky iterator template symbol visibility issues, and now the dashboards are all coming back green. Test it out and please let me know about any issues you encounter.

(0019845)
Marcus D. Hanwell (developer)
2010-03-11 16:19

This seems to be working well, and I recently enabled it for ParaView/other projects building VTK.

(0019846)
Sean McBride (developer)
2010-03-11 16:21

Marcus, does it still only use the visibility flags when building shared libs? Or does it work for static libs too?

(0019848)
Marcus D. Hanwell (developer)
2010-03-11 17:01

Thanks for reminding me - changed the conditional now. Please update from CVS head, VTK/CMake/vtkCompilerExtras.cmake 1.7 has the change.

(0019850)
Sean McBride (developer)
2010-03-11 18:03

I updated and everything still builds. But I do get lots of "has different visibility" linker warnings, both building VTK itself, and building my app that links against VTK.

(0019851)
Clinton Stimpson (developer)
2010-03-12 00:02

The public api should probably have default visibility regardless of being a shared or static build. Right now, it looks like vtkWin32Header.h does it for a shared build only, hence the link warnings on Mac OS X.

The link warnings can be avoided if the code using VTK is compiled with visibility flags, but that's assuming the code using VTK will accept being compiled that way.

(0019861)
Marcus D. Hanwell (developer)
2010-03-12 15:26

I am just compiling VTK statically on Windows (my Windows test machine is a lot slower than my main Linux workstation). It would require different handling in vtkABI.h, but the approach seems to work well.

#if (defined(_WIN32) || defined (__CYGWIN__)) && defined(VTK_BUILD_SHARED_LIBS)
# define VTK_ABI_IMPORT __declspec(dllimport)
# define VTK_ABI_EXPORT __declspec(dllexport)
# define VTK_ABI_HIDDEN
#elif __GNUC__ >= 4
...

In vtkABI.h, then vtkWin32Header.h remove the check for VTK_BUILD_SHARED_LIBS. I want to check the tests still pass on Windows before pushing changes that might affect it.

(0028256)
Marcus D. Hanwell (developer)
2012-02-13 14:54

I think what we have in VTK works well most of the time, i.e. shared builds with visibility flags enabled and in the static case the visibility is not changed. If we want to tackle visibility in static builds I would rather handle that on a new bug report, assuming there is sufficient demand. The move to vtkModular changes how this is handled quite a bit too, using the new export header module from CMake.

Issue History
Date Modified	Username	Field	Change
2009-09-16 12:00	Clinton Stimpson	New Issue
2009-10-18 22:12	Sean McBride	Note Added: 0018098
2010-01-07 19:22	Marcus Hanwell (old account)	Status	backlog => tabled
2010-01-07 19:22	Marcus Hanwell (old account)	Assigned To	=> Marcus Hanwell (old account)
2010-01-07 19:22	Marcus Hanwell (old account)	Note Added: 0019051
2010-01-07 19:24	Marcus Hanwell (old account)	Note Edited: 0019051
2010-01-08 17:57	Sean McBride	Note Added: 0019075
2010-01-30 15:27	Marcus Hanwell (old account)	Note Added: 0019369
2010-03-11 16:18	Marcus Hanwell (old account)	Assigned To	Marcus Hanwell (old account) => Marcus D. Hanwell
2010-03-11 16:19	Marcus D. Hanwell	Note Added: 0019845
2010-03-11 16:19	Marcus D. Hanwell	Status	tabled => @80@
2010-03-11 16:19	Marcus D. Hanwell	Resolution	open => fixed
2010-03-11 16:21	Sean McBride	Note Added: 0019846
2010-03-11 16:21	Sean McBride	Status	@80@ => @20@
2010-03-11 16:21	Sean McBride	Resolution	fixed => reopened
2010-03-11 17:01	Marcus D. Hanwell	Note Added: 0019848
2010-03-11 18:03	Sean McBride	Note Added: 0019850
2010-03-12 00:02	Clinton Stimpson	Note Added: 0019851
2010-03-12 15:26	Marcus D. Hanwell	Note Added: 0019861
2011-01-13 17:00		Source_changeset_attached	=> VTK master a2bd8391
2011-01-13 17:00		Source_changeset_attached	=> VTK master 020ef709
2011-06-16 13:11	Zack Galbreath	Category	=> (No Category)
2012-02-13 11:00	Jeff Baumes	Project	=> Titan
2012-02-13 11:00	Jeff Baumes	Type	=> incorrect functionality
2012-02-13 11:00	Jeff Baumes	Status	backlog => tabled
2012-02-13 11:00	Jeff Baumes	Description Updated
2012-02-13 14:54	Marcus D. Hanwell	Note Added: 0028256
2012-02-13 14:54	Marcus D. Hanwell	Status	tabled => closed
2012-02-13 14:54	Marcus D. Hanwell	Resolution	reopened => fixed

Notes
(0018098) Sean McBride (developer) 2009-10-18 22:12	I would love to see this too!

(0019051) Marcus Hanwell (old account) (developer) 2010-01-07 19:22 edited on: 2010-01-07 19:24	Francois just pointed this bug out to me. I have done this to other apps in the past, and worked with KDE developers when it was being added to KDE. Today I checked in vtkABI.h, and a few days ago some changes to kwsys that add visibility support. Locally I already have VTK compiled (just the libs - bindings were causing issues). In my mind it has benefits on start up time, I saw roughly a 37% reduction in the number of exported symbols for vtkCommon, and in helping Linux developers spot when they forget to export symbols until the Windows dashboard fails. It is a fairly big change, but I hope to check in what I have soon. I will also be adding some default flags to GCC to increase warnings for Debug and RelWithDebInfo builds. Feedback on these additions would be appreciated.

(0019075) Sean McBride (developer) 2010-01-08 17:57	I believe it will also help with dead code stripping. It could allow the linker to remove swaths of unused code, which could really reduce object code size in apps that use only small parts of VTK (and link to it statically).

(0019369) Marcus Hanwell (old account) (developer) 2010-01-30 15:27	VTK CVS currently has some support for building the standard VTK kits with visibility support using >GCC 4.2 and Linux and Mac. Fixed a couple of tricky iterator template symbol visibility issues, and now the dashboards are all coming back green. Test it out and please let me know about any issues you encounter.

(0019845) Marcus D. Hanwell (developer) 2010-03-11 16:19	This seems to be working well, and I recently enabled it for ParaView/other projects building VTK.

(0019846) Sean McBride (developer) 2010-03-11 16:21	Marcus, does it still only use the visibility flags when building shared libs? Or does it work for static libs too?

(0019848) Marcus D. Hanwell (developer) 2010-03-11 17:01	Thanks for reminding me - changed the conditional now. Please update from CVS head, VTK/CMake/vtkCompilerExtras.cmake 1.7 has the change.

(0019850) Sean McBride (developer) 2010-03-11 18:03	I updated and everything still builds. But I do get lots of "has different visibility" linker warnings, both building VTK itself, and building my app that links against VTK.

(0019851) Clinton Stimpson (developer) 2010-03-12 00:02	The public api should probably have default visibility regardless of being a shared or static build. Right now, it looks like vtkWin32Header.h does it for a shared build only, hence the link warnings on Mac OS X. The link warnings can be avoided if the code using VTK is compiled with visibility flags, but that's assuming the code using VTK will accept being compiled that way.

(0019861) Marcus D. Hanwell (developer) 2010-03-12 15:26	I am just compiling VTK statically on Windows (my Windows test machine is a lot slower than my main Linux workstation). It would require different handling in vtkABI.h, but the approach seems to work well. #if (defined(_WIN32) \|\| defined (__CYGWIN__)) && defined(VTK_BUILD_SHARED_LIBS) # define VTK_ABI_IMPORT __declspec(dllimport) # define VTK_ABI_EXPORT __declspec(dllexport) # define VTK_ABI_HIDDEN #elif __GNUC__ >= 4 ... In vtkABI.h, then vtkWin32Header.h remove the check for VTK_BUILD_SHARED_LIBS. I want to check the tests still pass on Windows before pushing changes that might affect it.

(0028256) Marcus D. Hanwell (developer) 2012-02-13 14:54	I think what we have in VTK works well most of the time, i.e. shared builds with visibility flags enabled and in the static case the visibility is not changed. If we want to tackle visibility in static builds I would rather handle that on a new bug report, assuming there is sufficient demand. The move to vtkModular changes how this is handled quite a bit too, using the new export header module from CMake.

Relationships