Conversation
| // resolve | ||
| if(useRWMC) | ||
| { | ||
| // TODO: shouldn't it be computed only at initialization stage and on window resize? |
There was a problem hiding this comment.
meh, it really doesn't matter much
| cmdbuf->bindDescriptorSets(EPBP_COMPUTE, pipeline->getLayout(), 0u, 1u, &m_descriptorSet0.get()); | ||
| cmdbuf->bindDescriptorSets(EPBP_COMPUTE, pipeline->getLayout(), 2u, 1u, &m_descriptorSet2.get()); |
There was a problem hiding this comment.
why not just use one descriptor set ?
There was a problem hiding this comment.
| uint64_t m_realFrameIx = 0; | ||
| std::array<smart_refctd_ptr<IGPUCommandBuffer>, MaxFramesInFlight> m_cmdBufs; | ||
| ISimpleManagedSurface::SAcquireResult m_currentImageAcquire = {}; | ||
| smart_refctd_ptr<IGPUDescriptorSet> m_descriptorSet0, m_descriptorSet2, m_presentDescriptorSet; |
There was a problem hiding this comment.
better names for the path tracing descriptor sets
| params.shader.entryPoint = "main"; | ||
| params.shader.entries = nullptr; | ||
| params.cached.requireFullSubgroups = true; | ||
| params.shader.requiredSubgroupSize = static_cast<IPipelineBase::SUBGROUP_SIZE>(5); |
There was a problem hiding this comment.
This will break on anything but Nvidia and it will break llvm-pipe
| m_cascadeView = createHDRIImageView(cascade, CascadeCount, IGPUImageView::ET_2D_ARRAY); | ||
| m_cascadeView->setObjectDebugName("Cascade View"); | ||
|
|
||
| // TODO: change cascade layout to general |
There was a problem hiding this comment.
not needed, you transition it from UNDEFINED to gENERAL every frame, that's sufficient
| #ifdef SPHERE_LIGHT | ||
| scene.light_spheres[0] = spheres[0]; | ||
| #endif | ||
| #ifdef TRIANGLE_LIGHT | ||
| scene.light_triangles[0] = triangles[0]; | ||
| #endif | ||
| #ifdef RECTANGLE_LIGHT | ||
| scene.light_rectangles[0] = rectangles[0]; | ||
| #endif |
There was a problem hiding this comment.
have an init() method, and hide that macro crap
|
|
||
| // mutable | ||
| scalar_type intersectionT; | ||
| ObjectID objectID; |
There was a problem hiding this comment.
to be returned in the intersection type of the intersector instead
| scalar_type intersectionT; | ||
| ObjectID objectID; | ||
|
|
||
| Payload<T> payload; |
There was a problem hiding this comment.
the whole payload should be a template arg
| isotropic_interaction_type iso_interaction; | ||
| anisotropic_interaction_type aniso_interaction; |
There was a problem hiding this comment.
nope, aniso has iso inside it
| struct SIntersectData | ||
| { | ||
| bool foundHit; | ||
| vector3_type intersection; |
There was a problem hiding this comment.
name it position
|
|
||
| struct SIntersectData | ||
| { | ||
| bool foundHit; |
There was a problem hiding this comment.
you don't need a separate bool, you can just check the position x coordinate is NaN
| return retval; | ||
| } | ||
|
|
||
| return_type operator()(uint32_t protoDimension, uint32_t _sample, uint32_t i) |
There was a problem hiding this comment.
what's the difference between _sample and i !?
There was a problem hiding this comment.
and protoDimension !?
There was a problem hiding this comment.
write comments about what they all mean, because right now its hard to remember
This is the mental effort I have to go to:
if protoDimension is supposed to be a LSB offset in the same sequence, and the _sample does MSB
but also the protoDimension doesn't have clear documented semantics
I also don't get what i is, its kinda like a sub-dimension or something
But your RawBufferLoad loads 3 dimensions at once.. so what is what!?
…does deferred pdf only
5 participants
@keptsecret after you merge
masteragain, you'll probably have the UI mess up and show changed from the merge commit, so close and reopen againTODO