Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[workspace] Add vtk_internal build from source #20068

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -105,6 +105,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 @@ -355,6 +356,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();
}

159 changes: 159 additions & 0 deletions tools/workspace/vtk_internal/repository.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
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(("vtk/{subdir}/vtk.module: Got multiple values for {key} " +
"but we assumed (because its name ended with 'S') that it " +
"was not supposed to be a list").format(
subdir = subdir,
key = key,
))

return result

def create_modules_bzl(repo_ctx):
"""Finds all vtk.module files, parses them, and writes their content into
a 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