Mesh loaders example refactor, CI friendly with benchmarks integration#250
Open
AnastaZIuk wants to merge 12 commits intomasterfrom
Open
Mesh loaders example refactor, CI friendly with benchmarks integration#250AnastaZIuk wants to merge 12 commits intomasterfrom
AnastaZIuk wants to merge 12 commits intomasterfrom
Conversation
Comment on lines
+54
to
+66
| include(FetchContent) | ||
| FetchContent_Declare(nbl_meshloaders_benchmark_dataset | ||
| GIT_REPOSITORY "${NBL_MESHLOADERS_BENCHMARK_DATASET_REPO}" | ||
| GIT_TAG "master" | ||
| GIT_SHALLOW TRUE | ||
| GIT_PROGRESS TRUE | ||
| SOURCE_DIR "${NBL_MESHLOADERS_BENCHMARK_DATASET_DIR}" | ||
| BINARY_DIR "${CMAKE_BINARY_DIR}/CMakeFiles/nbl_meshloaders_benchmark_dataset-build" | ||
| ) | ||
| FetchContent_GetProperties(nbl_meshloaders_benchmark_dataset) | ||
| if (NOT nbl_meshloaders_benchmark_dataset_POPULATED) | ||
| FetchContent_Populate(nbl_meshloaders_benchmark_dataset) | ||
| endif () |
There was a problem hiding this comment.
why yet another subrepo and not our media submodule?
Comment on lines
+14
to
+27
| core::blake3_hash_t meshloadersHashBufferLegacySequential(const asset::ICPUBuffer* const buffer) | ||
| { | ||
| if (!buffer) | ||
| return static_cast<core::blake3_hash_t>(core::blake3_hasher{}); | ||
| const auto* const ptr = buffer->getPointer(); | ||
| const size_t size = buffer->getSize(); | ||
| if (!ptr || size == 0ull) | ||
| return static_cast<core::blake3_hash_t>(core::blake3_hasher{}); | ||
| core::blake3_hasher hasher; | ||
| hasher.update(ptr, size); | ||
| return static_cast<core::blake3_hash_t>(hasher); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
what do we need this function for ?
Comment on lines
+104
to
+107
| core::blake3_hash_t MeshLoadersApp::hashGeometry(const ICPUPolygonGeometry* geo) | ||
| { | ||
| return CPolygonGeometryManipulator::computeDeterministicContentHash(geo); | ||
| } |
There was a problem hiding this comment.
this is a bit pointless passthrough func
Comment on lines
+159
to
+208
| core::vector<core::blake3_hash_t> legacySequentialHashes; | ||
| core::vector<core::blake3_hash_t> newSequentialHashes; | ||
| core::vector<core::blake3_hash_t> newParallelHashes; | ||
| legacySequentialHashes.reserve(buffers.size()); | ||
| newSequentialHashes.reserve(buffers.size()); | ||
| newParallelHashes.reserve(buffers.size()); | ||
|
|
||
| for (const auto& buffer : buffers) | ||
| { | ||
| if (!buffer) | ||
| continue; | ||
|
|
||
| const auto* const ptr = buffer->getPointer(); | ||
| const size_t size = buffer->getSize(); | ||
|
|
||
| const auto legacyStart = clock_t::now(); | ||
| const auto legacyHash = meshloadersHashBufferLegacySequential(buffer.get()); | ||
| caseLegacySequentialMs += toMs(clock_t::now() - legacyStart); | ||
| legacySequentialHashes.push_back(legacyHash); | ||
|
|
||
| const auto newSeqStart = clock_t::now(); | ||
| const auto newSeqHash = core::blake3_hash_buffer_sequential(ptr, size); | ||
| caseNewSequentialMs += toMs(clock_t::now() - newSeqStart); | ||
| newSequentialHashes.push_back(newSeqHash); | ||
|
|
||
| const auto newParStart = clock_t::now(); | ||
| const auto newParHash = core::blake3_hash_buffer(ptr, size); | ||
| caseNewParallelMs += toMs(clock_t::now() - newParStart); | ||
| newParallelHashes.push_back(newParHash); | ||
| } | ||
|
|
||
| if (legacySequentialHashes.size() != newSequentialHashes.size() || legacySequentialHashes.size() != newParallelHashes.size()) | ||
| failExit("Hash test buffer count mismatch for %s geo=%llu.", testCase.path.string().c_str(), static_cast<unsigned long long>(geoIx)); | ||
|
|
||
| for (size_t hashIx = 0u; hashIx < legacySequentialHashes.size(); ++hashIx) | ||
| { | ||
| if (legacySequentialHashes[hashIx] == newSequentialHashes[hashIx] && legacySequentialHashes[hashIx] == newParallelHashes[hashIx]) | ||
| continue; | ||
| failExit( | ||
| "Hash mismatch for %s geo=%llu buffer=%llu legacy_seq=%s new_seq=%s new_parallel=%s", | ||
| testCase.path.string().c_str(), | ||
| static_cast<unsigned long long>(geoIx), | ||
| static_cast<unsigned long long>(hashIx), | ||
| geometryHashToHex(legacySequentialHashes[hashIx]).c_str(), | ||
| geometryHashToHex(newSequentialHashes[hashIx]).c_str(), | ||
| geometryHashToHex(newParallelHashes[hashIx]).c_str()); | ||
| } | ||
|
|
||
| caseBufferCount += legacySequentialHashes.size(); | ||
| ++totalGeometryCount; |
There was a problem hiding this comment.
comparing just ICPUBuffer hashing impl should be its own unit test IMHO and can be done with synthetic data
There was a problem hiding this comment.
instead you could just test here that every IPreHashed dependent of the ICPUPolgyonGeometry had a null hash at load time, and after you collected buffers and hashed they were no longer null
| failExit("Hash test failed to access geometry %llu in %s.", static_cast<unsigned long long>(geoIx), testCase.path.string().c_str()); | ||
|
|
||
| core::vector<core::smart_refctd_ptr<ICPUBuffer>> buffers; | ||
| asset::collectGeometryBuffers(geometry, buffers); |
There was a problem hiding this comment.
please no free floating functions
Comment on lines
+353
to
+400
| bool MeshLoadersApp::compareImages(const asset::ICPUImageView* a, const asset::ICPUImageView* b, uint64_t& diffCount, uint8_t& maxDiff) | ||
| { | ||
| diffCount = 0u; | ||
| maxDiff = 0u; | ||
| if (!a || !b) | ||
| return false; | ||
|
|
||
| const auto* imgA = a->getCreationParameters().image.get(); | ||
| const auto* imgB = b->getCreationParameters().image.get(); | ||
| if (!imgA || !imgB) | ||
| return false; | ||
|
|
||
| const auto paramsA = imgA->getCreationParameters(); | ||
| const auto paramsB = imgB->getCreationParameters(); | ||
| if (paramsA.format != paramsB.format) | ||
| return false; | ||
| if (paramsA.extent != paramsB.extent) | ||
| return false; | ||
|
|
||
| const auto* bufA = imgA->getBuffer(); | ||
| const auto* bufB = imgB->getBuffer(); | ||
| if (!bufA || !bufB) | ||
| return false; | ||
|
|
||
| const size_t sizeA = bufA->getSize(); | ||
| if (sizeA != bufB->getSize()) | ||
| return false; | ||
|
|
||
| const auto* dataA = static_cast<const uint8_t*>(bufA->getPointer()); | ||
| const auto* dataB = static_cast<const uint8_t*>(bufB->getPointer()); | ||
| if (!dataA || !dataB) | ||
| return false; | ||
|
|
||
| for (size_t i = 0; i < sizeA; ++i) | ||
| { | ||
| const uint8_t va = dataA[i]; | ||
| const uint8_t vb = dataB[i]; | ||
| const uint8_t diff = va > vb ? static_cast<uint8_t>(va - vb) : static_cast<uint8_t>(vb - va); | ||
| if (diff) | ||
| { | ||
| ++diffCount; | ||
| if (diff > maxDiff) | ||
| maxDiff = diff; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
slap this in common examples header
Comment on lines
+386
to
+391
| for (size_t i = 0; i < sizeA; ++i) | ||
| { | ||
| const uint8_t va = dataA[i]; | ||
| const uint8_t vb = dataB[i]; | ||
| const uint8_t diff = va > vb ? static_cast<uint8_t>(va - vb) : static_cast<uint8_t>(vb - va); | ||
| if (diff) |
There was a problem hiding this comment.
this assumes 8bit per channel format
| return { {m_surface->getSurface()} }; | ||
|
|
||
| return {}; | ||
| } |
There was a problem hiding this comment.
what was wrong with using MonoWindowApplication like we did before ?
Comment on lines
+70
to
+86
| ISwapchain::SCreationParams swapchainParams = { .surface = smart_refctd_ptr<ISurface>(m_surface->getSurface()) }; | ||
| swapchainParams.sharedParams.imageUsage |= IGPUImage::E_USAGE_FLAGS::EUF_TRANSFER_SRC_BIT; | ||
| if (!swapchainParams.deduceFormat(m_physicalDevice)) | ||
| return logFail("Could not choose a Surface Format for the Swapchain!"); | ||
|
|
||
| auto scResources = std::make_unique<CSwapchainFramebuffersAndDepth>(m_device.get(), m_depthFormat, swapchainParams.surfaceFormat.format, getDefaultSubpassDependencies()); | ||
| auto* renderpass = scResources->getRenderpass(); | ||
| if (!renderpass) | ||
| return logFail("Failed to create Renderpass!"); | ||
|
|
||
| auto gQueue = getGraphicsQueue(); | ||
| if (!m_surface || !m_surface->init(gQueue, std::move(scResources), swapchainParams.sharedParams)) | ||
| return logFail("Could not create Window & Surface or initialize the Surface!"); | ||
|
|
||
| m_winMgr->setWindowSize(m_window.get(), m_initialResolution[0], m_initialResolution[1]); | ||
| m_surface->recreateSwapchain(); | ||
| return true; |
There was a problem hiding this comment.
MonoWindowApplication used to do this for you
Comment on lines
+89
to
+141
| void workLoopBody() override final | ||
| { | ||
| const uint32_t framesInFlightCount = hlsl::min(MaxFramesInFlight, m_surface->getMaxAcquiresInFlight()); | ||
| if (m_framesInFlight.size() >= framesInFlightCount) | ||
| { | ||
| const ISemaphore::SWaitInfo framesDone[] = { {.semaphore = m_framesInFlight.front().semaphore.get(), .value = m_framesInFlight.front().value} }; | ||
| if (m_device->blockForSemaphores(framesDone) != ISemaphore::WAIT_RESULT::SUCCESS) | ||
| return; | ||
| m_framesInFlight.pop_front(); | ||
| } | ||
|
|
||
| auto updatePresentationTimestamp = [&]() | ||
| { | ||
| m_currentImageAcquire = m_surface->acquireNextImage(); | ||
| oracle.reportEndFrameRecord(); | ||
| const auto timestamp = oracle.getNextPresentationTimeStamp(); | ||
| oracle.reportBeginFrameRecord(); | ||
| return timestamp; | ||
| }; | ||
|
|
||
| const auto nextPresentationTimestamp = updatePresentationTimestamp(); | ||
| if (!m_currentImageAcquire) | ||
| return; | ||
|
|
||
| const IQueue::SSubmitInfo::SSemaphoreInfo rendered[] = { renderFrame(nextPresentationTimestamp) }; | ||
| m_surface->present(m_currentImageAcquire.imageIndex, rendered); | ||
| if (rendered->semaphore) | ||
| m_framesInFlight.emplace_back(smart_refctd_ptr<ISemaphore>(rendered->semaphore), rendered->value); | ||
| } | ||
|
|
||
| bool keepRunning() override | ||
| { | ||
| if (m_surface->irrecoverable()) | ||
| return false; | ||
| return true; | ||
| } | ||
|
|
||
| bool onAppTerminated() override | ||
| { | ||
| m_inputSystem = nullptr; | ||
| m_device->waitIdle(); | ||
| m_framesInFlight.clear(); | ||
| m_surface = nullptr; | ||
| m_window = nullptr; | ||
| return base_t::onAppTerminated(); | ||
| } | ||
|
|
||
| protected: | ||
| inline void onAppInitializedFinish() | ||
| { | ||
| m_winMgr->show(m_window.get()); | ||
| oracle.reportBeginFrameRecord(); | ||
| } |
There was a problem hiding this comment.
this app is doing nothing that MonoWindowApplication wasn't doing before
Comment on lines
+234
to
+235
| core::vectorSIMDf position; | ||
| core::vectorSIMDf target; |
There was a problem hiding this comment.
there's a ban on introducing new vectorSIMDf variables
Comment on lines
+813
to
+821
| bool MeshLoadersApp::isValidAABB(const hlsl::shapes::AABB<3, double>& aabb) | ||
| { | ||
| return | ||
| meshloadersIsFinite(aabb.minVx) && | ||
| meshloadersIsFinite(aabb.maxVx) && | ||
| (aabb.minVx.x <= aabb.maxVx.x) && | ||
| (aabb.minVx.y <= aabb.maxVx.y) && | ||
| (aabb.minVx.z <= aabb.maxVx.z); | ||
| } |
There was a problem hiding this comment.
I think shapes::AABB has a similar method or something in HLSL for that already
Comment on lines
+779
to
+793
| hlsl::shapes::AABB<3, double> MeshLoadersApp::translateAABB(const hlsl::shapes::AABB<3, double>& aabb, const hlsl::float64_t3& translation) | ||
| { | ||
| auto out = aabb; | ||
| out.minVx += translation; | ||
| out.maxVx += translation; | ||
| return out; | ||
| } | ||
|
|
||
| hlsl::shapes::AABB<3, double> MeshLoadersApp::scaleAABB(const hlsl::shapes::AABB<3, double>& aabb, const double scale) | ||
| { | ||
| auto out = aabb; | ||
| out.minVx *= scale; | ||
| out.maxVx *= scale; | ||
| return out; | ||
| } |
There was a problem hiding this comment.
just add a pseudo_mul(NBL_CONST_REF_ARG(matrix<T,N,N+1>),NBL_CONST_REF_ARG(shapes::AABB<3,T>)) to the HLSL linalg header
| const double nearPlane = std::max(0.001, std::min({ nearByTight, nearByRadius, 1.0 })); | ||
| const double farPlane = std::max({ tightFar * 16.0, nearPlane + safeRadius * 24.0 + 10.0, dist + safeRadius * 24.0 }); | ||
|
|
||
| const auto projection = nbl::hlsl::buildProjectionMatrixPerspectiveFovRH<nbl::hlsl::float32_t>( |
There was a problem hiding this comment.
you're using the legacy function, not the new one in correct namespace
Comment on lines
+728
to
+765
| void MeshLoadersApp::setupCameraFromAABB(const hlsl::shapes::AABB<3, double>& bound) | ||
| { | ||
| auto validBound = bound; | ||
| if (!isValidAABB(validBound)) | ||
| { | ||
| m_logger->log("Total AABB invalid; using fallback unit AABB for camera setup.", ILogger::ELL_WARNING); | ||
| validBound = meshloadersFallbackUnitAABB(); | ||
| } | ||
| const auto extent = validBound.getExtent(); | ||
| const auto aspectRatio = double(m_window->getWidth()) / double(m_window->getHeight()); | ||
| const double fovY = 1.2; | ||
| const double fovX = 2.0 * std::atan(std::tan(fovY * 0.5) * aspectRatio); | ||
| const auto center = (validBound.minVx + validBound.maxVx) * 0.5; | ||
| const auto halfExtent = extent * 0.5; | ||
| const double halfX = std::max(halfExtent.x, 0.001); | ||
| const double halfY = std::max(halfExtent.y, 0.001); | ||
| const double halfZ = std::max(halfExtent.z, 0.001); | ||
| const double safeRadius = std::max({ halfX, halfY, halfZ }); | ||
|
|
||
| // Keep startup camera horizontal and in front of the scene. | ||
| const hlsl::float64_t3 dir(0.0, 0.0, 1.0); | ||
| const double planeHalfX = halfX; | ||
| const double planeHalfY = halfY; | ||
| const double depthHalf = halfZ; | ||
| const double distY = planeHalfY / std::tan(fovY * 0.5); | ||
| const double distX = planeHalfX / std::tan(fovX * 0.5); | ||
| const double framingMargin = std::max(0.1, safeRadius * 0.35); | ||
| const double dist = std::max(distX, distY) + depthHalf + framingMargin; | ||
| const double eyeHeightOffset = std::max(halfY * 0.2, 0.05); | ||
| const auto eyeCenter = center + hlsl::float64_t3(0.0, eyeHeightOffset, 0.0); | ||
| const auto pos = eyeCenter + dir * dist; | ||
|
|
||
| const double tightNear = std::max(0.0, dist - depthHalf - framingMargin); | ||
| const double tightFar = dist + depthHalf + framingMargin; | ||
| const double nearByTight = tightNear * 0.01; | ||
| const double nearByRadius = safeRadius * 0.002; | ||
| const double nearPlane = std::max(0.001, std::min({ nearByTight, nearByRadius, 1.0 })); | ||
| const double farPlane = std::max({ tightFar * 16.0, nearPlane + safeRadius * 24.0 + 10.0, dist + safeRadius * 24.0 }); |
There was a problem hiding this comment.
looks useful I want this as an util in Devsh-Graphics-Programming/Nabla#995
Comment on lines
+673
to
+726
| bool MeshLoadersApp::writeGeometry(smart_refctd_ptr<const ICPUPolygonGeometry> geometry, const std::string& savePath) | ||
| { | ||
| using clock_t = std::chrono::high_resolution_clock; | ||
| const auto writeOuterStart = clock_t::now(); | ||
| IAsset* assetPtr = const_cast<IAsset*>(static_cast<const IAsset*>(geometry.get())); | ||
| const auto ext = normalizeExtension(system::path(savePath)); | ||
| auto flags = asset::EWF_MESH_IS_RIGHT_HANDED; | ||
| if (ext != ".obj") | ||
| flags = static_cast<asset::E_WRITER_FLAGS>(flags | asset::EWF_BINARY); | ||
| IAssetWriter::SAssetWriteParams params{ assetPtr, flags }; | ||
| params.logger = getAssetLoadLogger(); | ||
| m_logger->log("Saving mesh to %s", ILogger::ELL_INFO, savePath.c_str()); | ||
| const auto openStart = clock_t::now(); | ||
| system::ISystem::future_t<core::smart_refctd_ptr<system::IFile>> writeFileFuture; | ||
| m_system->createFile(writeFileFuture, system::path(savePath), system::IFile::ECF_WRITE); | ||
| core::smart_refctd_ptr<system::IFile> writeFile; | ||
| writeFileFuture.acquire().move_into(writeFile); | ||
| const auto openMs = toMs(clock_t::now() - openStart); | ||
| if (!writeFile) | ||
| { | ||
| m_logger->log("Failed to open output file %s", ILogger::ELL_ERROR, savePath.c_str()); | ||
| return false; | ||
| } | ||
| const auto start = clock_t::now(); | ||
| if (!m_assetMgr->writeAsset(writeFile.get(), params)) | ||
| { | ||
| const auto ms = toMs(clock_t::now() - start); | ||
| m_logger->log("Failed to save %s after %.3f ms", ILogger::ELL_ERROR, savePath.c_str(), ms); | ||
| return false; | ||
| } | ||
| const auto writeMs = toMs(clock_t::now() - start); | ||
| const auto statStart = clock_t::now(); | ||
| uintmax_t size = 0u; | ||
| if (std::filesystem::exists(savePath)) | ||
| size = std::filesystem::file_size(savePath); | ||
| const auto statMs = toMs(clock_t::now() - statStart); | ||
| const auto outerMs = toMs(clock_t::now() - writeOuterStart); | ||
| const auto nonWriterMs = std::max(0.0, outerMs - writeMs); | ||
| m_logger->log("Asset write call perf: path=%s ext=%s time=%.3f ms size=%llu", ILogger::ELL_INFO, savePath.c_str(), ext.c_str(), writeMs, static_cast<unsigned long long>(size)); | ||
| m_logger->log( | ||
| "Asset write outer perf: path=%s ext=%s open=%.3f ms writeAsset=%.3f ms stat=%.3f ms total=%.3f ms non_writer=%.3f ms size=%llu", | ||
| ILogger::ELL_INFO, | ||
| savePath.c_str(), | ||
| ext.c_str(), | ||
| openMs, | ||
| writeMs, | ||
| statMs, | ||
| outerMs, | ||
| nonWriterMs, | ||
| static_cast<unsigned long long>(size)); | ||
| m_logger->log("Writer perf: path=%s ext=%s time=%.3f ms size=%llu", ILogger::ELL_INFO, savePath.c_str(), ext.c_str(), writeMs, static_cast<unsigned long long>(size)); | ||
| m_logger->log("Mesh successfully saved!", ILogger::ELL_INFO); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
looks useful put in example common headers
Comment on lines
+74
to
+288
| if (modelPath.empty()) | ||
| failExit("Empty model path."); | ||
| if (!std::filesystem::exists(modelPath)) | ||
| failExit("Missing input: %s", modelPath.string().c_str()); | ||
| using clock_t = std::chrono::high_resolution_clock; | ||
| const auto loadOuterStart = clock_t::now(); | ||
|
|
||
| m_modelPath = modelPath.string(); | ||
|
|
||
| // free up | ||
| m_renderer->m_instances.clear(); | ||
| m_renderer->clearGeometries({ .semaphore = m_semaphore.get(),.value = m_realFrameIx }); | ||
| m_assetMgr->clearAllAssetCache(); | ||
|
|
||
| //! load the geometry | ||
| IAssetLoader::SAssetLoadParams params = makeLoadParams(); | ||
| AssetLoadCallResult loadResult = {}; | ||
| if (!loadAssetCallFromPath(modelPath, params, loadResult)) | ||
| failExit("Failed to open input file %s.", modelPath.string().c_str()); | ||
| const auto loadMs = loadResult.getAssetMs; | ||
| auto asset = std::move(loadResult.bundle); | ||
| m_logger->log( | ||
| "Asset load call perf: path=%s time=%.3f ms size=%llu", | ||
| ILogger::ELL_INFO, | ||
| m_modelPath.c_str(), | ||
| loadMs, | ||
| static_cast<unsigned long long>(loadResult.inputSize)); | ||
| if (asset.getContents().empty()) | ||
| failExit("Failed to load asset %s.", m_modelPath.c_str()); | ||
|
|
||
| core::vector<smart_refctd_ptr<const ICPUPolygonGeometry>> geometries; | ||
| const auto extractStart = clock_t::now(); | ||
| if (!appendGeometriesFromBundle(asset, geometries)) | ||
| failExit("Asset loaded but not a supported type for %s.", m_modelPath.c_str()); | ||
| const auto extractMs = toMs(clock_t::now() - extractStart); | ||
| if (geometries.empty()) | ||
| failExit("No geometry found in asset %s.", m_modelPath.c_str()); | ||
|
|
||
| const auto outerMs = toMs(clock_t::now() - loadOuterStart); | ||
| const auto nonLoaderMs = std::max(0.0, outerMs - loadMs); | ||
| m_logger->log( | ||
| "Asset load outer perf: path=%s getAsset=%.3f ms extract=%.3f ms total=%.3f ms non_loader=%.3f ms", | ||
| ILogger::ELL_INFO, | ||
| m_modelPath.c_str(), | ||
| loadMs, | ||
| extractMs, | ||
| outerMs, | ||
| nonLoaderMs); | ||
|
|
||
| m_currentCpuGeom = geometries[0]; | ||
|
|
||
| using aabb_t = hlsl::shapes::AABB<3, double>; | ||
| auto printAABB = [&](const aabb_t& aabb, const char* extraMsg = "")->void | ||
| { | ||
| m_logger->log("%s AABB is (%f,%f,%f) -> (%f,%f,%f)", ILogger::ELL_INFO, extraMsg, aabb.minVx.x, aabb.minVx.y, aabb.minVx.z, aabb.maxVx.x, aabb.maxVx.y, aabb.maxVx.z); | ||
| }; | ||
| auto bound = aabb_t::create(); | ||
| // convert the geometries | ||
| { | ||
| smart_refctd_ptr<CAssetConverter> converter = CAssetConverter::create({ .device = m_device.get() }); | ||
|
|
||
| const auto transferFamily = getTransferUpQueue()->getFamilyIndex(); | ||
|
|
||
| struct SInputs : CAssetConverter::SInputs | ||
| { | ||
| virtual inline std::span<const uint32_t> getSharedOwnershipQueueFamilies(const size_t groupCopyID, const asset::ICPUBuffer* buffer, const CAssetConverter::patch_t<asset::ICPUBuffer>& patch) const | ||
| { | ||
| return sharedBufferOwnership; | ||
| } | ||
|
|
||
| core::vector<uint32_t> sharedBufferOwnership; | ||
| } inputs = {}; | ||
| core::vector<CAssetConverter::patch_t<ICPUPolygonGeometry>> patches(geometries.size(), CSimpleDebugRenderer::DefaultPolygonGeometryPatch); | ||
| { | ||
| inputs.logger = m_logger.get(); | ||
| std::get<CAssetConverter::SInputs::asset_span_t<ICPUPolygonGeometry>>(inputs.assets) = { &geometries.front().get(),geometries.size() }; | ||
| std::get<CAssetConverter::SInputs::patch_span_t<ICPUPolygonGeometry>>(inputs.patches) = patches; | ||
| // set up shared ownership so we don't have to | ||
| core::unordered_set<uint32_t> families; | ||
| families.insert(transferFamily); | ||
| families.insert(getGraphicsQueue()->getFamilyIndex()); | ||
| if (families.size() > 1) | ||
| for (const auto fam : families) | ||
| inputs.sharedBufferOwnership.push_back(fam); | ||
| } | ||
|
|
||
| // reserve | ||
| auto reservation = converter->reserve(inputs); | ||
| if (!reservation) | ||
| { | ||
| failExit("Failed to reserve GPU objects for CPU->GPU conversion."); | ||
| } | ||
|
|
||
| // convert | ||
| { | ||
| auto semaphore = m_device->createSemaphore(0u); | ||
|
|
||
| constexpr auto MultiBuffering = 2; | ||
| std::array<smart_refctd_ptr<IGPUCommandBuffer>, MultiBuffering> commandBuffers = {}; | ||
| { | ||
| auto pool = m_device->createCommandPool(transferFamily, IGPUCommandPool::CREATE_FLAGS::RESET_COMMAND_BUFFER_BIT | IGPUCommandPool::CREATE_FLAGS::TRANSIENT_BIT); | ||
| pool->createCommandBuffers(IGPUCommandPool::BUFFER_LEVEL::PRIMARY, commandBuffers, smart_refctd_ptr(m_logger)); | ||
| } | ||
| commandBuffers.front()->begin(IGPUCommandBuffer::USAGE::ONE_TIME_SUBMIT_BIT); | ||
|
|
||
| std::array<IQueue::SSubmitInfo::SCommandBufferInfo, MultiBuffering> commandBufferSubmits; | ||
| for (auto i = 0; i < MultiBuffering; i++) | ||
| commandBufferSubmits[i].cmdbuf = commandBuffers[i].get(); | ||
|
|
||
| SIntendedSubmitInfo transfer = {}; | ||
| transfer.queue = getTransferUpQueue(); | ||
| transfer.scratchCommandBuffers = commandBufferSubmits; | ||
| transfer.scratchSemaphore = { | ||
| .semaphore = semaphore.get(), | ||
| .value = 0u, | ||
| .stageMask = PIPELINE_STAGE_FLAGS::ALL_TRANSFER_BITS | ||
| }; | ||
|
|
||
| CAssetConverter::SConvertParams cpar = {}; | ||
| cpar.utilities = m_utils.get(); | ||
| cpar.transfer = &transfer; | ||
|
|
||
| auto future = reservation.convert(cpar); | ||
| if (future.copy() != IQueue::RESULT::SUCCESS) | ||
| failExit("Failed to await submission feature."); | ||
| } | ||
|
|
||
| auto tmp = hlsl::float32_t4x3( | ||
| hlsl::float32_t3(1, 0, 0), | ||
| hlsl::float32_t3(0, 1, 0), | ||
| hlsl::float32_t3(0, 0, 1), | ||
| hlsl::float32_t3(0, 0, 0)); | ||
| core::vector<hlsl::float32_t3x4> worldTforms; | ||
| const auto& converted = reservation.getGPUObjects<ICPUPolygonGeometry>(); | ||
| m_aabbInstances.resize(converted.size()); | ||
| if (m_drawBBMode == DBBM_OBB) | ||
| m_obbInstances.resize(converted.size()); | ||
| for (uint32_t i = 0; i < converted.size(); i++) | ||
| { | ||
| const auto& cpuGeom = geometries[i].get(); | ||
| auto promoted = getGeometryAABB(cpuGeom); | ||
| if (!isValidAABB(promoted)) | ||
| { | ||
| m_logger->log("Invalid geometry AABB for %s (geo=%u). Using fallback unit AABB for framing.", ILogger::ELL_WARNING, m_modelPath.c_str(), i); | ||
| promoted = meshloadersFallbackUnitAABB(); | ||
| } | ||
| printAABB(promoted, "Geometry"); | ||
| const auto promotedWorld = hlsl::float64_t3x4(worldTforms.emplace_back(hlsl::transpose(tmp))); | ||
| const auto translation = hlsl::float64_t3( | ||
| static_cast<double>(tmp[3].x), | ||
| static_cast<double>(tmp[3].y), | ||
| static_cast<double>(tmp[3].z)); | ||
| const auto transformed = translateAABB(promoted, translation); | ||
| printAABB(transformed, "Transformed"); | ||
| bound = hlsl::shapes::util::union_(transformed, bound); | ||
|
|
||
| #ifdef NBL_BUILD_DEBUG_DRAW | ||
| auto& aabbInst = m_aabbInstances[i]; | ||
| const auto tmpAabb = shapes::AABB<3, float>(promoted.minVx, promoted.maxVx); | ||
|
|
||
| hlsl::float32_t3x4 aabbTransform = ext::debug_draw::DrawAABB::getTransformFromAABB(tmpAabb); | ||
| const auto tmpWorld = hlsl::float32_t3x4(promotedWorld); | ||
| const auto world4x4 = float32_t4x4{ | ||
| tmpWorld[0], | ||
| tmpWorld[1], | ||
| tmpWorld[2], | ||
| float32_t4(0, 0, 0, 1) | ||
| }; | ||
|
|
||
| aabbInst.color = { 1, 1, 1, 1 }; | ||
| aabbInst.transform = math::linalg::promoted_mul(world4x4, aabbTransform); | ||
|
|
||
| if (m_drawBBMode == DBBM_OBB) | ||
| { | ||
| auto& obbInst = m_obbInstances[i]; | ||
| const auto obb = CPolygonGeometryManipulator::calculateOBB( | ||
| cpuGeom->getPositionView().getElementCount(), | ||
| [geo = cpuGeom, &world4x4](size_t vertex_i) { | ||
| hlsl::float32_t3 pt; | ||
| geo->getPositionView().decodeElement(vertex_i, pt); | ||
| return pt; | ||
| }); | ||
| obbInst.color = { 0, 0, 1, 1 }; | ||
| obbInst.transform = math::linalg::promoted_mul(world4x4, obb.transform); | ||
| } | ||
| #endif | ||
| } | ||
|
|
||
| printAABB(bound, "Total"); | ||
| if (!m_renderer->addGeometries({ &converted.front().get(),converted.size() })) | ||
| failExit("Failed to add geometries to renderer."); | ||
| if (m_logger) | ||
| { | ||
| const auto& gpuGeos = m_renderer->getGeometries(); | ||
| for (size_t geoIx = 0u; geoIx < gpuGeos.size(); ++geoIx) | ||
| { | ||
| const auto& gpuGeo = gpuGeos[geoIx]; | ||
| m_logger->log( | ||
| "Renderer geo state: idx=%llu elem=%u posView=%u normalView=%u indexType=%u", | ||
| ILogger::ELL_DEBUG, | ||
| static_cast<unsigned long long>(geoIx), | ||
| gpuGeo.elementCount, | ||
| static_cast<uint32_t>(gpuGeo.positionView), | ||
| static_cast<uint32_t>(gpuGeo.normalView), | ||
| static_cast<uint32_t>(gpuGeo.indexType)); | ||
| } | ||
| } | ||
|
|
||
| auto worlTformsIt = worldTforms.begin(); | ||
| for (const auto& geo : m_renderer->getGeometries()) | ||
| m_renderer->m_instances.push_back({ | ||
| .world = *(worlTformsIt++), | ||
| .packedGeo = &geo | ||
| }); | ||
| } |
Member
There was a problem hiding this comment.
I'd slap this in the renderer, but without the OBB/AABB stuff and do the OBB/AABB stuff via a callback / lambda injection
Comment on lines
+13
to
+70
| namespace | ||
| { | ||
| inline bool meshloadersIsFinite(const double value) | ||
| { | ||
| return std::isfinite(value); | ||
| } | ||
|
|
||
| inline bool meshloadersIsFinite(const hlsl::float64_t3& value) | ||
| { | ||
| return meshloadersIsFinite(value.x) && meshloadersIsFinite(value.y) && meshloadersIsFinite(value.z); | ||
| } | ||
|
|
||
| hlsl::shapes::AABB<3, double> meshloadersComputeFinitePositionAABB(const ICPUPolygonGeometry* geometry) | ||
| { | ||
| auto aabb = hlsl::shapes::AABB<3, double>::create(); | ||
| if (!geometry) | ||
| return aabb; | ||
| const auto positionView = geometry->getPositionView(); | ||
| const auto vertexCount = positionView.getElementCount(); | ||
| bool hasFiniteVertex = false; | ||
| for (size_t i = 0u; i < vertexCount; ++i) | ||
| { | ||
| hlsl::float32_t3 decoded = {}; | ||
| positionView.decodeElement(i, decoded); | ||
| const hlsl::float64_t3 p = { | ||
| static_cast<double>(decoded.x), | ||
| static_cast<double>(decoded.y), | ||
| static_cast<double>(decoded.z) | ||
| }; | ||
| if (!meshloadersIsFinite(p)) | ||
| continue; | ||
| if (!hasFiniteVertex) | ||
| { | ||
| aabb.minVx = p; | ||
| aabb.maxVx = p; | ||
| hasFiniteVertex = true; | ||
| continue; | ||
| } | ||
| aabb.minVx.x = std::min(aabb.minVx.x, p.x); | ||
| aabb.minVx.y = std::min(aabb.minVx.y, p.y); | ||
| aabb.minVx.z = std::min(aabb.minVx.z, p.z); | ||
| aabb.maxVx.x = std::max(aabb.maxVx.x, p.x); | ||
| aabb.maxVx.y = std::max(aabb.maxVx.y, p.y); | ||
| aabb.maxVx.z = std::max(aabb.maxVx.z, p.z); | ||
| } | ||
| if (hasFiniteVertex) | ||
| return aabb; | ||
| return hlsl::shapes::AABB<3, double>::create(); | ||
| } | ||
|
|
||
| hlsl::shapes::AABB<3, double> meshloadersFallbackUnitAABB() | ||
| { | ||
| hlsl::shapes::AABB<3, double> fallback = hlsl::shapes::AABB<3, double>::create(); | ||
| fallback.minVx = hlsl::float64_t3(-1.0, -1.0, -1.0); | ||
| fallback.maxVx = hlsl::float64_t3(1.0, 1.0, 1.0); | ||
| return fallback; | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't get why you need these functions
Comment on lines
+31
to
+57
| const auto vertexCount = positionView.getElementCount(); | ||
| bool hasFiniteVertex = false; | ||
| for (size_t i = 0u; i < vertexCount; ++i) | ||
| { | ||
| hlsl::float32_t3 decoded = {}; | ||
| positionView.decodeElement(i, decoded); | ||
| const hlsl::float64_t3 p = { | ||
| static_cast<double>(decoded.x), | ||
| static_cast<double>(decoded.y), | ||
| static_cast<double>(decoded.z) | ||
| }; | ||
| if (!meshloadersIsFinite(p)) | ||
| continue; | ||
| if (!hasFiniteVertex) | ||
| { | ||
| aabb.minVx = p; | ||
| aabb.maxVx = p; | ||
| hasFiniteVertex = true; | ||
| continue; | ||
| } | ||
| aabb.minVx.x = std::min(aabb.minVx.x, p.x); | ||
| aabb.minVx.y = std::min(aabb.minVx.y, p.y); | ||
| aabb.minVx.z = std::min(aabb.minVx.z, p.z); | ||
| aabb.maxVx.x = std::max(aabb.maxVx.x, p.x); | ||
| aabb.maxVx.y = std::max(aabb.maxVx.y, p.y); | ||
| aabb.maxVx.z = std::max(aabb.maxVx.z, p.z); | ||
| } |
There was a problem hiding this comment.
this is not how to compute an AABB, you're not iterating over the actually used vertices
Comment on lines
+558
to
+568
| bool MeshLoadersApp::onAppTerminated() | ||
| { | ||
| return device_base_t::onAppTerminated(); | ||
| } | ||
|
|
||
| bool MeshLoadersApp::keepRunning() | ||
| { | ||
| if (m_shouldQuit) | ||
| return false; | ||
| return device_base_t::keepRunning(); | ||
| } |
There was a problem hiding this comment.
thats same as monoWindowApplication noi point overriding
Comment on lines
+441
to
+489
| if (!m_nonInteractiveTest) | ||
| { | ||
| bool reloadInteractiveRequested = false; | ||
| bool reloadListRequested = false; | ||
| bool addRowViewRequested = false; | ||
| bool clearRowViewRequested = false; | ||
| camera.beginInputProcessing(nextPresentationTimestamp); | ||
| mouse.consumeEvents([&](const IMouseEventChannel::range_t& events) -> void { camera.mouseProcess(events); }, m_logger.get()); | ||
| keyboard.consumeEvents([&](const IKeyboardEventChannel::range_t& events) -> void | ||
| { | ||
| for (const auto& event : events) | ||
| { | ||
| if (event.action != SKeyboardEvent::ECA_RELEASED) | ||
| continue; | ||
| if (event.keyCode == E_KEY_CODE::EKC_R) | ||
| { | ||
| if (isRowViewActive()) | ||
| reloadListRequested = true; | ||
| else | ||
| reloadInteractiveRequested = true; | ||
| } | ||
| else if (event.keyCode == E_KEY_CODE::EKC_A) | ||
| { | ||
| if (isRowViewActive()) | ||
| addRowViewRequested = true; | ||
| } | ||
| else if (event.keyCode == E_KEY_CODE::EKC_X) | ||
| { | ||
| if (isRowViewActive()) | ||
| clearRowViewRequested = true; | ||
| } | ||
| } | ||
| camera.keyboardProcess(events); | ||
| }, | ||
| m_logger.get() | ||
| ); | ||
| camera.endInputProcessing(nextPresentationTimestamp); | ||
| if (clearRowViewRequested) | ||
| resetRowViewScene(); | ||
| if (addRowViewRequested) | ||
| addRowViewCase(); | ||
| if (reloadListRequested) | ||
| { | ||
| if (!reloadFromTestList()) | ||
| failExit("Failed to reload test list."); | ||
| } | ||
| if (reloadInteractiveRequested) | ||
| reloadInteractive(); | ||
| } |
There was a problem hiding this comment.
can you pack it up into a separate struct/class to manage this state, not sure I like the App class ahving tens of methods and individual members
Comment on lines
+137
to
+145
| const auto resolveRuntimeCWD = [](const path& preferred)->path | ||
| { | ||
| if (preferred.empty() || preferred == path("/") || preferred == path("\\")) | ||
| return path(std::filesystem::current_path()); | ||
| return preferred; | ||
| }; | ||
| const path effectiveInputCWD = resolveRuntimeCWD(localInputCWD); | ||
| const path effectiveOutputCWD = resolveRuntimeCWD(localOutputCWD); | ||
|
|
There was a problem hiding this comment.
what do you need to do this for?
Comment on lines
+27
to
+113
| template<typename ArgContainer> | ||
| std::string makeCaptionModelPath(const std::string& modelPath, const ArgContainer& argv) | ||
| { | ||
| if (modelPath.empty()) | ||
| return {}; | ||
|
|
||
| std::error_code ec; | ||
| if (modelPath.find('/') == std::string::npos && modelPath.find('\\') == std::string::npos) | ||
| { | ||
| if (!std::filesystem::exists(std::filesystem::path(modelPath), ec)) | ||
| { | ||
| ec.clear(); | ||
| return modelPath; | ||
| } | ||
| ec.clear(); | ||
| } | ||
| std::filesystem::path targetPath(modelPath); | ||
| targetPath = targetPath.lexically_normal(); | ||
| const auto canonicalTarget = std::filesystem::weakly_canonical(targetPath, ec); | ||
| if (!ec) | ||
| targetPath = canonicalTarget; | ||
| else | ||
| ec.clear(); | ||
|
|
||
| if (!targetPath.is_absolute()) | ||
| { | ||
| const auto absoluteTarget = std::filesystem::absolute(targetPath, ec); | ||
| if (!ec) | ||
| targetPath = absoluteTarget.lexically_normal(); | ||
| else | ||
| ec.clear(); | ||
| } | ||
| if (!targetPath.is_absolute()) | ||
| return targetPath.generic_string(); | ||
|
|
||
| auto relativeFromBase = [&](const std::filesystem::path& basePath) -> std::string | ||
| { | ||
| if (basePath.empty()) | ||
| return {}; | ||
| auto canonicalBase = std::filesystem::weakly_canonical(basePath, ec); | ||
| if (ec) | ||
| { | ||
| ec.clear(); | ||
| canonicalBase = std::filesystem::absolute(basePath, ec); | ||
| } | ||
| if (ec) | ||
| { | ||
| ec.clear(); | ||
| return {}; | ||
| } | ||
| const auto relativePath = std::filesystem::relative(targetPath, canonicalBase, ec); | ||
| if (ec || relativePath.empty() || relativePath.is_absolute()) | ||
| { | ||
| ec.clear(); | ||
| return {}; | ||
| } | ||
| return relativePath.lexically_normal().generic_string(); | ||
| }; | ||
|
|
||
| std::string bestRelativePath; | ||
| if (!argv.empty() && !argv[0].empty()) | ||
| { | ||
| const auto exePath = std::filesystem::absolute(std::filesystem::path(argv[0]), ec); | ||
| if (!ec) | ||
| { | ||
| const auto relativeToExe = relativeFromBase(exePath.parent_path()); | ||
| if (!relativeToExe.empty()) | ||
| bestRelativePath = relativeToExe; | ||
| } | ||
| else | ||
| ec.clear(); | ||
| } | ||
|
|
||
| const auto cwd = std::filesystem::current_path(ec); | ||
| if (!ec) | ||
| { | ||
| const auto relativeToCwd = relativeFromBase(cwd); | ||
| if (!relativeToCwd.empty() && (bestRelativePath.empty() || relativeToCwd.size() < bestRelativePath.size())) | ||
| bestRelativePath = relativeToCwd; | ||
| } | ||
| else | ||
| ec.clear(); | ||
|
|
||
| if (!bestRelativePath.empty()) | ||
| return bestRelativePath; | ||
| return targetPath.generic_string(); | ||
| } |
There was a problem hiding this comment.
what does it do, and why is it so long ?
| bool hasAabb = false; | ||
| }; | ||
|
|
||
| struct RowViewPerfStats |
There was a problem hiding this comment.
whats a Row view, document that concept
Comment on lines
+266
to
+325
| bool initTestCases(); | ||
| bool pickModelPath(system::path& outPath); | ||
| bool loadTestList(const system::path& jsonPath); | ||
| bool isRowViewActive() const; | ||
|
|
||
| static std::string normalizeExtension(const system::path& path); | ||
| bool isWriteExtensionSupported(const std::string& ext) const; | ||
| system::path resolveSavePath(const system::path& modelPath) const; | ||
|
|
||
| static std::string sanitizeCaseNameForFilename(std::string name); | ||
| system::path getGeometryHashReferencePath(const std::string& caseName) const; | ||
| static std::string geometryHashToHex(const core::blake3_hash_t& hash); | ||
| static bool tryParseNibble(char c, uint8_t& out); | ||
| static bool tryParseGeometryHashHex(std::string hex, core::blake3_hash_t& outHash); | ||
| bool readGeometryHashReference(const system::path& refPath, core::blake3_hash_t& outHash) const; | ||
| bool writeGeometryHashReference(const system::path& refPath, const core::blake3_hash_t& hash) const; | ||
|
|
||
| bool startCase(size_t index); | ||
| bool advanceToNextCase(); | ||
| void reloadInteractive(); | ||
| bool addRowViewCase(); | ||
| bool addRowViewCaseFromPath(const system::path& picked); | ||
| bool reloadFromTestList(); | ||
| void resetRowViewScene(); | ||
|
|
||
| bool loadModel(const system::path& modelPath, bool updateCamera, bool storeCamera); | ||
| bool loadRowView(RowViewReloadMode mode); | ||
| bool writeGeometry(smart_refctd_ptr<const ICPUPolygonGeometry> geometry, const std::string& savePath); | ||
| bool runHashConsistencyChecks(); | ||
|
|
||
| void setupCameraFromAABB(const hlsl::shapes::AABB<3, double>& bound); | ||
| static hlsl::shapes::AABB<3, double> translateAABB(const hlsl::shapes::AABB<3, double>& aabb, const hlsl::float64_t3& translation); | ||
| static hlsl::shapes::AABB<3, double> scaleAABB(const hlsl::shapes::AABB<3, double>& aabb, double scale); | ||
|
|
||
| void storeCameraState(); | ||
| void applyCameraState(const CameraState& state); | ||
|
|
||
| static bool isValidAABB(const hlsl::shapes::AABB<3, double>& aabb); | ||
| hlsl::shapes::AABB<3, double> getGeometryAABB(const ICPUPolygonGeometry* geometry) const; | ||
|
|
||
| system::ILogger* getAssetLoadLogger() const; | ||
| IAssetLoader::SAssetLoadParams makeLoadParams() const; | ||
| bool loadAssetCallFromPath(const system::path& modelPath, const IAssetLoader::SAssetLoadParams& params, AssetLoadCallResult& out); | ||
| bool initLoaderPerfLogger(const system::path& logPath); | ||
|
|
||
| std::string makeUniqueCaseName(const system::path& path); | ||
| static double toMs(const std::chrono::high_resolution_clock::duration& d); | ||
| std::string makeCacheKey(const system::path& path) const; | ||
|
|
||
| void logRowViewPerf(const RowViewPerfStats& stats) const; | ||
| void logRowViewAssetLoad(const system::path& path, double ms, bool cached) const; | ||
| void logRowViewLoadTotal(double ms, size_t hits, size_t misses) const; | ||
|
|
||
| core::blake3_hash_t hashGeometry(const ICPUPolygonGeometry* geo); | ||
| bool validateWrittenAsset(const system::path& path); | ||
| bool captureScreenshot(const system::path& path, core::smart_refctd_ptr<asset::ICPUImageView>& outImage); | ||
| bool appendGeometriesFromBundle(const asset::SAssetBundle& bundle, core::vector<smart_refctd_ptr<const ICPUPolygonGeometry>>& out) const; | ||
| bool compareImages(const asset::ICPUImageView* a, const asset::ICPUImageView* b, uint64_t& diffCount, uint8_t& maxDiff); | ||
|
|
||
| void advanceCase(); |
There was a problem hiding this comment.
this needs cleaning up and probably splitting off into 2 or 3 separate classes, maube some of it needs to move to examples common
Comment on lines
+357
to
+399
| bool m_nonInteractiveTest = false; | ||
| bool m_rowViewEnabled = true; | ||
| bool m_forceRowViewForCurrentTestList = false; | ||
| bool m_rowViewScreenshotCaptured = false; | ||
| bool m_fileDialogOpen = false; | ||
|
|
||
| bool m_saveGeom = true; | ||
| std::optional<const std::string> m_specifiedGeomSavePath; | ||
| nbl::system::path m_saveGeomPrefixPath; | ||
| nbl::system::path m_screenshotPrefixPath; | ||
| nbl::system::path m_rowViewScreenshotPath; | ||
| nbl::system::path m_testListPath; | ||
| nbl::system::path m_geometryHashReferenceDir; | ||
| nbl::system::path m_caseGeometryHashReferencePath; | ||
| std::optional<nbl::system::path> m_loaderPerfLogPath; | ||
| std::optional<nbl::system::path> m_rowAddPath; | ||
| uint32_t m_rowDuplicateCount = 0u; | ||
| smart_refctd_ptr<system::ILogger> m_assetLoadLogger; | ||
| smart_refctd_ptr<system::ILogger> m_loaderPerfLogger; | ||
| bool m_updateGeometryHashReferences = false; | ||
| bool m_forceLoaderContentHashes = true; | ||
| bool m_hashTestOnly = false; | ||
| asset::SFileIOPolicy::SRuntimeTuning::Mode m_runtimeTuningMode = asset::SFileIOPolicy::SRuntimeTuning::Mode::Heuristic; | ||
|
|
||
| RunMode m_runMode = RunMode::Batch; | ||
| Phase m_phase = Phase::RenderOriginal; | ||
| uint32_t m_phaseFrameCounter = 0u; | ||
| size_t m_caseIndex = 0u; | ||
| core::vector<TestCase> m_cases; | ||
| std::unordered_map<std::string, uint32_t> m_caseNameCounts; | ||
| std::unordered_map<std::string, CachedGeometryEntry> m_rowViewCache; | ||
| bool m_shouldQuit = false; | ||
|
|
||
| nbl::system::path m_writtenPath; | ||
| nbl::system::path m_loadedScreenshotPath; | ||
| nbl::system::path m_writtenScreenshotPath; | ||
|
|
||
| core::smart_refctd_ptr<const ICPUPolygonGeometry> m_currentCpuGeom; | ||
| core::blake3_hash_t m_referenceGeometryHash = {}; | ||
| bool m_hasReferenceGeometryHash = false; | ||
|
|
||
| core::smart_refctd_ptr<asset::ICPUImageView> m_loadedScreenshot; | ||
| core::smart_refctd_ptr<asset::ICPUImageView> m_writtenScreenshot; |
There was a problem hiding this comment.
same here, only keep stuff thats persistent and more or less constant in the app, rest are some context transient state that should be passed around as context structs
Comment on lines
+146
to
+322
| m_runMode = RunMode::Batch; | ||
| m_saveGeomPrefixPath = effectiveOutputCWD / "saved"; | ||
| m_screenshotPrefixPath = effectiveOutputCWD / "screenshots"; | ||
| m_testListPath = effectiveInputCWD / "inputs.json"; | ||
| m_forceRowViewForCurrentTestList = false; | ||
| #if defined(NBL_MESHLOADERS_DEFAULT_BENCHMARK_TESTLIST_PATH) | ||
| const path defaultBenchmarkTestListPath = path(NBL_MESHLOADERS_DEFAULT_BENCHMARK_TESTLIST_PATH); | ||
| #else | ||
| const path defaultBenchmarkTestListPath; | ||
| #endif | ||
|
|
||
| argparse::ArgumentParser parser("12_meshloaders"); | ||
| parser.add_argument("--savegeometry") | ||
| .help("Save the mesh on exit or reload") | ||
| .flag(); | ||
|
|
||
| parser.add_argument("--savepath") | ||
| .nargs(1) | ||
| .help("Specify the file to which the mesh will be saved"); | ||
| parser.add_argument("--ci") | ||
| .help("Run in CI mode: load test list, write .ply, capture screenshots, compare data, and exit.") | ||
| .flag(); | ||
| parser.add_argument("--hash-test") | ||
| .help("Run headless hash consistency check: parallel vs sequential content hash recompute, then exit.") | ||
| .flag(); | ||
| parser.add_argument("--interactive") | ||
| .help("Use file dialog to select a single model.") | ||
| .flag(); | ||
| parser.add_argument("--testlist") | ||
| .nargs(1) | ||
| .help("JSON file with test cases. Relative paths are resolved against local input CWD."); | ||
| parser.add_argument("--row-add") | ||
| .nargs(1) | ||
| .help("Add a model path to row view on startup without using a dialog."); | ||
| parser.add_argument("--row-duplicate") | ||
| .nargs(1) | ||
| .help("Duplicate the last case N times on startup."); | ||
| parser.add_argument("--loader-perf-log") | ||
| .nargs(1) | ||
| .help("Write loader diagnostics to a file instead of stdout."); | ||
| parser.add_argument("--loader-content-hashes") | ||
| .help("Force loaders to compute CPU buffer content hashes before returning. Enabled by default.") | ||
| .flag(); | ||
| parser.add_argument("--runtime-tuning") | ||
| .nargs(1) | ||
| .help("Runtime tuning mode for loaders: none|heuristic|hybrid. Default: heuristic."); | ||
| parser.add_argument("--update-references") | ||
| .help("Update or create geometry hash references for CI validation.") | ||
| .flag(); | ||
|
|
||
| try | ||
| { | ||
| parser.parse_args({ argv.data(), argv.data() + argv.size() }); | ||
| } | ||
| catch (const std::exception& e) | ||
| { | ||
| return logFail(e.what()); | ||
| } | ||
|
|
||
| if (parser["--savegeometry"] == true) | ||
| m_saveGeom = true; | ||
| if (parser["--interactive"] == true) | ||
| m_runMode = RunMode::Interactive; | ||
| if (parser["--ci"] == true) | ||
| m_runMode = RunMode::CI; | ||
| if (parser["--hash-test"] == true) | ||
| { | ||
| m_hashTestOnly = true; | ||
| m_runMode = RunMode::CI; | ||
| } | ||
| const bool hasExplicitTestListArg = parser.present("--testlist").has_value(); | ||
|
|
||
| if (parser.present("--savepath")) | ||
| { | ||
| auto tmp = path(parser.get<std::string>("--savepath")); | ||
|
|
||
| if (tmp.empty() || !tmp.has_filename()) | ||
| return logFail("Invalid path has been specified in --savepath argument"); | ||
|
|
||
| if (!std::filesystem::exists(tmp.parent_path())) | ||
| return logFail("Path specified in --savepath argument doesn't exist"); | ||
|
|
||
| m_specifiedGeomSavePath.emplace(std::move(tmp.generic_string())); | ||
| } | ||
|
|
||
| if (hasExplicitTestListArg) | ||
| { | ||
| auto tmp = path(parser.get<std::string>("--testlist")); | ||
| if (tmp.empty()) | ||
| return logFail("Invalid path has been specified in --testlist argument"); | ||
| if (tmp.is_relative()) | ||
| tmp = effectiveInputCWD / tmp; | ||
| m_testListPath = tmp; | ||
| } | ||
| else if (m_runMode == RunMode::Batch && !defaultBenchmarkTestListPath.empty()) | ||
| { | ||
| std::error_code benchmarkPathEc; | ||
| if (std::filesystem::exists(defaultBenchmarkTestListPath, benchmarkPathEc) && !benchmarkPathEc) | ||
| { | ||
| m_testListPath = defaultBenchmarkTestListPath; | ||
| m_forceRowViewForCurrentTestList = true; | ||
| m_logger->log("Using benchmark test list for default batch startup: %s", ILogger::ELL_INFO, m_testListPath.string().c_str()); | ||
| } | ||
| } | ||
| if (parser.present("--row-add")) | ||
| { | ||
| auto tmp = path(parser.get<std::string>("--row-add")); | ||
| if (tmp.is_relative()) | ||
| tmp = effectiveInputCWD / tmp; | ||
| m_rowAddPath = tmp; | ||
| } | ||
| if (parser.present("--row-duplicate")) | ||
| { | ||
| auto countStr = parser.get<std::string>("--row-duplicate"); | ||
| try | ||
| { | ||
| m_rowDuplicateCount = static_cast<uint32_t>(std::stoul(countStr)); | ||
| } | ||
| catch (const std::exception&) | ||
| { | ||
| return logFail("Invalid --row-duplicate value."); | ||
| } | ||
| } | ||
| if (parser.present("--loader-perf-log")) | ||
| { | ||
| auto tmp = path(parser.get<std::string>("--loader-perf-log")); | ||
| if (tmp.empty()) | ||
| return logFail("Invalid --loader-perf-log value."); | ||
| if (tmp.is_relative()) | ||
| tmp = effectiveOutputCWD / tmp; | ||
| m_loaderPerfLogPath = tmp; | ||
| } | ||
| if (parser["--update-references"] == true) | ||
| m_updateGeometryHashReferences = true; | ||
| if (parser["--loader-content-hashes"] == true) | ||
| m_forceLoaderContentHashes = true; | ||
| if (parser.present("--runtime-tuning")) | ||
| { | ||
| auto mode = parser.get<std::string>("--runtime-tuning"); | ||
| std::transform(mode.begin(), mode.end(), mode.begin(), [](unsigned char c) { return static_cast<char>(std::tolower(c)); }); | ||
| if (mode == "none") | ||
| m_runtimeTuningMode = asset::SFileIOPolicy::SRuntimeTuning::Mode::None; | ||
| else if (mode == "heuristic") | ||
| m_runtimeTuningMode = asset::SFileIOPolicy::SRuntimeTuning::Mode::Heuristic; | ||
| else if (mode == "hybrid") | ||
| m_runtimeTuningMode = asset::SFileIOPolicy::SRuntimeTuning::Mode::Hybrid; | ||
| else | ||
| return logFail("Invalid --runtime-tuning value. Expected: none|heuristic|hybrid."); | ||
| } | ||
|
|
||
| const path inputReferencesDir = effectiveInputCWD / "references"; | ||
| const path outputReferencesDir = effectiveOutputCWD / "references"; | ||
| std::error_code referenceDirEc; | ||
| const bool hasInputReferencesDir = std::filesystem::is_directory(inputReferencesDir, referenceDirEc) && !referenceDirEc; | ||
| referenceDirEc.clear(); | ||
| const bool hasOutputReferencesDir = std::filesystem::is_directory(outputReferencesDir, referenceDirEc) && !referenceDirEc; | ||
| m_geometryHashReferenceDir = hasOutputReferencesDir || !hasInputReferencesDir ? outputReferencesDir : inputReferencesDir; | ||
| if (hasOutputReferencesDir && !hasInputReferencesDir) | ||
| m_logger->log("Geometry hash references resolved to output directory: %s", system::ILogger::ELL_INFO, m_geometryHashReferenceDir.string().c_str()); | ||
| if (m_runMode == RunMode::CI || m_updateGeometryHashReferences) | ||
| { | ||
| std::error_code ec; | ||
| std::filesystem::create_directories(m_geometryHashReferenceDir, ec); | ||
| if (ec) | ||
| return logFail("Failed to create geometry hash reference directory: %s", m_geometryHashReferenceDir.string().c_str()); | ||
| } | ||
|
|
||
| if (m_saveGeom) | ||
| std::filesystem::create_directories(m_saveGeomPrefixPath); | ||
| std::filesystem::create_directories(m_screenshotPrefixPath); | ||
| m_assetLoadLogger = m_logger; | ||
| if (m_loaderPerfLogPath) | ||
| { | ||
| if (!initLoaderPerfLogger(*m_loaderPerfLogPath)) | ||
| return false; | ||
| m_logger->log("Loader diagnostics will be written to %s", ILogger::ELL_INFO, m_loaderPerfLogPath->string().c_str()); | ||
| } |
There was a problem hiding this comment.
most of this should go live somewhere else, especially the argument parser setup
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.