From 8fc8baa5792cbdabfdc8581db994c203556c52b9 Mon Sep 17 00:00:00 2001 From: Ken Pruiksma Date: Wed, 6 Oct 2021 17:03:03 -0500 Subject: [PATCH] Terrain FP now pulls data from Terrain system (#4492) * Feature processor now pulls data instead of the render component pushing it. This results in fewer and cheaper heightmap rebuilds. The feature processor can also handle dirty regions correctly now, although it doesn't seem like they are being passed in correctly yet. Signed-off-by: Ken Pruiksma * Fixing issues with initialization, dirty region tracking, and total rendered world size. Signed-off-by: Ken Pruiksma * Fixing bug with resizing the world Signed-off-by: Ken Pruiksma * Decreasing the scope of a mutex Signed-off-by: Ken Pruiksma * Fixes from PR review Signed-off-by: Ken Pruiksma * Fixed a math issue with float rounding. Fixed static AZ::Name usage. Signed-off-by: Ken Pruiksma * Removing unused variable Signed-off-by: Ken Pruiksma --- .../TerrainWorldRendererComponent.cpp | 71 -------- .../TerrainWorldRendererComponent.h | 5 - .../TerrainFeatureProcessor.cpp | 167 +++++++++++++----- .../TerrainRenderer/TerrainFeatureProcessor.h | 49 ++--- 4 files changed, 144 insertions(+), 148 deletions(-) diff --git a/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.cpp b/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.cpp index f3eed41717..9cd9ce9fb2 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.cpp +++ b/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.cpp @@ -13,8 +13,6 @@ #include #include -#include - #include #include #include @@ -135,17 +133,13 @@ namespace Terrain { m_terrainFeatureProcessor = scene->EnableFeatureProcessor(); } - - AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusConnect(); m_terrainRendererActive = true; } void TerrainWorldRendererComponent::Deactivate() { // On component deactivation, unregister the feature processor and remove it from the default scene. - m_terrainRendererActive = false; - AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusDisconnect(); if (AZ::RPI::Scene* scene = GetScene(); scene) { @@ -178,69 +172,4 @@ namespace Terrain } return false; } - - void TerrainWorldRendererComponent::OnTerrainDataDestroyBegin() - { - // If the terrain is being destroyed, remove all existing terrain data from the feature processor. - - if (m_terrainFeatureProcessor) - { - m_terrainFeatureProcessor->RemoveTerrainData(); - } - } - - void TerrainWorldRendererComponent::OnTerrainDataChanged([[maybe_unused]] const AZ::Aabb& dirtyRegion, [[maybe_unused]] TerrainDataChangedMask dataChangedMask) - { - // Block other threads from accessing the surface data bus while we are in GetValue (which may call into the SurfaceData bus). - // We lock our surface data mutex *before* checking / setting "isRequestInProgress" so that we prevent race conditions - // that create false detection of cyclic dependencies when multiple requests occur on different threads simultaneously. - // (One case where this was previously able to occur was in rapid updating of the Preview widget on the - // GradientSurfaceDataComponent in the Editor when moving the threshold sliders back and forth rapidly) - auto& surfaceDataContext = SurfaceData::SurfaceDataSystemRequestBus::GetOrCreateContext(false); - typename SurfaceData::SurfaceDataSystemRequestBus::Context::DispatchLockGuard scopeLock(surfaceDataContext.m_contextMutex); - - AZ::Vector2 queryResolution = AZ::Vector2(1.0f); - AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult( - queryResolution, &AzFramework::Terrain::TerrainDataRequests::GetTerrainHeightQueryResolution); - - AZ::Aabb worldBounds = AZ::Aabb::CreateNull(); - AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult( - worldBounds, &AzFramework::Terrain::TerrainDataRequests::GetTerrainAabb); - - - AZ::Transform transform = AZ::Transform::CreateTranslation(worldBounds.GetCenter()); - - uint32_t width = aznumeric_cast( - (float)worldBounds.GetXExtent() / queryResolution.GetX()); - uint32_t height = aznumeric_cast( - (float)worldBounds.GetYExtent() / queryResolution.GetY()); - AZStd::vector pixels; - pixels.resize_no_construct(width * height); - const uint32_t pixelDataSize = width * height * sizeof(float); - memset(pixels.data(), 0, pixelDataSize); - - for (uint32_t y = 0; y < height; y++) - { - for (uint32_t x = 0; x < width; x++) - { - bool terrainExists = true; - float terrainHeight = 0.0f; - AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult( - terrainHeight, &AzFramework::Terrain::TerrainDataRequests::GetHeightFromFloats, - (x * queryResolution.GetX()) + worldBounds.GetMin().GetX(), - (y * queryResolution.GetY()) + worldBounds.GetMin().GetY(), - AzFramework::Terrain::TerrainDataRequests::Sampler::EXACT, - &terrainExists); - - pixels[(y * width) + x] = - (terrainHeight - worldBounds.GetMin().GetZ()) / worldBounds.GetExtents().GetZ(); - } - } - - if (m_terrainFeatureProcessor) - { - m_terrainFeatureProcessor->UpdateTerrainData(transform, worldBounds, queryResolution.GetX(), width, height, pixels); - } - } - } diff --git a/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.h b/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.h index cd2a4ea5aa..354b2fde34 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.h +++ b/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.h @@ -9,8 +9,6 @@ #pragma once #include -#include -#include namespace LmbrCentral { @@ -54,7 +52,6 @@ namespace Terrain class TerrainWorldRendererComponent : public AZ::Component - , public AzFramework::Terrain::TerrainDataNotificationBus::Handler { public: template @@ -77,8 +74,6 @@ namespace Terrain bool WriteOutConfig(AZ::ComponentConfig* outBaseConfig) const override; protected: - void OnTerrainDataDestroyBegin() override; - void OnTerrainDataChanged(const AZ::Aabb& dirtyRegion, TerrainDataChangedMask dataChangedMask) override; AZ::RPI::Scene* GetScene() const; diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp index 59984a1b92..560fe1eb60 100644 --- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp +++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp @@ -10,40 +10,41 @@ #include #include +#include #include #include +#include #include #include -#include +#include + #include #include #include #include -#include -#include #include #include -#include +#include #include #include + #include #include #include #include -#include -#include -#include -#include -#include + #include +#include + namespace Terrain { namespace { [[maybe_unused]] const char* TerrainFPName = "TerrainFeatureProcessor"; + const char* TerrainHeightmapChars = "TerrainHeightmap"; } namespace MaterialInputs @@ -71,7 +72,9 @@ namespace Terrain void TerrainFeatureProcessor::Activate() { m_areaData = {}; + m_dirtyRegion = AZ::Aabb::CreateNull(); Initialize(); + AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusConnect(); } void TerrainFeatureProcessor::Initialize() @@ -99,13 +102,16 @@ namespace Terrain AZ_Error(TerrainFPName, false, "Failed to create Terrain render buffers!"); return; } + OnTerrainDataChanged(AZ::Aabb::CreateNull(), TerrainDataChangedMask::HeightData); } void TerrainFeatureProcessor::Deactivate() { + AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusDisconnect(); + AZ::RPI::MaterialReloadNotificationBus::Handler::BusDisconnect(); + m_patchModel = {}; m_areaData = {}; - AZ::RPI::MaterialReloadNotificationBus::Handler::BusDisconnect(); } void TerrainFeatureProcessor::Render(const AZ::RPI::FeatureProcessor::RenderPacket& packet) @@ -113,51 +119,126 @@ namespace Terrain ProcessSurfaces(packet); } - void TerrainFeatureProcessor::UpdateTerrainData( - const AZ::Transform& transform, - const AZ::Aabb& worldBounds, - float sampleSpacing, - uint32_t width, uint32_t height, const AZStd::vector& heightData) + void TerrainFeatureProcessor::OnTerrainDataDestroyBegin() { - if (!worldBounds.IsValid()) + m_areaData = {}; + } + + void TerrainFeatureProcessor::OnTerrainDataChanged(const AZ::Aabb& dirtyRegion, TerrainDataChangedMask dataChangedMask) + { + if (dataChangedMask != TerrainDataChangedMask::HeightData && dataChangedMask != TerrainDataChangedMask::Settings) { return; } + AZ::Aabb worldBounds = AZ::Aabb::CreateNull(); + AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult( + worldBounds, &AzFramework::Terrain::TerrainDataRequests::GetTerrainAabb); + + const AZ::Aabb& regionToUpdate = dirtyRegion.IsValid() ? dirtyRegion : worldBounds; + + m_dirtyRegion.AddAabb(regionToUpdate); + m_dirtyRegion.Clamp(worldBounds); + + AZ::Transform transform = AZ::Transform::CreateTranslation(worldBounds.GetCenter()); + + AZ::Vector2 queryResolution = AZ::Vector2(1.0f); + AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult( + queryResolution, &AzFramework::Terrain::TerrainDataRequests::GetTerrainHeightQueryResolution); + m_areaData.m_transform = transform; m_areaData.m_heightScale = worldBounds.GetZExtent(); m_areaData.m_terrainBounds = worldBounds; - m_areaData.m_heightmapImageHeight = height; - m_areaData.m_heightmapImageWidth = width; - m_areaData.m_sampleSpacing = sampleSpacing; + m_areaData.m_heightmapImageWidth = aznumeric_cast(worldBounds.GetXExtent() / queryResolution.GetX()); + m_areaData.m_heightmapImageHeight = aznumeric_cast(worldBounds.GetYExtent() / queryResolution.GetY()); + m_areaData.m_updateWidth = aznumeric_cast(m_dirtyRegion.GetXExtent() / queryResolution.GetX()); + m_areaData.m_updateHeight = aznumeric_cast(m_dirtyRegion.GetYExtent() / queryResolution.GetY()); + // Currently query resolution is multidimensional but the rendering system only supports this changing in one dimension. + m_areaData.m_sampleSpacing = queryResolution.GetX(); + m_areaData.m_propertiesDirty = true; + } - // Create heightmap image data + void TerrainFeatureProcessor::UpdateTerrainData() + { + static const AZ::Name TerrainHeightmapName = AZ::Name(TerrainHeightmapChars); + + uint32_t width = m_areaData.m_updateWidth; + uint32_t height = m_areaData.m_updateHeight; + const AZ::Aabb& worldBounds = m_areaData.m_terrainBounds; + float queryResolution = m_areaData.m_sampleSpacing; + + AZ::RHI::Size worldSize = AZ::RHI::Size(m_areaData.m_heightmapImageWidth, m_areaData.m_heightmapImageHeight, 1); + + if (!m_areaData.m_heightmapImage || m_areaData.m_heightmapImage->GetDescriptor().m_size != worldSize) { - m_areaData.m_propertiesDirty = true; + // World size changed, so the whole world needs updating. + width = worldSize.m_width; + height = worldSize.m_height; + m_dirtyRegion = worldBounds; - AZ::RHI::Size imageSize; - imageSize.m_width = width; - imageSize.m_height = height; - - AZStd::vector uint16Heights; - uint16Heights.reserve(heightData.size()); - for (float sampleHeight : heightData) - { - float clampedSample = AZ::GetClamp(sampleHeight, 0.0f, 1.0f); - constexpr uint16_t MaxUint16 = 0xFFFF; - uint16Heights.push_back(aznumeric_cast(clampedSample * MaxUint16)); - } - - AZ::Data::Instance streamingImagePool = AZ::RPI::ImageSystemInterface::Get()->GetSystemStreamingPool(); - m_areaData.m_heightmapImage = AZ::RPI::StreamingImage::CreateFromCpuData(*streamingImagePool, - AZ::RHI::ImageDimension::Image2D, - imageSize, - AZ::RHI::Format::R16_UNORM, - (uint8_t*)uint16Heights.data(), - heightData.size() * sizeof(uint16_t)); + AZ::Data::Instance imagePool = AZ::RPI::ImageSystemInterface::Get()->GetSystemAttachmentPool(); + AZ::RHI::ImageDescriptor imageDescriptor = AZ::RHI::ImageDescriptor::Create2D( + AZ::RHI::ImageBindFlags::ShaderRead, width, height, AZ::RHI::Format::R16_UNORM + ); + m_areaData.m_heightmapImage = AZ::RPI::AttachmentImage::Create(*imagePool.get(), imageDescriptor, TerrainHeightmapName, nullptr, nullptr); AZ_Error(TerrainFPName, m_areaData.m_heightmapImage, "Failed to initialize the heightmap image!"); } + AZStd::vector pixels; + pixels.reserve(width * height); + + { + // Block other threads from accessing the surface data bus while we are in GetHeightFromFloats (which may call into the SurfaceData bus). + // We lock our surface data mutex *before* checking / setting "isRequestInProgress" so that we prevent race conditions + // that create false detection of cyclic dependencies when multiple requests occur on different threads simultaneously. + // (One case where this was previously able to occur was in rapid updating of the Preview widget on the + // GradientSurfaceDataComponent in the Editor when moving the threshold sliders back and forth rapidly) + + auto& surfaceDataContext = SurfaceData::SurfaceDataSystemRequestBus::GetOrCreateContext(false); + typename SurfaceData::SurfaceDataSystemRequestBus::Context::DispatchLockGuard scopeLock(surfaceDataContext.m_contextMutex); + + for (uint32_t y = 0; y < height; y++) + { + for (uint32_t x = 0; x < width; x++) + { + bool terrainExists = true; + float terrainHeight = 0.0f; + AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult( + terrainHeight, &AzFramework::Terrain::TerrainDataRequests::GetHeightFromFloats, + (x * queryResolution) + m_dirtyRegion.GetMin().GetX(), + (y * queryResolution) + m_dirtyRegion.GetMin().GetY(), + AzFramework::Terrain::TerrainDataRequests::Sampler::EXACT, + &terrainExists); + + float clampedHeight = AZ::GetClamp((terrainHeight - worldBounds.GetMin().GetZ()) / worldBounds.GetExtents().GetZ(), 0.0f, 1.0f); + float expandedHeight = AZStd::roundf(clampedHeight * AZStd::numeric_limits::max()); + uint16_t uint16Height = aznumeric_cast(expandedHeight); + + pixels.push_back(uint16Height); + } + } + } + + if (m_areaData.m_heightmapImage) + { + const float left = (m_dirtyRegion.GetMin().GetX() - worldBounds.GetMin().GetX()) / queryResolution; + const float top = (m_dirtyRegion.GetMin().GetY() - worldBounds.GetMin().GetY()) / queryResolution; + AZ::RHI::ImageUpdateRequest imageUpdateRequest; + imageUpdateRequest.m_imageSubresourcePixelOffset.m_left = aznumeric_cast(left); + imageUpdateRequest.m_imageSubresourcePixelOffset.m_top = aznumeric_cast(top); + imageUpdateRequest.m_sourceSubresourceLayout.m_bytesPerRow = width * sizeof(uint16_t); + imageUpdateRequest.m_sourceSubresourceLayout.m_bytesPerImage = width * height * sizeof(uint16_t); + imageUpdateRequest.m_sourceSubresourceLayout.m_rowCount = height; + imageUpdateRequest.m_sourceSubresourceLayout.m_size.m_width = width; + imageUpdateRequest.m_sourceSubresourceLayout.m_size.m_height = height; + imageUpdateRequest.m_sourceSubresourceLayout.m_size.m_depth = 1; + imageUpdateRequest.m_sourceData = pixels.data(); + imageUpdateRequest.m_image = m_areaData.m_heightmapImage->GetRHIImage(); + + m_areaData.m_heightmapImage->UpdateImageContents(imageUpdateRequest); + } + + m_dirtyRegion = AZ::Aabb::CreateNull(); } void TerrainFeatureProcessor::ProcessSurfaces(const FeatureProcessor::RenderPacket& process) @@ -169,8 +250,10 @@ namespace Terrain return; } - if (m_areaData.m_propertiesDirty && m_materialInstance) + if (m_areaData.m_propertiesDirty && m_materialInstance && m_materialInstance->CanCompile()) { + UpdateTerrainData(); + m_areaData.m_propertiesDirty = false; m_sectorData.clear(); diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h index 32f88565c2..d8df9b328c 100644 --- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h +++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h @@ -9,24 +9,12 @@ #pragma once #include -#include -#include -#include + +#include #include -#include -#include - -#include +#include #include -#include -#include -#include -#include -#include -#include -#include -#include #include namespace AZ::RPI @@ -35,6 +23,7 @@ namespace AZ::RPI { class AsyncAssetLoader; } + class Material; class Model; } @@ -43,6 +32,7 @@ namespace Terrain class TerrainFeatureProcessor final : public AZ::RPI::FeatureProcessor , private AZ::RPI::MaterialReloadNotificationBus::Handler + , private AzFramework::Terrain::TerrainDataNotificationBus::Handler { public: AZ_RTTI(TerrainFeatureProcessor, "{D7DAC1F9-4A9F-4D3C-80AE-99579BF8AB1C}", AZ::RPI::FeatureProcessor); @@ -54,26 +44,13 @@ namespace Terrain TerrainFeatureProcessor() = default; ~TerrainFeatureProcessor() = default; - // AZ::Component overrides... + // AZ::RPI::FeatureProcessor overrides... void Activate() override; void Deactivate() override; - - // AZ::RPI::FeatureProcessor overrides... void Render(const AZ::RPI::FeatureProcessor::RenderPacket& packet) override; - // AZ::RPI::MaterialReloadNotificationBus::Handler overrides... - void OnMaterialReinitialized(const AZ::Data::Instance& material) override; - void SetWorldSize(AZ::Vector2 sizeInMeters); - void UpdateTerrainData(const AZ::Transform& transform, const AZ::Aabb& worldBounds, float sampleSpacing, - uint32_t width, uint32_t height, const AZStd::vector& heightData); - - void RemoveTerrainData() - { - m_areaData = {}; - } - private: struct ShaderTerrainData // Must align with struct in Object Srg @@ -104,10 +81,19 @@ namespace Terrain AZStd::vector m_indices; }; + // AZ::RPI::MaterialReloadNotificationBus::Handler overrides... + void OnMaterialReinitialized(const AZ::Data::Instance& material) override; + + // AzFramework::Terrain::TerrainDataNotificationBus overrides... + void OnTerrainDataDestroyBegin() override; + void OnTerrainDataChanged(const AZ::Aabb& dirtyRegion, TerrainDataChangedMask dataChangedMask) override; + void Initialize(); void InitializeTerrainPatch(uint16_t gridSize, float gridSpacing, PatchData& patchdata); bool InitializePatchModel(); + void UpdateTerrainData(); + void ProcessSurfaces(const FeatureProcessor::RenderPacket& process); AZ::Outcome> CreateBufferAsset( @@ -132,14 +118,17 @@ namespace Terrain AZ::Transform m_transform{ AZ::Transform::CreateIdentity() }; AZ::Aabb m_terrainBounds{ AZ::Aabb::CreateNull() }; float m_heightScale{ 0.0f }; - AZ::Data::Instance m_heightmapImage; + AZ::Data::Instance m_heightmapImage; uint32_t m_heightmapImageWidth{ 0 }; uint32_t m_heightmapImageHeight{ 0 }; + uint32_t m_updateWidth{ 0 }; + uint32_t m_updateHeight{ 0 }; bool m_propertiesDirty{ true }; float m_sampleSpacing{ 0.0f }; }; TerrainAreaData m_areaData; + AZ::Aabb m_dirtyRegion{ AZ::Aabb::CreateNull() }; struct SectorData {