From 096497165270f5634c28cd68861c6fb447094e7b Mon Sep 17 00:00:00 2001 From: purvisa-at-google-com <51710753+purvisa-at-google-com@users.noreply.github.com> Date: Tue, 30 Aug 2022 16:18:54 +0100 Subject: [PATCH] Fix build system and errors for Sunrise code (#1192) * Fix build system and errors for Sunrise code * Change dependency specification --- BUILD.bazel | 4 +- replay2/core_utils/non_copyable.h | 5 ++ replay2/handle_remapper/BUILD.bazel | 12 ++++- replay2/memory_remapper/BUILD.bazel | 2 +- replay2/replay_context/replay_context.h | 8 +-- tools/build/rules/vulkan_generator.bzl | 2 +- vulkan_generator/handle_remapper/generator.py | 52 +++++++++---------- 7 files changed, 51 insertions(+), 34 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 3c5d49f5c3..25c8def517 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -456,8 +456,8 @@ test_suite( "//gapis/stringtable/minidown/scanner:go_default_test", "//gapis/stringtable/parser:go_default_test", "//gapis/vertex:go_default_test", - "//replay2/handle_remapper:handle_remapper_test", - "//replay2/memory_remapper:memory_remapper_test", + "//replay2/handle_remapper:handle_remapper_tests", + "//replay2/memory_remapper:memory_remapper_tests", "//test/integration/replay:go_default_test", "//test/integration/service:go_default_test", "//vulkan_generator/tests:test_vulkan_parsing", diff --git a/replay2/core_utils/non_copyable.h b/replay2/core_utils/non_copyable.h index 71ce069271..3797304809 100644 --- a/replay2/core_utils/non_copyable.h +++ b/replay2/core_utils/non_copyable.h @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +#ifndef REPLAY2_CORE_UTILS_NON_COPYABLE_H +#define REPLAY2_CORE_UTILS_NON_COPYABLE_H + namespace agi { namespace replay2 { @@ -24,3 +27,5 @@ class NonCopyable { } // namespace replay2 } // namespace agi + +#endif diff --git a/replay2/handle_remapper/BUILD.bazel b/replay2/handle_remapper/BUILD.bazel index b5460e7698..1ab260d851 100644 --- a/replay2/handle_remapper/BUILD.bazel +++ b/replay2/handle_remapper/BUILD.bazel @@ -19,6 +19,14 @@ basic_vulkan_generator( target = "handle_remapper", ) +cc_import( + name = "handle_remapper_src_hdrs", + hdrs = glob( + [":handle_remapper_src"], + exclude = ["*.cc"], + ), +) + cc_library( name = "handle_remapper", srcs = glob( @@ -34,12 +42,14 @@ cc_library( ), visibility = ["//visibility:public"], deps = [ + "handle_remapper_src_hdrs", "//replay2/core_utils", + "//replay2/vulkan_base", ], ) cc_test( - name = "handle_remapper_test", + name = "handle_remapper_tests", srcs = [":handle_remapper_src"], deps = [ "//replay2/core_utils", diff --git a/replay2/memory_remapper/BUILD.bazel b/replay2/memory_remapper/BUILD.bazel index 4c18602fd2..147e19619b 100644 --- a/replay2/memory_remapper/BUILD.bazel +++ b/replay2/memory_remapper/BUILD.bazel @@ -23,7 +23,7 @@ cc_library( ) cc_test( - name = "memory_remapper_test", + name = "memory_remapper_tests", srcs = glob(["tests/*.cc"]), deps = [ "memory_remapper", diff --git a/replay2/replay_context/replay_context.h b/replay2/replay_context/replay_context.h index 30f7930281..1c6eda9570 100644 --- a/replay2/replay_context/replay_context.h +++ b/replay2/replay_context/replay_context.h @@ -16,17 +16,19 @@ #include "replay2/handle_remapper/handle_remapper.h" #include "replay2/memory_remapper/memory_remapper.h" +#include + namespace agi { namespace replay2 { class ReplayContext : public NonCopyable { public: - ReplayContext(const std::string& replayIdentifier) : replayIdentifier_(replayIdentifier_) {} + ReplayContext(const std::string& replayIdentifier) : replayIdentifier_(replayIdentifier) {} const std::string& getReplayIdentifier() const { return replayIdentifier_; } - HandleRemapper& HandleRemapper() const { return handleRemapper_; } - MemoryRemapper& MemoryRemapper() const { return memoryRemapper_; } + HandleRemapper& getHandleRemapper() { return handleRemapper_; } + MemoryRemapper& getMemoryRemapper() { return memoryRemapper_; } private: std::string replayIdentifier_; diff --git a/tools/build/rules/vulkan_generator.bzl b/tools/build/rules/vulkan_generator.bzl index 0466de2d1d..e5e131c965 100644 --- a/tools/build/rules/vulkan_generator.bzl +++ b/tools/build/rules/vulkan_generator.bzl @@ -9,9 +9,9 @@ def _basic_vulkan_generator_impl(ctx): inputs = [ctx.file._xml], outputs = outs, arguments = [ + ctx.file._xml.path, ctx.attr.target, outs[0].dirname, - ctx.file._xml.path, ], mnemonic = ("".join(["BasicVulkanGenerator", ctx.attr.target])).replace("_", ""), executable = ctx.executable._generator, diff --git a/vulkan_generator/handle_remapper/generator.py b/vulkan_generator/handle_remapper/generator.py index 7638f3f9dc..38af51d31f 100644 --- a/vulkan_generator/handle_remapper/generator.py +++ b/vulkan_generator/handle_remapper/generator.py @@ -193,7 +193,7 @@ def generate_handle_remapper_h(file_path: Path, vulkan_metadata: types.VulkanInf arguments={"captureHandle": "VulkanHandle"})) public_functions.append("") - remapper_class_def = codegen.create_class_definition("VulkanHandleRemapper", + remapper_class_def = codegen.create_class_definition("HandleRemapper", public_inheritance=["NonCopyable"], public_functions=public_functions, public_members=public_members, @@ -230,18 +230,18 @@ def generate_handle_remapper_cpp(file_path: Path, vulkan_metadata: types.VulkanI implgenerator = dispatchable_implgen if dispatchable else nondispatchable_implgen add_definition = codegen.create_function_definition( - f"""VulkanHandleRemapper::{handle_add_name(handle)}""", + f"""HandleRemapper::{handle_add_name(handle)}""", arguments={"captureHandle": "VulkanHandle", "replayHandle": "VulkanHandle"}, code=implgenerator.handle_add_code(handle)) remove_definition = codegen.create_function_definition( - f"""VulkanHandleRemapper::{handle_remove_name(handle)}""", + f"""HandleRemapper::{handle_remove_name(handle)}""", arguments={"captureHandle": "VulkanHandle"}, code=implgenerator.handle_remove_code(handle)) remap_definition = codegen.create_function_definition( - f"""VulkanHandleRemapper::{handle_remap_name(handle)}""", + f"""HandleRemapper::{handle_remap_name(handle)}""", arguments={"captureHandle": "VulkanHandle"}, return_type="VulkanHandle", code=implgenerator.handle_remap_code(handle)) @@ -275,51 +275,51 @@ def generate_handle_remapper_tests(file_path: Path, vulkan_metadata: types.Vulka for handle in vulkan_metadata.types.handles: tests_cpp.write(dedent(f""" - TEST(VulkanHandleRemapper, {handle}BasicRemap) {{ - VulkanHandleRemapper mapper; - EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), VulkanHandleRemapper::RemapNonExistantHandleException); + TEST(HandleRemapper, {handle}BasicRemap) {{ + HandleRemapper mapper; + EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), HandleRemapper::RemapNonExistantHandleException); EXPECT_NO_THROW(mapper.{handle_add_name(handle)}(1234, 5678)); EXPECT_NO_THROW(EXPECT_EQ(mapper.{handle_remap_name(handle)}(1234), 5678)); EXPECT_NO_THROW(mapper.{handle_remove_name(handle)}(1234)); - EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), VulkanHandleRemapper::RemapNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), HandleRemapper::RemapNonExistantHandleException); }}""")) tests_cpp.write(dedent(f""" - TEST(VulkanHandleRemapper, {handle}UnknownRemap) {{ - VulkanHandleRemapper mapper; + TEST(HandleRemapper, {handle}UnknownRemap) {{ + HandleRemapper mapper; EXPECT_NO_THROW(mapper.{handle_add_name(handle)}(1234, 5678)); - EXPECT_THROW(mapper.{handle_remap_name(handle)}(5678), VulkanHandleRemapper::RemapNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remap_name(handle)}(5678), HandleRemapper::RemapNonExistantHandleException); }}""")) if vulkan_metadata.types.handles[handle].dispatchable: tests_cpp.write(dedent(f""" - TEST(VulkanHandleRemapper, Dispatchable{handle}Redefinition) {{ - VulkanHandleRemapper mapper; + TEST(HandleRemapper, Dispatchable{handle}Redefinition) {{ + HandleRemapper mapper; EXPECT_NO_THROW(mapper.{handle_add_name(handle)}(1234, 5678)); - EXPECT_THROW(mapper.{handle_add_name(handle)}(1234, 5678), VulkanHandleRemapper::HandleCollisionException); - EXPECT_THROW(mapper.{handle_add_name(handle)}(1234, 8765), VulkanHandleRemapper::HandleCollisionException); + EXPECT_THROW(mapper.{handle_add_name(handle)}(1234, 5678), HandleRemapper::HandleCollisionException); + EXPECT_THROW(mapper.{handle_add_name(handle)}(1234, 8765), HandleRemapper::HandleCollisionException); EXPECT_NO_THROW(EXPECT_EQ(mapper.{handle_remap_name(handle)}(1234), 5678)); EXPECT_NO_THROW(mapper.{handle_remove_name(handle)}(1234)); - EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), VulkanHandleRemapper::RemapNonExistantHandleException); - EXPECT_THROW(mapper.{handle_remove_name(handle)}(1234), VulkanHandleRemapper::RemoveNonExistantHandleException); - EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), VulkanHandleRemapper::RemapNonExistantHandleException); - EXPECT_THROW(mapper.{handle_remove_name(handle)}(1234), VulkanHandleRemapper::RemoveNonExistantHandleException); - EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), VulkanHandleRemapper::RemapNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), HandleRemapper::RemapNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remove_name(handle)}(1234), HandleRemapper::RemoveNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), HandleRemapper::RemapNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remove_name(handle)}(1234), HandleRemapper::RemoveNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), HandleRemapper::RemapNonExistantHandleException); }}""")) else: tests_cpp.write(dedent(f""" - TEST(VulkanHandleRemapper, NonDispatchable{handle}Redefinition) {{ - VulkanHandleRemapper mapper; + TEST(HandleRemapper, NonDispatchable{handle}Redefinition) {{ + HandleRemapper mapper; EXPECT_NO_THROW(mapper.{handle_add_name(handle)}(1234, 5678)); EXPECT_NO_THROW(mapper.{handle_add_name(handle)}(1234, 5678)); - EXPECT_THROW(mapper.{handle_add_name(handle)}(1234, 8765), VulkanHandleRemapper::NonDispatchableHandleRedefinitionException); + EXPECT_THROW(mapper.{handle_add_name(handle)}(1234, 8765), HandleRemapper::NonDispatchableHandleRedefinitionException); EXPECT_NO_THROW(EXPECT_EQ(mapper.{handle_remap_name(handle)}(1234), 5678)); EXPECT_NO_THROW(mapper.{handle_remove_name(handle)}(1234)); EXPECT_NO_THROW(EXPECT_EQ(mapper.{handle_remap_name(handle)}(1234), 5678)); EXPECT_NO_THROW(mapper.{handle_remove_name(handle)}(1234)); - EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), VulkanHandleRemapper::RemapNonExistantHandleException); - EXPECT_THROW(mapper.{handle_remove_name(handle)}(1234), VulkanHandleRemapper::RemoveNonExistantHandleException); - EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), VulkanHandleRemapper::RemapNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), HandleRemapper::RemapNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remove_name(handle)}(1234), HandleRemapper::RemoveNonExistantHandleException); + EXPECT_THROW(mapper.{handle_remap_name(handle)}(1234), HandleRemapper::RemapNonExistantHandleException); }}""")) tests_cpp.write(dedent("""