aylward
commented
2 years ago Please feel free to edit the file as you see fit!
Thanks! s
On Thu, Nov 18, 2021 at 5:06 PM Jean-Christophe Fillion-Robin < @.***> wrote:
@.**** commented on this pull request.
Last, I also suggest adding a "Commercial Support" section
In README.md https://github.com/Kitware/LookingGlassVTKModule/pull/29#discussion_r752641182 :
+The SDK is described
+here.
+
+You can request access to their SDK
+here.
Currently in the License section, it is specified that vtkLookingGlassInterface.cxx uses content from HoloPlayCore, which is distributed under MIT license.
I suggest to be more explicit about the license term associated with the complete SDK by indicating that the relevant details can be found in the LICENSE.txt distributed with the SDK.
In README.md https://github.com/Kitware/LookingGlassVTKModule/pull/29#discussion_r752641709 :
- HoloPlayCore_INCLUDE_DIR
- HoloPlayCore_LIBRARY
⬇️ Suggested change
-1. HoloPlayCore_INCLUDE_DIR
-2. HoloPlayCore_LIBRARY
+1.
HoloPlayCore_INCLUDE_DIR+2.
HoloPlayCore_LIBRARY
In README.md https://github.com/Kitware/LookingGlassVTKModule/pull/29#discussion_r752642231 :
- VTK_MODULE_ENABLE_VTK_RenderingLookingGlass to YES
VTK_USE_VIDEO_FOR_WINDOWS to ON
VTK_USE_MICROSOFT_MEDIA_FOUNDATION to ON
⬇️ Suggested change
-1. VTK_MODULE_ENABLE_VTK_RenderingLookingGlass to YES
-2. VTK_USE_VIDEO_FOR_WINDOWS to ON
-3. VTK_USE_MICROSOFT_MEDIA_FOUNDATION to ON
+1.
VTK_MODULE_ENABLE_VTK_RenderingLookingGlasstoYES+2.
VTK_USE_VIDEO_FOR_WINDOWSto ON` (only on Windows)+3.
VTK_USE_MICROSOFT_MEDIA_FOUNDATIONto ON` (only on Windows)
In README.md https://github.com/Kitware/LookingGlassVTKModule/pull/29#discussion_r752647526 :
+The SDK is described
+here.
+
+You can request access to their SDK
+here.
+
+Access is granted nearly instantaneously. Their SDK includes dylibs
+(shared libraries) for MacOS, Linux, and Windows (32 and 64bit).
⬇️ Suggested change
-(shared libraries) for MacOS, Linux, and Windows (32 and 64bit).
+(shared libraries) for macOS, Linux, and Windows (32 and 64bit).
Small nitpick: Since the release of Sierra, the case to reference the operating system should be macOS. See https://en.wikipedia.org/wiki/macOS
In README.md https://github.com/Kitware/LookingGlassVTKModule/pull/29#discussion_r752652020 :
-Note that almost all of the key functionality is held in
-vtkLookingGlassInterface which is used by both the render window classes and
-the render pass implementations. This module should work, showing the quilt
-image even if no hardware is present.
+## Running VTK applications
+
+Since the HoloPlayCoreSDK is distribued as shared libraries, their location
+must be known when executing a VTK-based application if that version of
+VTK was compiled with this module enabled.
+
+We recommend adding the path to the appropriate shared lib to your system's
+PATH/LD_LIBRARY_PATH variables. They will then be automatically found
+when running VTK applications that were compiled with this module enabled.
+
+System-level access to the shared libs is also required when running the
+Python wrapped VTK if this module was enabled when it was compiled.
Moving forward library path specified through env. variable like PATH will not be automatically found, this has some security implication (e.g DLL injection https://en.wikipedia.org/wiki/DLL_injection)
Start with Python 3.8, this will not work.
DLL dependencies for extension modules and DLLs loaded with ctypes https://docs.python.org/3/library/ctypes.html#module-ctypes on Windows are now resolved more securely. Only the system paths, the directory containing the DLL or PYD file, and directories added with add_dll_directory() https://docs.python.org/3/library/os.html#os.add_dll_directory are searched for load-time dependencies. Specifically, PATH and the current working directory are no longer used, and modifications to these will no longer have any effect on normal DLL resolution. If your application relies on these mechanisms, you should check for add_dll_directory() https://docs.python.org/3/library/os.html#os.add_dll_directory and if it exists, use it to add your DLLs directory while loading your library. Note that Windows 7 users will need to ensure that Windows Update KB2533623 has been installed (this is also verified by the installer). (Contributed by Steve Dower in bpo-36085 https://bugs.python.org/issue36085.)
Source: https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew
In README.md https://github.com/Kitware/LookingGlassVTKModule/pull/29#discussion_r752652729 :
-Note that almost all of the key functionality is held in
-vtkLookingGlassInterface which is used by both the render window classes and
-the render pass implementations. This module should work, showing the quilt
-image even if no hardware is present.
+## Running VTK applications
+
+Since the HoloPlayCoreSDK is distribued as shared libraries, their location
+must be known when executing a VTK-based application if that version of
+VTK was compiled with this module enabled.
+
+We recommend adding the path to the appropriate shared lib to your system's
Generally speaking, we should not recommend updating system environment. Instead, application specific launcher should be used (e.g using script or sourcing env. file)
In README.md https://github.com/Kitware/LookingGlassVTKModule/pull/29#discussion_r752653501 :
+## Running VTK applications
+
+Since the HoloPlayCoreSDK is distribued as shared libraries, their location
+must be known when executing a VTK-based application if that version of
+VTK was compiled with this module enabled.
+
+We recommend adding the path to the appropriate shared lib to your system's
+PATH/LD_LIBRARY_PATH variables. They will then be automatically found
+when running VTK applications that were compiled with this module enabled.
+
+System-level access to the shared libs is also required when running the
+Python wrapped VTK if this module was enabled when it was compiled.
+
+## Rendering to a display and generating Quilts
+
+The key functionality of this module is held in the
vtkLookingGlassInterface⬇️ Suggested change
-The key functionality of this module is held in the
vtkLookingGlassInterface+The key functionality of this module is held in the vtkLookingGlassInterface
In README.md https://github.com/Kitware/LookingGlassVTKModule/pull/29#discussion_r752660911 :
+VTK was compiled with this module enabled.
+
+We recommend adding the path to the appropriate shared lib to your system's
+PATH/LD_LIBRARY_PATH variables. They will then be automatically found
+when running VTK applications that were compiled with this module enabled.
+
+System-level access to the shared libs is also required when running the
+Python wrapped VTK if this module was enabled when it was compiled.
+
+## Rendering to a display and generating Quilts
+
+The key functionality of this module is held in the
vtkLookingGlassInterface+class. It is used by the render window classes and by the render pass
+implementations. In theory, this module should be able to generate and
+display quilt images even if no Looking Glass hardware is present, but
+those capabilities has not been extensively tested.
I suggest to move this last sentence into a "What is next/Roadmap" section.
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/Kitware/LookingGlassVTKModule/pull/29#pullrequestreview-810477163, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACEJL2GAGTDQBHJSHM6F4LUMV2FBANCNFSM5IKTAZDQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
-- Stephen R. Aylward, Ph.D. Senior Director of Strategic Initiatives
Kitware: Delivering innovative, open source, scientific software.
Updating README.md to specify that Looking Glass' HoloPlayCoreSDK is required and how to obtain it.
Also providing documentation on VTK options that need to be enabled when configuring this module.