CAD DTM Rendering #186
CAD DTM Rendering #186
Conversation
…Examples-and-Tests into dtm
62_CAD/shaders/globals.hlsl
Outdated
| @@ -120,7 +126,7 @@ enum class MajorAxis : uint32_t | |||
| struct MainObject | |||
| { | |||
| uint32_t styleIdx; | |||
| uint32_t pad; // do I even need this on the gpu side? it's stored in structured buffer not bda | |||
| uint32_t dtmSettingsIdx; // do I even need this on the gpu side? it's stored in structured buffer not bda | |||
There was a problem hiding this comment.
remove my comment, realize it was dumb, of course the pad was needed.
| }; | ||
|
|
||
| // Set 0 - Scene Data and Globals, buffer bindings don't change the buffers only get updated | ||
| [[vk::binding(0, 0)]] ConstantBuffer<Globals> globals : register(b0); | ||
| [[vk::binding(1, 0)]] StructuredBuffer<DrawObject> drawObjects : register(t0); | ||
| [[vk::binding(2, 0)]] StructuredBuffer<MainObject> mainObjects : register(t1); | ||
| [[vk::binding(3, 0)]] StructuredBuffer<LineStyle> lineStyles : register(t2); | ||
| [[vk::binding(4, 0)]] StructuredBuffer<DTMSettings> dtmSettingsBuff : register(t3); |
There was a problem hiding this comment.
small rename, just dtmSettings
| @@ -265,6 +271,12 @@ NBL_CONSTEXPR float InvalidStyleStretchValue = nbl::hlsl::numeric_limits<float>: | |||
| // TODO[Przemek]: we will need something similar to LineStyles but related to heigh shading settings which is user customizable (like LineStyle stipple patterns) and requires upper_bound to figure out the color based on height value. | |||
| // We'll discuss that later or what it will be looking like and how it's gonna get passed to our shaders. | |||
|
|
|||
| struct TriangleMeshVertex | |||
There was a problem hiding this comment.
If this is your vertex data you pull in vertex buffer, it needs to be in double precision.
- no need for this struct, just use a
pfloat64_t3the z value is height
There was a problem hiding this comment.
does it matter when its this struct or pfloat64_t3?
There was a problem hiding this comment.
if performance wise it doesn't matter then i would prefer to keep the struct
There was a problem hiding this comment.
it makes code a little bit more self explanatory
| outV.setObjType(ObjectType::TRIANGLE_MESH); | ||
| outV.setMainObjectIdx(pc.triangleMeshMainObjectIndex); | ||
|
|
||
| TriangleMeshVertex vtx = vk::RawBufferLoad<TriangleMeshVertex>(pc.triangleMeshVerticesBaseAddress + sizeof(TriangleMeshVertex) * vertexID, 4u); |
There was a problem hiding this comment.
pull a pfloat64_t3 and change alignment to 8u
|
|
||
| outV.position.xy = transformedPos; | ||
| outV.position = transformFromSreenSpaceToNdc(outV.position.xy, globals.resolution); | ||
| outV.setHeightAtMeshVertex(vtx.height); |
There was a problem hiding this comment.
I think height being in float is just fine.
So you could just cast the height value to float here, no need to worry about dealing with .z being double.
in the gpu-driven thing I'm doing I might make this height atrribute of each vertex a contigous separate block in floats instead of doubles that need to be casted.
for convenience, just use pfloat64_t3 for your vertices and cast the height to float (for now)
| // long story short, in order for stipple patterns to be consistent: | ||
| // - point with lesser x coord should be starting point | ||
| // - if x coord of both points are equal then point with lesser y value should be starting point | ||
| if (end.x < start.x) |
There was a problem hiding this comment.
do this checks to see whether to use end or start when constructing the line (before doing sdf of it in https://github.com/Devsh-Graphics-Programming/Nabla-Examples-and-Tests/pull/186/files#r2013523975)
62_CAD/CTriangleMesh.h
Outdated
| { | ||
| return sizeof(index_t) * m_indices.size(); | ||
| } | ||
| inline size_t getIdxCnt() const |
There was a problem hiding this comment.
Vtx -> Vertex
Idx -> Index
Buff -> Buffer
Cnt -> Count
Please :)
62_CAD/DrawResourcesFiller.cpp
Outdated
| @@ -84,7 +84,7 @@ void DrawResourcesFiller::allocateGeometryBuffer(ILogicalDevice* logicalDevice, | |||
|
|
|||
| IGPUBuffer::SCreationParams geometryCreationParams = {}; | |||
| geometryCreationParams.size = size; | |||
| geometryCreationParams.usage = bitflag(IGPUBuffer::EUF_STORAGE_BUFFER_BIT) | IGPUBuffer::EUF_SHADER_DEVICE_ADDRESS_BIT | IGPUBuffer::EUF_TRANSFER_DST_BIT; | |||
| geometryCreationParams.usage = bitflag(IGPUBuffer::EUF_STORAGE_BUFFER_BIT) | IGPUBuffer::EUF_SHADER_DEVICE_ADDRESS_BIT | IGPUBuffer::EUF_TRANSFER_DST_BIT | IGPUBuffer::EUF_INDEX_BUFFER_BIT; | |||
There was a problem hiding this comment.
add comment;
// INDEX_BUFFER USAGE for DTMs
62_CAD/DrawResourcesFiller.cpp
Outdated
| // copy into gemoetry cpu buffer insteaed | ||
|
|
||
| // TODO: rename, its not just points | ||
| const uint32_t maxGeometryBufferPoints = static_cast<uint32_t>(maxGeometryBufferSize - currentGeometryBufferSize); |
There was a problem hiding this comment.
Rename to remainingGeometryBufferSize
| const uint32_t maxGeometryBufferPoints = static_cast<uint32_t>(maxGeometryBufferSize - currentGeometryBufferSize); | ||
|
|
||
| // TODO: assert of geometry buffer size, do i need to check if size of objects to be added <= maxGeometryBufferPoints? | ||
| // TODO: auto submit instead of assert |
There was a problem hiding this comment.
Good Todo comment, we'll tackle that later together
62_CAD/DrawResourcesFiller.cpp
Outdated
| submitDraws(intendedNextSubmit); | ||
| resetGeometryCounters(); | ||
| resetMainObjectCounters(); | ||
| resetLineStyleCounters(); |
There was a problem hiding this comment.
I think here you also need a reset dtm settings counter
62_CAD/DrawResourcesFiller.cpp
Outdated
| { | ||
| bool success = true; | ||
| // Copy LineStyles | ||
| uint32_t remainingLineStyles = currentDTMSettingsCount - inMemDTMSettingsCount; |
There was a problem hiding this comment.
Comment and var rename needed here
62_CAD/DrawResourcesFiller.cpp
Outdated
|
|
||
| // TODO: this needs to be redone.. what if submit happens after that line? | ||
| // we need to make sure somehow that function below will not submit, we need both outline and contour styles in GPU memory | ||
| dtmSettings.outlineLineStyleIdx = addLineStyle_SubmitIfNeeded(dtmSettingsInfo.outlineLineStyleInfo, intendedNextSubmit); |
There was a problem hiding this comment.
Also Asset here that the max line styles is >= 2, because you need at least 2 resident lineStyles. And you don't want the second call to evict the first
62_CAD/DrawResourcesFiller.cpp
Outdated
| DTMSettings dtmSettings; | ||
|
|
||
| // TODO: this needs to be redone.. what if submit happens after that line? | ||
| // we need to make sure somehow that function below will not submit, we need both outline and contour styles in GPU memory |
There was a problem hiding this comment.
Yes you're right, even if you have many line styles and the first one fits into men, the second one might evict everything.
In the case that remainingLineStyles is < 2, do the auto submit calls and resets. And then call addLineStyle_Internal for both (asset they succeed, because they definitely should)
Also be careful to not call this in the middle of adding geometry that should reference this dtm settings.
62_CAD/DrawResourcesFiller.cpp
Outdated
| return i; | ||
| } | ||
|
|
||
| if (currentDTMSettingsCount >= maxDtmSettings) |
There was a problem hiding this comment.
We should be extra careful and document more robustly.
Auto submit can easily break with multiple _SubmitIfNeeded calls.
I'm sure I've stated explicitly somewhere in the comments that user shall not batch calls to addMainObj.
Make sure you add the same comments to line styles and dtm settings (it shouldn't be called twice or more in a row and then used via index)
62_CAD/DrawResourcesFiller.cpp
Outdated
| // TODO: auto submit instead of assert | ||
| assert(geometryBufferDataToAddByteSize <= maxGeometryBufferPoints); | ||
|
|
||
| // TODO: vertices need to be aligned to 8? |
There was a problem hiding this comment.
Yes I probably missed this in other places, we need to align to 8 bytes because the vertices have doubles in them. Keep this comment, I'll come back to it
62_CAD/DrawResourcesFiller.cpp
Outdated
| // TODO: vertices need to be aligned to 8? | ||
| uint64_t vtxBufferAddress; | ||
| { | ||
| void* dst = reinterpret_cast<char*>(cpuDrawBuffers.geometryBuffer->getPointer()) + currentGeometryBufferSize; |
There was a problem hiding this comment.
uint8_t please no char.
There was a problem hiding this comment.
Just store as uin8_t*, and advance it with+= and use the same pointer for the next copy
…y not storing any index but address to active + functions to begin/endMainObject and setActiveLineStyle
… what vertex buffers and index buffers look like
CAD DTM Rendering Example
Devsh-Graphics-Programming/Nabla#858