Skip to content

Commit c23f8d9

Browse files
jwnimmer-triDuy-Nguyen Ta
authored andcommitted
gl_renderer: Do not install private headers (RobotLocomotion#13807)
Split the renderer out from the factory, for clarity.
1 parent aab6d3f commit c23f8d9

2 files changed

Lines changed: 67 additions & 29 deletions

File tree

geometry/render/gl_renderer/BUILD.bazel

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,30 @@ load(
1515
"drake_cc_package_library_gl_per_os",
1616
)
1717

18-
package(default_visibility = ["//geometry/render/gl_renderer:__subpackages__"])
18+
# This gl_renderer package is only implemented on Ubuntu. For macOS, only the
19+
# render_engine_gl_factory is available and it will always throw an exception.
20+
# For Ubuntu, the factory is the sole public entry point, even though the
21+
# implementation is made up of several other distinct components.
22+
#
23+
# Because the components are only conditionally available, and because we do
24+
# not want Drake's installed headers to depend on GL headers, we only install
25+
# the header for render_engine_gl_factory and nothing else.
26+
#
27+
# Similarly, only the package-level library //geometry/render/gl_renderer is
28+
# public as a Bazel target; all of the other targets are private.
29+
30+
package(default_visibility = ["//visibility:private"])
1931

2032
drake_cc_package_library_gl_per_os(
2133
name = "gl_renderer",
2234
macos_deps = [
23-
":render_engine_gl",
35+
":render_engine_gl_factory",
2436
],
2537
ubuntu_deps = [
2638
":opengl_context",
2739
":opengl_geometry",
2840
":render_engine_gl",
41+
":render_engine_gl_factory",
2942
":shader_program",
3043
":shape_meshes",
3144
],
@@ -58,35 +71,46 @@ drake_cc_library_gl_ubuntu_only(
5871
],
5972
)
6073

61-
# The pure OpenGL-based render engine implementation.
62-
drake_cc_library(
74+
drake_cc_library_gl_ubuntu_only(
6375
name = "render_engine_gl",
76+
srcs = [
77+
"render_engine_gl.cc",
78+
],
79+
hdrs = [
80+
"buffer_dim.h",
81+
"render_engine_gl.h",
82+
],
83+
deps = [
84+
":opengl_context",
85+
":opengl_geometry",
86+
":shader_program",
87+
":shape_meshes",
88+
"//common:essential",
89+
"//geometry/render:render_engine",
90+
"//geometry/render:render_label",
91+
"//systems/sensors:image",
92+
],
93+
)
94+
95+
drake_cc_library(
96+
name = "render_engine_gl_factory",
6497
srcs = select({
65-
"//tools/cc_toolchain:apple": ["no_render_engine_gl_factory.cc"],
66-
"//conditions:default": [
67-
"render_engine_gl.cc",
68-
"render_engine_gl_factory.cc",
98+
"//tools/cc_toolchain:apple": [
99+
"no_render_engine_gl_factory.cc",
69100
],
70-
}),
71-
hdrs = select({
72-
"//tools/cc_toolchain:apple": ["render_engine_gl_factory.h"],
73101
"//conditions:default": [
74-
"buffer_dim.h",
75-
"render_engine_gl.h",
76-
"render_engine_gl_factory.h",
102+
"render_engine_gl_factory.cc",
77103
],
78104
}),
105+
hdrs = [
106+
"render_engine_gl_factory.h",
107+
],
79108
deps = select({
80-
"//tools/cc_toolchain:apple": ["//geometry/render:render_engine"],
81-
"//conditions:default": [
82-
":opengl_context",
83-
":opengl_geometry",
84-
":shader_program",
85-
":shape_meshes",
86-
"//common:essential",
109+
"//tools/cc_toolchain:apple": [
87110
"//geometry/render:render_engine",
88-
"//geometry/render:render_label",
89-
"//systems/sensors:image",
111+
],
112+
"//conditions:default": [
113+
":render_engine_gl",
90114
],
91115
}),
92116
)
@@ -168,7 +192,7 @@ drake_cc_googletest(
168192
"//conditions:default": [],
169193
}),
170194
deps = [
171-
":render_engine_gl",
195+
":render_engine_gl_factory",
172196
"//common/test_utilities:expect_throws_message",
173197
],
174198
)
@@ -200,9 +224,6 @@ drake_cc_googletest_gl_ubuntu_only(
200224
add_lint_tests(
201225
cpplint_extra_srcs = [
202226
"no_render_engine_gl_factory.cc",
203-
"render_engine_gl.cc",
204-
"render_engine_gl.h",
205227
"render_engine_gl_factory.cc",
206-
"render_engine_gl_factory.h",
207228
],
208229
)

geometry/render/gl_renderer/defs.bzl

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,34 @@ load(
1010
)
1111

1212
def drake_cc_googletest_gl_ubuntu_only(**kwargs):
13+
"""Declares a drake_cc_googletest iff we are building on Ubuntu.
14+
Otherwise, does nothing.
15+
"""
1316
if DISTRIBUTION == "ubuntu":
1417
drake_cc_googletest(**kwargs)
1518

16-
def drake_cc_library_gl_ubuntu_only(**kwargs):
19+
def drake_cc_library_gl_ubuntu_only(name, hdrs = [], **kwargs):
20+
"""Declares a drake_cc_library iff we are building on Ubuntu.
21+
Otherwise, does nothing.
22+
"""
1723
if DISTRIBUTION == "ubuntu":
18-
drake_cc_library(**kwargs)
24+
# Because this library is not cross-platform, we must use default
25+
# visibility (i.e., private) and not install its private headers.
26+
drake_cc_library(
27+
name = name,
28+
hdrs = hdrs,
29+
install_hdrs_exclude = hdrs,
30+
visibility = None,
31+
**kwargs
32+
)
1933

2034
def drake_cc_package_library_gl_per_os(
2135
macos_deps = [],
2236
ubuntu_deps = [],
2337
**kwargs):
38+
"""Declares a drake_cc_package_library, where the deps of the library are
39+
conditioned on whether we are building on macOS or Ubuntu.
40+
"""
2441
if DISTRIBUTION == "macos":
2542
drake_cc_package_library(deps = macos_deps, **kwargs)
2643
elif DISTRIBUTION == "ubuntu":

0 commit comments

Comments
 (0)