Skip to content

Commit

Permalink
[workspace] Add vtk_internal build from source
Browse files Browse the repository at this point in the history
This commit does NOT switch Drake to use the VTK build from source.

Instead, we'll only compile vtkImageIO and then validate it with a
temporary unit test. Future work will switch the various parts of
Drake to use the VTK source build.

The goal here is to get the build system infrastructure under control,
so that it's easy to tweak and apply it down the road.
  • Loading branch information
jwnimmer-tri committed Aug 28, 2023
1 parent 5a42a06 commit 5a5968b
Show file tree
Hide file tree
Showing 12 changed files with 1,208 additions and 0 deletions.
25 changes: 25 additions & 0 deletions systems/sensors/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -613,4 +613,29 @@ drake_cc_googletest(
],
)

# This "reader writer canary" is the same test program as above, but linked
# with @vtk_internal (VTK built from source) instead of @vtk (the host VTK).
# This is a temporary measure for CI to help us understand if the VTK source
# build is satisfactory. Once all of Drake has switched to @vtk_internal, we
# can (and should) remove this test rule, since it will be redundant.
drake_cc_googletest(
name = "vtk_image_reader_writer_canary_test",
srcs = [
"image_file_format.cc",
"image_file_format.h",
"test/vtk_image_reader_writer_test.cc",
"vtk_image_reader_writer.cc",
"vtk_image_reader_writer.h",
],
deps = [
"//common:essential",
"//common:temp_directory",
"//common:unused",
"//common/test_utilities:expect_throws_message",
"@vtk_internal//:vtkCommonCore",
"@vtk_internal//:vtkCommonDataModel",
"@vtk_internal//:vtkIOImage",
],
)

add_lint_tests()
3 changes: 3 additions & 0 deletions tools/workspace/default.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ load("@drake//tools/workspace/uwebsockets:repository.bzl", "uwebsockets_reposito
load("@drake//tools/workspace/uwebsockets_internal:repository.bzl", "uwebsockets_internal_repository") # noqa
load("@drake//tools/workspace/voxelized_geometry_tools:repository.bzl", "voxelized_geometry_tools_repository") # noqa
load("@drake//tools/workspace/vtk:repository.bzl", "vtk_repository")
load("@drake//tools/workspace/vtk_internal:repository.bzl", "vtk_internal_repository") # noqa
load("@drake//tools/workspace/x11:repository.bzl", "x11_repository")
load("@drake//tools/workspace/xmlrunner_py:repository.bzl", "xmlrunner_py_repository") # noqa
load("@drake//tools/workspace/yaml_cpp_internal:repository.bzl", "yaml_cpp_internal_repository") # noqa
Expand Down Expand Up @@ -352,6 +353,8 @@ def add_default_repositories(excludes = [], mirrors = DEFAULT_MIRRORS):
voxelized_geometry_tools_repository(name = "voxelized_geometry_tools", mirrors = mirrors) # noqa
if "vtk" not in excludes:
vtk_repository(name = "vtk", mirrors = mirrors)
if "vtk_internal" not in excludes:
vtk_internal_repository(name = "vtk_internal", mirrors = mirrors)
if "x11" not in excludes:
x11_repository(name = "x11")
if "xmlrunner_py" not in excludes:
Expand Down
3 changes: 3 additions & 0 deletions tools/workspace/vtk_internal/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
load("//tools/lint:lint.bzl", "add_lint_tests")

add_lint_tests()
41 changes: 41 additions & 0 deletions tools/workspace/vtk_internal/package.BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# -*- bazel -*-

load(
"@drake//tools/workspace/vtk_internal:rules.bzl",
"compile_all_modules",
"generate_abi_namespace",
"generate_common_core_sources",
"vtk_cc_module",
)

config_setting(
name = "osx",
constraint_values = ["@platforms//os:osx"],
)

# Generate some source files on the fly, using Bazel re-implementations of
# various CMake scripts.

generate_abi_namespace()

generate_common_core_sources()

# Help solve a circular dependency between CommonCore <=> CommonDataModel.
# This library inter-operates with both the VTK::CommonCore in settings.bzl
# and the repository patch file patches/common_core_vs_data_model_cycle.patch.

cc_library(
name = "VTK__CommonDataModel_vtkDataObject",
hdrs = [
"Common/DataModel/vtkCommonDataModelModule.h",
"Common/DataModel/vtkDataObject.h",
],
strip_include_prefix = "Common/DataModel",
linkstatic = True,
)

# Add a cc_library rule for all modules in settings.bzl that are marked with
# non-default visibility, and also add private cc_library rules for all of
# their required transitive dependency modules (per the vtk.module metadata).

compile_all_modules()
29 changes: 29 additions & 0 deletions tools/workspace/vtk_internal/patches/common_core_version.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
[vtk] Adjust VTK_VERSION macros for simplicity

Consolidate the version specification to a 3-tuple of individual numbers;
with Bazel, we can't rely on upstream's CMakeLists code to concatenate them.

This change is Drake-specific, so we do not plan to upstream it.

--- Common/Core/vtkVersionFull.h.in
+++ Common/Core/vtkVersionFull.h.in
@@ -17,6 +17,6 @@

/* This is in its own header to reduce build dependencies */

-#define VTK_VERSION_FULL "@VTK_VERSION_FULL@"
+#define VTK_VERSION_FULL VTK_VERSION "-drake"

#endif

--- Common/Core/vtkVersionMacros.h.in
+++ Common/Core/vtkVersionMacros.h.in
@@ -20,7 +20,7 @@
#define VTK_MAJOR_VERSION @VTK_MAJOR_VERSION@
#define VTK_MINOR_VERSION @VTK_MINOR_VERSION@
#define VTK_BUILD_VERSION @VTK_BUILD_VERSION@
-#define VTK_VERSION "@VTK_VERSION@"
+#define VTK_VERSION "@VTK_MAJOR_VERSION@.@VTK_MINOR_VERSION@.@VTK_BUILD_VERSION@"

#define VTK_SOURCE_VERSION "vtk version " VTK_VERSION

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[vtk] Adjust an include path to work around a dependency cycle

A file in Common/Core includes a header from Common/DataModel.
Instead of using a relative include that violates layering, we'll
include it from the current directory instead, and customize our
package.BUILD.bazel file to be sure it's available there using the
//:VTK__CommonDataModel_vtkDataObject label.

This situation is unique to Bazel, so we do not plan to upstream
any changes related to this problem.

--- Common/Core/vtkInformationDataObjectKey.cxx
+++ Common/Core/vtkInformationDataObjectKey.cxx
@@ -15,7 +15,7 @@
#include "vtkInformationDataObjectKey.h"

#if defined(vtkCommonDataModel_ENABLED)
-#include "../DataModel/vtkDataObject.h"
+#include /* ../DataModel/ */ "vtkDataObject.h"
#endif

//------------------------------------------------------------------------------
19 changes: 19 additions & 0 deletions tools/workspace/vtk_internal/patches/common_core_warnings.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[vtk] Fix warnings in VTK::CommonCore

vtkVariantInlineOperators: -Wlogical-op

TODO(jwnimmer-tri) We should try to upstream some flavor of this patch.

--- Common/Core/vtkVariantInlineOperators.h
+++ Common/Core/vtkVariantInlineOperators.h
@@ -14,7 +14,9 @@
VTK_ABI_NAMESPACE_BEGIN
inline bool IsSigned64Bit(int VariantType)
{
- return ((VariantType == VTK_LONG_LONG) || (VariantType == VTK_TYPE_INT64));
+ if (VariantType == VTK_TYPE_INT64) return true;
+ if (VariantType == VTK_LONG_LONG) return true;
+ return false;
}

inline bool IsSigned(int VariantType)
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[vtk] Fix warning in VTK::CommonDataModel

vtkBoundingBox: -Wold-style-cast

TODO(jwnimmer-tri) We should upstream this patch.

--- Common/DataModel/vtkBoundingBox.h
+++ Common/DataModel/vtkBoundingBox.h
@@ -86,7 +86,7 @@
static void ComputeBounds(vtkPoints* pts, const unsigned char* ptUses, double bounds[6]);
static void ComputeBounds(
vtkPoints* pts, const std::atomic<unsigned char>* ptUses, double bounds[6]);
- void ComputeBounds(vtkPoints* pts) { this->ComputeBounds(pts, (unsigned char*)nullptr); }
+ void ComputeBounds(vtkPoints* pts) { this->ComputeBounds(pts, static_cast<unsigned char*>(nullptr)); }
void ComputeBounds(vtkPoints* pts, unsigned char* ptUses)
{
double bds[6];
42 changes: 42 additions & 0 deletions tools/workspace/vtk_internal/patches/io_image_formats.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
[vtk] Disable Image formats we don't need

The list of supported (uncommented) image types seen in this patch
should be congruent with the *.cxx files listed in settings.bzl for
VTK::IOImage.

This situation is unique to Drake, so we do not plan to upstream this
patch.

--- IO/Image/vtkImageReader2Factory.cxx
+++ IO/Image/vtkImageReader2Factory.cxx
@@ -179,23 +179,23 @@ void vtkImageReader2Factory::InitializeReaders()

vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkPNGReader::New()));
reader->Delete();
- vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkPNMReader::New()));
+ // vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkPNMReader::New()));
reader->Delete();
vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkTIFFReader::New()));
reader->Delete();
- vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkBMPReader::New()));
+ // vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkBMPReader::New()));
reader->Delete();
- vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkSLCReader::New()));
+ // vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkSLCReader::New()));
reader->Delete();
- vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkHDRReader::New()));
+ // vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkHDRReader::New()));
reader->Delete();
vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkJPEGReader::New()));
reader->Delete();
- vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkGESignaReader::New()));
+ // vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkGESignaReader::New()));
reader->Delete();
- vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkMetaImageReader::New()));
+ // vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkMetaImageReader::New()));
reader->Delete();
- vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkTGAReader::New()));
+ // vtkImageReader2Factory::AvailableReaders->AddItem((reader = vtkTGAReader::New()));
reader->Delete();
}

153 changes: 153 additions & 0 deletions tools/workspace/vtk_internal/repository.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
load(
"@drake//tools/workspace:github.bzl",
"setup_github_repository",
)
load(
"@drake//tools/workspace:execute.bzl",
"execute_or_fail",
)

def parse_module(repo_ctx, subdir):
"""Parses and returns a vtk.module file as a dict.
An upstream `Foo/Bar/vtk.module` file is formatted like this:
NAME
VTK::FooBar
THIRD_PARTY
DEPENDS
VTK::CommonCore
VTK::CommonDataModel
Our returned dict will look like this:
{
"subdir": "Foo/Bar",
"NAME": "VTK::FooBar",
"THIRD_PARTY": 1,
"DEPENDS: [
"VTK::CommonCore",
"VTK::CommonDataModel"
],
}
Note that even if a list-valued field like "DEPENDS" only has one item in
a given module file, we still parse it as a one-item list so that our code
that consumes the information can iterate over it without any hassle.
Internally, to make it easier to parse each dict item as a single line,
we'll munge the file contents to have one line per dict key:
NAME=VTK::FooBar
THIRD_PARTY
DEPENDS=VTK::CommonCore=VTK::CommonDataModel
"""

result = dict(subdir = subdir)
content = repo_ctx.read(subdir + "/vtk.module")
lines = content.replace("\n ", "=").splitlines()
for line in lines:
tokens = line.split("=")
key, values = tokens[0], tokens[1:]

# To date, VTK upstream is consistent that key names ending in "S" are
# list-valued, and key names not ending in "S" are not list-valued. If
# they ever break that pattern, we'll need a lookup table here instead
# of this heuristic.
parse_as_list = key.endswith("S")

if parse_as_list:
result[key] = values
elif len(values) == 1:
result[key] = values[0]
elif len(values) == 0:
# This a a boolean-like item (either present, or not).
# We'll encode that as an int to avoid JSON encoding snafus.
result[key] = 1
else:
fail("{}: Too may values for {}".format(subdir, key))
return result

def create_modules_bzl(repo_ctx):
"""Finds all vtk.module files, parses them, and writes their content into
an loadable `modules.bzl` file in the root of the repository.
This is necessary because BUILD files can't parse external metadata as part
of their rules; the only thing they can do is load `*.bzl` files, so we
must convert the module metadata to bzl.
"""

# Find all vtk.module files.
subdirs = []
for line in execute_or_fail(
repo_ctx,
["/usr/bin/find", ".", "-name", "vtk.module"],
).stdout.splitlines():
# Remove the leading "./" and tailing "/vtk.module".
subdir = line[2:-11]
subdirs.append(subdir)

# Parse all vtk.module files.
modules = dict()
for subdir in subdirs:
content = parse_module(repo_ctx, subdir)
modules[content["NAME"]] = content

# Encode the output. Because we lean on json encoding, this will not work
# correctly if anything in the data structure is None, True, or False.
# We rely on parse_module() to avoid that situation.
bzl_content = "MODULES = " + json.encode(modules) + "\n"

# Pass along the os.name and os.arch for convenience.
platform = dict(name = repo_ctx.os.name, arch = repo_ctx.os.arch)
bzl_content += "PLATFORM = " + json.encode(platform) + "\n"

# Write the output.
repo_ctx.file("modules.bzl", content = bzl_content)

def _impl(repo_ctx):
error = setup_github_repository(repo_ctx).error
if error != None:
fail(error)
create_modules_bzl(repo_ctx)
repo_ctx.symlink(repo_ctx.attr.settings_bzl, "settings.bzl")

vtk_internal_repository = repository_rule(
attrs = {
# These are the attributes for setup_github_repository.
"repository": attr.string(
default = "Kitware/VTK",
),
"commit": attr.string(
# TODO(jwnimmer-tri) Once there's a tagged release with support
# for VTK_ABI_NAMESPACE, we should switch to an official version
# number here. That probably means waiting for the VTK 10 release.
default = "d706250a1422ae1e7ece0fa09a510186769a5fec",
),
"commit_pin": attr.int(
# See above. There's not any satisfactory tagged version yet.
default = 1,
),
"sha256": attr.string(
default = "6106493c8a6e9bd36250e80e4441a1644cd9bff706e6171607f996f0233f515c", # noqa
),
"build_file": attr.label(
default = "@drake//tools/workspace/vtk_internal:package.BUILD.bazel", # noqa
),
"patches": attr.label_list(
default = [
"@drake//tools/workspace/vtk_internal:patches/common_core_version.patch", # noqa
"@drake//tools/workspace/vtk_internal:patches/common_core_vs_data_model_cycle.patch", # noqa
"@drake//tools/workspace/vtk_internal:patches/common_core_warnings.patch", # noqa
"@drake//tools/workspace/vtk_internal:patches/common_data_model_warnings.patch", # noqa
"@drake//tools/workspace/vtk_internal:patches/io_image_formats.patch", # noqa
],
),
"extra_strip_prefix": attr.string(),
"mirrors": attr.string_list_dict(),
# This attribute is specific to our rule, not setup_github_repository.
"settings_bzl": attr.label(
allow_single_file = True,
default = Label("@drake//tools/workspace/vtk_internal:settings.bzl"), # noqa
),
},
implementation = _impl,
)
Loading

0 comments on commit 5a5968b

Please sign in to comment.