Skip to content

Commit

Permalink
Cleanup comments
Browse files Browse the repository at this point in the history
Remove/modify stale ones, remove ones stating the obvious where section
comments aren't useful
  • Loading branch information
sndels committed Aug 5, 2024
1 parent 608b198 commit e4097d1
Show file tree
Hide file tree
Showing 14 changed files with 39 additions and 68 deletions.
24 changes: 8 additions & 16 deletions src/App.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ void App::recreateSwapchainAndRelated(ScopedScratch scopeAlloc)
// queue simultaneously
gDevice.graphicsQueue().waitIdle();

{ // Drop the config as we should always use swapchain's active config
{
const SwapchainConfig config{
scopeAlloc.child_scope(), {gWindow.width(), gWindow.height()}};
m_swapchain->recreate(config);
Expand Down Expand Up @@ -512,9 +512,6 @@ void App::drawFrame(ScopedScratch scopeAlloc, uint32_t scopeHighWatermark)

m_world->uploadMeshDatas(scopeAlloc.child_scope(), nextFrame);

// -1 seems like a safe value here since an 8 sample halton sequence is
// used. See A Survey of Temporal Antialiasing Techniques by Yang, Liu and
// Salvi for details.
const float lodBias = m_renderer->lodBias();
m_world->uploadMaterialDatas(nextFrame, lodBias);

Expand All @@ -533,8 +530,8 @@ void App::drawFrame(ScopedScratch scopeAlloc, uint32_t scopeHighWatermark)
m_cameraParameters = params;
m_cam->setParameters(params);
if (m_forceCamUpdate)
// Disable free look for animated cameras when update is forced
// (camera changed)
// Disable free look for animated cameras when the camera
// changed
m_camFreeLook = !m_world->isCurrentCameraDynamic();
m_forceCamUpdate = false;
}
Expand All @@ -559,7 +556,6 @@ void App::drawFrame(ScopedScratch scopeAlloc, uint32_t scopeHighWatermark)
.flags = vk::CommandBufferUsageFlagBits::eOneTimeSubmit,
});

// We need to build TLAS if things are animated or we can build new BLASes
if (m_renderer->rtInUse() || m_world->unbuiltBlases())
{
PROFILER_CPU_GPU_SCOPE(cb, "BuildTLAS");
Expand Down Expand Up @@ -598,7 +594,6 @@ void App::drawFrame(ScopedScratch scopeAlloc, uint32_t scopeHighWatermark)
const Optional<vec4> nonLinearDepth = m_renderer->tryDepthReadback();
if (nonLinearDepth.has_value())
{
// First we get the projected direction and linear depth
const vec2 uv =
(m_pickedFocusPx + 0.5f) /
vec2{m_viewportExtent.width, m_viewportExtent.height};
Expand All @@ -609,7 +604,6 @@ void App::drawFrame(ScopedScratch scopeAlloc, uint32_t scopeHighWatermark)
const float projectedDepth = length(projectedDir);
projectedDir /= projectedDepth;

// Camera looks at -Z in view space
const float cosTheta = dot(vec3{0.f, 0.f, -1.f}, projectedDir);

CameraParameters params = m_cam->parameters();
Expand Down Expand Up @@ -649,7 +643,6 @@ uint32_t App::nextSwapchainImage(ScopedScratch scopeAlloc, uint32_t nextFrame)
gDevice.graphicsQueue().submit(1, &submitInfo, vk::Fence{}),
"recreate_swap_dummy_submit");

// Recreate the swap chain as necessary
recreateSwapchainAndRelated(scopeAlloc.child_scope());
nextImage = m_swapchain->acquireNextImage(imageAvailable);
}
Expand All @@ -673,7 +666,6 @@ float App::currentTimelineTimeS() const

void App::capFramerate()
{
// Enforce fps cap by spinlocking to have any hope to be somewhat consistent
// Note that this is always based on the previous frame so it only limits
// fps and doesn't help actual frame timing
const float minDt =
Expand Down Expand Up @@ -755,7 +747,6 @@ void App::drawProfiling(
if (t.name.size() > longestNameLength)
longestNameLength = asserted_cast<size_t>(t.name.size());

// Double the maximum name length for headroom
String tmp{scopeAlloc};
tmp.resize(longestNameLength * 2);
const auto leftJustified =
Expand Down Expand Up @@ -1066,7 +1057,7 @@ void App::updateDebugLines(const Scene &scene, uint32_t nextFrame)
{
auto &debugLines = gRenderResources.debugLines[nextFrame];
debugLines.reset();
{ // Add debug geom for lights
{
constexpr auto debugLineLength = 0.2f;
constexpr auto debugRed = vec3{1.f, 0.05f, 0.05f};
constexpr auto debugGreen = vec3{0.05f, 1.f, 0.05f};
Expand Down Expand Up @@ -1163,12 +1154,13 @@ void App::handleResizes(ScopedScratch scopeAlloc, bool shouldResizeSwapchain)
{
const bool viewportResized = m_renderer->viewportResized();

// Recreate swapchain if so indicated and explicitly handle resizes
if (shouldResizeSwapchain || gWindow.resized())
recreateSwapchainAndRelated(scopeAlloc.child_scope());
else if (viewportResized || m_forceViewportRecreate)
{ // Don't recreate viewport related on the same frame as swapchain is
// resized since we don't know the new viewport area until the next frame
{
// Don't recreate viewport related on the same frame as swapchain is
// resized since we don't know the new viewport area until the next
// frame
recreateViewportRelated();
m_forceViewportRecreate = false;
}
Expand Down
5 changes: 0 additions & 5 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,18 +139,14 @@ int main(int argc, char *argv[])
const Timer t;

#ifdef LIVEPP_PATH
// create a default agent, loading the Live++ agent from the given path
lpp::LppDefaultAgent lppAgent =
lpp::LppCreateDefaultAgent(nullptr, LIVEPP_PATH);

// bail out in case the agent is not valid
if (!lpp::LppIsValidDefaultAgent(&lppAgent))
{
LOG_ERR("Couldn't create Live++ agent. Is LIVEPP_PATH correct?");
return 1;
}

// enable Live++ for all loaded modules
lppAgent.EnableModule(
lpp::LppGetCurrentModulePath(),
lpp::LPP_MODULES_OPTION_ALL_IMPORT_MODULES, nullptr, nullptr);
Expand Down Expand Up @@ -215,7 +211,6 @@ int main(int argc, char *argv[])
}

#ifdef LIVEPP_PATH
// destroy the Live++ agent
lpp::LppDestroyDefaultAgent(&lppAgent);
#endif // LIVEPP_PATH

Expand Down
8 changes: 7 additions & 1 deletion src/render/Renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,13 @@ vec2 Renderer::viewportOffsetInUi() const
return vec2{offset.x, offset.y};
}

float Renderer::lodBias() const { return m_applyTaa ? -1.f : 0.f; }
float Renderer::lodBias() const
{
// -1 seems like a safe value here since an 8 sample halton sequence is
// used. See A Survey of Temporal Antialiasing Techniques by Yang, Liu and
// Salvi for details.
return m_applyTaa ? -1.f : 0.f;
}

bool Renderer::rtInUse() const { return m_referenceRt || m_deferredRt; }

Expand Down
6 changes: 4 additions & 2 deletions src/render/RtReference.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ RtReference::Output RtReference::record(
if (gRenderResources.images->isValidHandle(m_previousIllumination))
gRenderResources.images->release(m_previousIllumination);

// Create dummy texture that won't be read from to satisfy binds
// Create dummy texture to satisfy binds even though it won't be
// read from
m_previousIllumination = gRenderResources.images->create(
ImageDescription{
.format = vk::Format::eR32G32B32A32Sfloat,
Expand All @@ -221,7 +222,8 @@ RtReference::Output RtReference::record(
"previousRTIllumination");
m_accumulationDirty = true;
}
else // We clear debug names each frame
else
// We clear debug names each frame
gRenderResources.images->appendDebugName(
m_previousIllumination, "previousRTIllumination");

Expand Down
8 changes: 4 additions & 4 deletions src/render/TemporalAntiAliasing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,9 @@ uint32_t pcFlags(PCBlock::Flags flags)
ret |= (uint32_t)flags.ignoreHistory;
ret |= (uint32_t)flags.catmullRom << 1;
ret |= (uint32_t)flags.colorClipping << 2;
// Two bits reserved for color clipping type
static_assert(
(uint32_t)TemporalAntiAliasing::ColorClippingType::Count - 1 < 0b11);
ret |= (uint32_t)flags.velocitySampling << 4;
// Two bits reserved for velocity sampling type
static_assert(
(uint32_t)TemporalAntiAliasing::VelocitySamplingType::Count - 1 < 0b11);
ret |= (uint32_t)flags.luminanceWeighting << 6;
Expand Down Expand Up @@ -169,12 +167,14 @@ TemporalAntiAliasing::Output TemporalAntiAliasing::record(
if (gRenderResources.images->isValidHandle(m_previousResolveOutput))
gRenderResources.images->release(m_previousResolveOutput);

// Create dummy texture that won't be read from to satisfy binds
// Create dummy texture to satisfy binds even though it won't be
// read from
m_previousResolveOutput =
createIllumination(renderExtent, resolveDebugName);
ignoreHistory = true;
}
else // We clear debug names each frame
else
// We clear debug names each frame
gRenderResources.images->appendDebugName(
m_previousResolveOutput, resolveDebugName);

Expand Down
1 change: 0 additions & 1 deletion src/render/TextureDebug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ uint32_t pcFlags(PCBlock::Flags flags)
uint32_t ret = 0;

ret |= (uint32_t)flags.channelType;
// Three bits reserved for the channel type
static_assert((uint32_t)TextureDebug::ChannelType::Count - 1 < 0b111);
ret |= (uint32_t)flags.absBeforeRange << 3;
ret |= (uint32_t)flags.zoom << 4;
Expand Down
1 change: 0 additions & 1 deletion src/scene/Camera.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ struct CameraParameters
float focalLength{0.f};
};

// Vector types in uniforms need to be aligned to 16 bytes
struct CameraUniforms
{
glm::mat4 worldToCamera;
Expand Down
1 change: 0 additions & 1 deletion src/scene/DebugGeometry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

struct DebugLines
{
// Writing more than 100k lines per frame sounds slow
static const vk::DeviceSize sMaxLineCount = 100'000;
// A line is two positions and a color
static const vk::DeviceSize sLineBytes = sizeof(float) * 9;
Expand Down
42 changes: 15 additions & 27 deletions src/scene/DeferredLoadingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ void loadNextTexture(DeferredLoadingContext *ctx)
Texture2D tex;
tex.init(
ScopedScratch{scopeBacking}, ctx->sceneDir / image.uri, ctx->cb,
ctx->stagingBuffers[0],
ctx->stagingBuffer,
Texture2DOptions{
.generateMipMaps = true,
.colorSpace = colorSpace,
Expand Down Expand Up @@ -1051,24 +1051,6 @@ void loadingWorker(DeferredLoadingContext *ctx)

} // namespace

Buffer createTextureStaging()
{
// Assume at most 4k at 8bits per channel
const vk::DeviceSize stagingSize = static_cast<size_t>(4096) *
static_cast<size_t>(4096) *
sizeof(uint32_t);
return gDevice.createBuffer(BufferCreateInfo{
.desc =
BufferDescription{
.byteSize = stagingSize,
.usage = vk::BufferUsageFlagBits::eTransferSrc,
.properties = vk::MemoryPropertyFlagBits::eHostVisible |
vk::MemoryPropertyFlagBits::eHostCoherent,
},
.debugName = "Texture2DStaging",
});
}

DeferredLoadingContext::~DeferredLoadingContext()
{
// Don't check for m_initialized as we might be cleaning up after a failed
Expand All @@ -1079,8 +1061,7 @@ DeferredLoadingContext::~DeferredLoadingContext()
worker->join();
}

for (Buffer &buffer : stagingBuffers)
gDevice.destroy(buffer);
gDevice.destroy(stagingBuffer);

gDevice.destroy(geometryUploadBuffer);
cgltf_free(gltfData);
Expand All @@ -1104,10 +1085,19 @@ void DeferredLoadingContext::init(
loadedTextures.reserve(gltfData->images_count);
materials.reserve(gltfData->materials_count);

// One of these is used by the worker implementation, all by the
// single threaded one
for (uint32_t i = 0; i < stagingBuffers.capacity(); ++i)
stagingBuffers[i] = createTextureStaging();
const vk::DeviceSize stagingSize = static_cast<size_t>(4096) *
static_cast<size_t>(4096) *
sizeof(uint32_t);
stagingBuffer = gDevice.createBuffer(BufferCreateInfo{
.desc =
BufferDescription{
.byteSize = stagingSize,
.usage = vk::BufferUsageFlagBits::eTransferSrc,
.properties = vk::MemoryPropertyFlagBits::eHostVisible |
vk::MemoryPropertyFlagBits::eHostCoherent,
},
.debugName = "Texture2DStaging",
});

geometryUploadBuffer = gDevice.createBuffer(BufferCreateInfo{
.desc =
Expand All @@ -1129,7 +1119,6 @@ void DeferredLoadingContext::launch()
WHEELS_ASSERT(
!worker.has_value() && "Tried to launch deferred loading worker twice");

// Fill sets to query image colorspaces from
for (const cgltf_material &material :
Span{gltfData->materials, gltfData->materials_count})
{
Expand Down Expand Up @@ -1227,7 +1216,6 @@ UploadedGeometryData DeferredLoadingContext::uploadGeometryData(
// count of the previous one.
geometryBufferRemainingByteCounts[dstBufferI] -= cacheHeader.blobByteCount;

// Offsets into GPU buffer are for u32
const uint32_t startOffsetU32 =
startByteOffset / asserted_cast<uint32_t>(sizeof(uint32_t));
const uint32_t startOffsetU16 =
Expand Down
2 changes: 1 addition & 1 deletion src/scene/DeferredLoadingContext.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class DeferredLoadingContext
uint32_t loadedImageCount{0};
uint32_t loadedMaterialCount{0};
wheels::Array<Material> materials{gAllocators.loadingWorker};
wheels::StaticArray<Buffer, MAX_FRAMES_IN_FLIGHT> stagingBuffers;
Buffer stagingBuffer;

private:
uint32_t getGeometryBuffer(uint32_t byteCount);
Expand Down
2 changes: 0 additions & 2 deletions src/scene/Texture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ struct UncompressedPixelData
uint32_t channels{0};
};

// Returns path of the corresponding cached file, creating the folders up to
// it if those don't exist
std::filesystem::path cachePath(const std::filesystem::path &source)
{
const auto cacheFolder = source.parent_path() / "prosper_cache";
Expand Down
3 changes: 0 additions & 3 deletions src/scene/World.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,6 @@ bool World::Impl::buildNextBlas(ScopedScratch scopeAlloc, vk::CommandBuffer cb)

buildInfo.scratchData = scratchBuffer.deviceAddress;

// Make sure we can use the scratch
scratchBuffer.transition(cb, BufferState::AccelerationStructureBuild);

const vk::AccelerationStructureBuildRangeInfoKHR *pRangeInfo =
Expand Down Expand Up @@ -795,7 +794,6 @@ void World::Impl::buildCurrentTlas(vk::CommandBuffer cb)

Buffer &World::Impl::reserveScratch(vk::DeviceSize byteSize)
{
// See if we have a big enough buffer available
for (ScratchBuffer &sb : m_scratchBuffers)
{
// Don't check for use within this frame as we assume barriers will be
Expand All @@ -807,7 +805,6 @@ Buffer &World::Impl::reserveScratch(vk::DeviceSize byteSize)
}
}

// Didn't find a viable buffer so allocate a new one
m_scratchBuffers.push_back(ScratchBuffer{
.buffer = gDevice.createBuffer(BufferCreateInfo{
.desc =
Expand Down
2 changes: 0 additions & 2 deletions src/utils/Profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ GpuFrameProfiler::GpuFrameProfiler(GpuFrameProfiler &&other) noexcept
, m_queryScopeIndices{WHEELS_MOV(other.m_queryScopeIndices)}
, m_scopeHasStats{WHEELS_MOV(other.m_scopeHasStats)}
{
// Avoid dtor destroying what we moved
other.m_timestampBuffer.handle = vk::Buffer{};
other.m_statisticsBuffer.handle = vk::Buffer{};
other.m_pools.statistics = vk::QueryPool{};
Expand All @@ -96,7 +95,6 @@ GpuFrameProfiler &GpuFrameProfiler::operator=(GpuFrameProfiler &&other) noexcept
m_queryScopeIndices = WHEELS_MOV(other.m_queryScopeIndices);
m_scopeHasStats = WHEELS_MOV(other.m_scopeHasStats);

// Avoid dtor destroying what we moved
other.m_timestampBuffer.handle = vk::Buffer{};
other.m_statisticsBuffer.handle = vk::Buffer{};
other.m_pools.statistics = vk::QueryPool{};
Expand Down
2 changes: 0 additions & 2 deletions src/utils/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ std::filesystem::path binPath(const std::filesystem::path &path)

String readFileString(Allocator &alloc, const std::filesystem::path &path)
{
// Open from end to find size from initial position
std::ifstream file(path, std::ios::ate | std::ios::binary);
if (!file.is_open())
throw std::runtime_error(
Expand All @@ -60,7 +59,6 @@ String readFileString(Allocator &alloc, const std::filesystem::path &path)
String buffer{alloc, fileSize};
buffer.resize(fileSize);

// Seek to beginning and read
file.seekg(0);
file.read(
reinterpret_cast<char *>(buffer.data()),
Expand Down

0 comments on commit e4097d1

Please sign in to comment.