From fd210218b14b7717814d4042bb6a44e1ac2992cd Mon Sep 17 00:00:00 2001 From: Arkex-ai <279266118+Arkex-ai@users.noreply.github.com> Date: Tue, 10 Nov 2026 08:11:23 -0300 Subject: [PATCH 1/2] fix(vk): make frame dump copies visible to host reads Assisted-by: OpenAI Codex --- docs/dump-readback.md | 38 +++++++ gpu/vk/vk_dump.cpp | 19 ++++ tests/dump_readback_test.cpp | 193 +++++++++++++++++++++++++++++++++++ tests/run_all.py | 1 + tests/test_dump_readback.py | 56 ++++++++++ 5 files changed, 307 insertions(+) create mode 100644 docs/dump-readback.md create mode 100644 tests/dump_readback_test.cpp create mode 100644 tests/test_dump_readback.py diff --git a/docs/dump-readback.md b/docs/dump-readback.md new file mode 100644 index 000000000..b34fd7a41 --- /dev/null +++ b/docs/dump-readback.md @@ -0,0 +1,38 @@ +# Frame dump readback visibility + +Frame dumps are diagnostic inputs: their statistics, images and raw float data +must describe the completed GPU copy. `vk_dump.cpp` allocates mapped staging +buffers with `create_buffer`, which does not require a host-coherent memory type. +Waiting for completion alone does not establish every memory-domain/cache step. + +Each dump copy records a COPY/TRANSFER_WRITE to HOST/HOST_READ dependency. The +existing replay caller flushes/waits its submission outside an open burst before +calling `dump_write`. The consumer then invalidates the copied allocation range +before reading pixels or storage-buffer words. VMA handles non-coherent atom +alignment and makes invalidation a no-op where host coherence permits it. + +Typed color copies contain RGBA32 float, unsigned or signed integer texels. +Depth and RGB9E5 copies retain their original dimensions and two/four-byte packed +texels until invalidation; depth conversion may then downscale or replace the +mapped pointer with temporary float output. Invalidating after that conversion +would already be too late and could use the wrong range. Buffer dumps invalidate +their actual capped, word-aligned copied size, including the final word. + +This changes only the opt-in dump path. Existing GPU barriers, submission waits, +allocation policy, file layout, numeric conversions and cleanup are preserved. +It does not make PNG/raw files a single atomic transaction or change generic +device-loss/wait-error handling. The readback contract assumes successful GPU +completion, as the existing dump caller does. + +`tests/test_dump_readback.py` compiles the actual dump translation unit, actual +full-barrier implementation and dump consumer call into an authored CPU model. +Its 56 scenarios cover coherent/non-coherent caches, explicit/permissive host +visibility, typed and packed formats, downscaled depth, buffer tail, raw float +data, statistics-only output, open-burst deferral and cleanup. Copy data, waits, +cache behavior and Vulkan calls are modeled; PNG encoding is outside this test. +This is protocol/numeric regression coverage, not proof of physical non-coherent +hardware, driver behavior, campaign rendering or a performance improvement. + +References: [Khronos readback example](https://docs.vulkan.org/guide/latest/synchronization_examples.html#cpu-read-back-of-data-written-by-a-compute-shader), +[Vulkan memory domains](https://docs.vulkan.org/spec/latest/appendices/memorymodel.html), +[VMA mapped-memory cache management](https://gpuopen-librariesandsdks.github.io/VulkanMemoryAllocator/html/memory_mapping.html). diff --git a/gpu/vk/vk_dump.cpp b/gpu/vk/vk_dump.cpp index afc56b096..fb663c92c 100644 --- a/gpu/vk/vk_dump.cpp +++ b/gpu/vk/vk_dump.cpp @@ -80,6 +80,19 @@ void barrier(VkCommandBuffer cmd, VkImage image, VkImageLayout from, VkImageLayo vkCmdPipelineBarrier2(cmd, &d); } +// Completion alone does not make the staging copy available to host reads. +void host_read_barrier(VkCommandBuffer cmd) { + VkMemoryBarrier2 b{VK_STRUCTURE_TYPE_MEMORY_BARRIER_2}; + b.srcStageMask = VK_PIPELINE_STAGE_2_COPY_BIT; + b.srcAccessMask = VK_ACCESS_2_TRANSFER_WRITE_BIT; + b.dstStageMask = VK_PIPELINE_STAGE_2_HOST_BIT; + b.dstAccessMask = VK_ACCESS_2_HOST_READ_BIT; + VkDependencyInfo d{VK_STRUCTURE_TYPE_DEPENDENCY_INFO}; + d.memoryBarrierCount = 1; + d.pMemoryBarriers = &b; + vkCmdPipelineBarrier2(cmd, &d); +} + uint8_t unit(float v) { return (uint8_t)std::clamp((int)std::lround(v * 255.0f), 0, 255); } @@ -341,6 +354,7 @@ void dump_image(VkCommandBuffer cmd, Texture* tex, uint32_t level, uint32_t laye std::min(layer, tex->layers - 1), 1}, {0, 0, 0}, {it.w, it.h, 1}}; vkCmdCopyImageToBuffer(cmd, tex->image, VK_IMAGE_LAYOUT_GENERAL, it.buffer, 1, &c); + host_read_barrier(cmd); full_barrier(cmd); items.push_back(std::move(it)); return; @@ -389,6 +403,7 @@ void dump_image(VkCommandBuffer cmd, Texture* tex, uint32_t level, uint32_t laye barrier(cmd, it.image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); const VkBufferImageCopy c{0, 0, 0, {VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1}, {0, 0, 0}, {it.w, it.h, 1}}; vkCmdCopyImageToBuffer(cmd, it.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, it.buffer, 1, &c); + host_read_barrier(cmd); full_barrier(cmd); items.push_back(std::move(it)); } @@ -422,6 +437,7 @@ void dump_buffer(VkCommandBuffer cmd, VkBuffer src, VkDeviceSize off, VkDeviceSi vkCmdPipelineBarrier2(cmd, &d); const VkBufferCopy c{off, 0, n}; vkCmdCopyBuffer(cmd, src, it.buffer, 1, &c); + host_read_barrier(cmd); vkCmdPipelineBarrier2(cmd, &d); buf_items.push_back(std::move(it)); } @@ -432,12 +448,15 @@ bool dump_pending() { void dump_write() { for (Item& it : items) { + // Before write_item converts depth and changes its dimensions/map. + invalidate_mapped(it.buffer_alloc, (uint64_t)it.w * it.h * (it.depth_bytes ? it.depth_bytes : 16)); write_item(it); destroy_buffer(it.buffer, it.buffer_alloc); if (it.image) destroy_image(it.image, it.image_alloc); } items.clear(); for (BufItem& it : buf_items) { + invalidate_mapped(it.alloc, it.n); const uint32_t* w = (const uint32_t*)it.map; const size_t nw = it.n / 4; size_t nz = 0; diff --git a/tests/dump_readback_test.cpp b/tests/dump_readback_test.cpp new file mode 100644 index 000000000..e2eca5e35 --- /dev/null +++ b/tests/dump_readback_test.cpp @@ -0,0 +1,193 @@ +#define VK_NO_PROTOTYPES +#define _CRT_SECURE_NO_WARNINGS +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace mkwvk { +struct Allocation { + std::vector gpu, cpu; + bool copied=false, host=false, completed=false, destroyed=false; +}; +using VmaAllocation=Allocation*; +struct Texture { + VkImage image{}; VkSampleCountFlagBits samples=VK_SAMPLE_COUNT_1_BIT; + VkImageAspectFlags aspect=VK_IMAGE_ASPECT_COLOR_BIT; + VkFormat format=VK_FORMAT_R32G32B32A32_SFLOAT; + uint64_t gpu=0; int id=1; + uint32_t levels=1,width=2,height=1,layers=1,depth=1; + VkImageType type=VK_IMAGE_TYPE_2D; +}; +struct Device {VkPhysicalDevice physical{};} dev; +std::vector> allocations; +std::vector payload, png; +std::vector lines; +bool coherent=false, implicit_host=false; +int invalidations=0; +int waits=0; +std::atomic par_active{false}; +std::atomic work_epoch{0}; +VkCommandBuffer last_barrier_cmd{}; +uint64_t last_barrier_epoch=UINT64_MAX; +std::atomic st_barriers_left_out{0}, st_barriers_made{0}; +struct GprofSpan { GprofSpan(VkCommandBuffer,const char*){} }; +void logf(const char* format,...) { + char b[2048];va_list a;va_start(a,format);vsnprintf(b,sizeof b,format,a);va_end(a);lines.emplace_back(b); +} +bool create_buffer(uint64_t n,VkBufferUsageFlags,bool,VkBuffer& b,VmaAllocation& a,void** map) { + auto x=std::make_unique();x->gpu.resize(n);x->cpu.resize(n); + a=x.get();b=reinterpret_cast(a);*map=x->cpu.data();allocations.push_back(std::move(x));return true; +} +void destroy_buffer(VkBuffer,VmaAllocation a){a->destroyed=true;} +bool create_image(const VkImageCreateInfo&,VkImage& i,VmaAllocation& a){i=reinterpret_cast(2);a=nullptr;return true;} +void destroy_image(VkImage,VmaAllocation){} +void invalidate_mapped(VmaAllocation a,uint64_t n) { + if(!a->completed || n!=a->gpu.size()) {std::fprintf(stderr,"bad invalidate order/range\n");std::exit(9);} + invalidations++; + if(a->host) a->cpu=a->gpu; +} +void vkGetPhysicalDeviceFormatProperties(VkPhysicalDevice,VkFormat,VkFormatProperties* p){p->optimalTilingFeatures=VK_FORMAT_FEATURE_BLIT_SRC_BIT;} +void vkCmdPipelineBarrier2(VkCommandBuffer,const VkDependencyInfo* d) { + for(uint32_t i=0;imemoryBarrierCount;i++) { + const auto& b=d->pMemoryBarriers[i]; + bool transfer=(b.srcStageMask&(VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT|VK_PIPELINE_STAGE_2_ALL_TRANSFER_BIT|VK_PIPELINE_STAGE_2_COPY_BIT))!=0; + bool writes=(b.srcAccessMask&(VK_ACCESS_2_MEMORY_WRITE_BIT|VK_ACCESS_2_TRANSFER_WRITE_BIT))!=0; + if(transfer && writes && (b.dstStageMask&VK_PIPELINE_STAGE_2_HOST_BIT) && (b.dstAccessMask&(VK_ACCESS_2_HOST_READ_BIT|VK_ACCESS_2_MEMORY_READ_BIT))) + for(auto& a:allocations) if(a->copied&&!a->destroyed)a->host=true; + } + work_epoch++; +} +void copy_to(VkBuffer b,size_t n) { + auto a=reinterpret_cast(b); + if(n!=a->gpu.size()||payload.size()!=n){std::fprintf(stderr,"bad copy range\n");std::exit(8);} + a->gpu=payload;a->copied=true;a->host=implicit_host;work_epoch++; +} +void vkCmdCopyImageToBuffer(VkCommandBuffer,VkImage,VkImageLayout,VkBuffer b,uint32_t,const VkBufferImageCopy*){copy_to(b,payload.size());} +void vkCmdCopyBuffer(VkCommandBuffer,VkBuffer,VkBuffer b,uint32_t,const VkBufferCopy* c){copy_to(b,c->size);} +void vkCmdBlitImage(VkCommandBuffer,VkImage,VkImageLayout,VkImage,VkImageLayout,uint32_t,const VkImageBlit*,VkFilter){work_epoch++;} +void wait_value(uint64_t) { + waits++; + for(auto& a:allocations) if(a->copied&&!a->destroyed){a->completed=true;if(coherent&&a->host)a->cpu=a->gpu;} +} +void write_png(const char*,const uint8_t* p,uint32_t w,uint32_t h){png.assign(p,p+(size_t)w*h*4);} +int _mkdir(const char*){return 0;} +/* FULL_BARRIER */ +} // namespace mkwvk + +/* SOURCE */ + +namespace mkwvk { +void consume_dump(bool burst_open) { + const uint64_t value=1; + /* CALLER */ +} +} + +void require(bool ok,const char* what) { + if(!ok){std::fprintf(stderr,"FAIL %s\n",what);std::exit(1);} +} +template void upload(const std::vector& v) { + mkwvk::payload.resize(v.size()*sizeof(T)); + std::memcpy(mkwvk::payload.data(),v.data(),mkwvk::payload.size()); +} + +int main(int argc,char** argv) { + if(argc!=4)return 2; + using namespace mkwvk; + const std::string which=argv[1]; + dir="."; + coherent=atoi(argv[2])!=0;implicit_host=atoi(argv[3])!=0; + Texture t;t.image=reinterpret_cast(1); + const auto cmd=reinterpret_cast(1); + std::vector expected{64,128,191,255,128,64,255,255}; + std::vector floats{.25f,.5f,.75f,1.f,.5f,.25f,1.f,1.f}; + if(which=="empty") { + consume_dump(false);require(waits==0 && invalidations==0 && png.empty(),"empty queue reads/waits");return 0; + } + armed=true; + if(which=="ssbo" || which=="ssbo-tail") { + std::vector words(which=="ssbo"?8:16384); + words[1]=1;words[3]=2;words.back()=3;upload(words); + dump_buffer(cmd,reinterpret_cast(5),12,which=="ssbo"?35:65539,"ssbo"); + expected.clear(); + } else { + if(which=="uint") { + t.format=VK_FORMAT_R32G32B32A32_UINT; + upload(std::vector{0,5,4000000000u,1,100,10,UINT32_MAX,1}); + expected={0,0,0,255,255,255,255,255}; + } else if(which=="sint") { + t.format=VK_FORMAT_R32G32B32A32_SINT; + upload(std::vector{INT32_MIN,-5,-2,1,INT32_MAX,5,2,1}); + expected={0,0,0,255,255,255,255,255}; + } else if(which=="nan-inf") { + t.width=3; + floats={.25f,.5f,.75f,1.f,std::numeric_limits::quiet_NaN(),0,0,1, + std::numeric_limits::infinity(),0,0,1}; + upload(floats);expected={64,128,191,255,255,0,255,255,0,255,0,255}; + } else if(which=="d16" || which=="d24" || which=="d32" || which=="depth-scaled") { + t.aspect=VK_IMAGE_ASPECT_DEPTH_BIT; + t.width=which=="depth-scaled"?64:4; + t.format=which=="d16"?VK_FORMAT_D16_UNORM:which=="d24"?VK_FORMAT_D24_UNORM_S8_UINT:VK_FORMAT_D32_SFLOAT; + if(which=="d16")upload(std::vector{0,16384,32768,65535}); + else if(which=="d24")upload(std::vector{0xaa000000,0xbb400000,0xcc800000,0xddffffff}); + else { + floats.resize(t.width); + for(uint32_t i=0;i{(16u<<27)|(256u<<18)|(128u<<9)|64u, + (16u<<27)|(64u<<18)|(256u<<9)|128u}); + expected={64,128,255,255,128,255,64,255}; + } else upload(floats); + dump_image(cmd,&t,0,0,which.c_str()); + } + require(dump_pending() && allocations.size()==1,"producer queues copy"); + consume_dump(true); + require(waits==0 && invalidations==0 && png.empty() && !allocations[0]->destroyed,"open burst deferred"); + consume_dump(false); + require(waits==1,"completed-submission wait"); + if(which=="ssbo" || which=="ssbo-tail") { + const auto wanted=which=="ssbo"?"3 of 8 words not zero:":"3 of 16384 words not zero:"; + require(lines.size()==1 && lines[0].find(wanted)!=std::string::npos,"actual buffer nonzero statistics"); + require(lines[0].find(which=="ssbo"?"[7]=0x3":"[16383]=0x3")!=std::string::npos,"buffer tail visible"); + } else if(which=="stats") { + require(png.empty() && lines.size()==1 && lines[0].find("mean 0.375 0.375 0.875 1")!=std::string::npos,"statistics-only values"); + } else { + require(png==expected,"actual typed dump output"); + if(which=="nan-inf")require(lines[0].find("NaN 1, Inf 1 of 3")!=std::string::npos,"NaN/Inf statistics"); + if(which=="raw") { + std::ifstream f; + for(const auto& entry:std::filesystem::directory_iterator(".")) + if(entry.path().extension()==".f32")f=std::ifstream(entry.path(),std::ios::binary); + std::vector bytes((std::istreambuf_iterator(f)),{}); + require(bytes.size()==8+floats.size()*sizeof(float),"raw size"); + uint32_t wh[2]{};memcpy(wh,bytes.data(),8); + require(wh[0]==2 && wh[1]==1 && !memcmp(bytes.data()+8,floats.data(),floats.size()*4),"raw dimensions/pixels"); + } + } + require(!dump_pending() && allocations[0]->destroyed,"resource cleanup"); + consume_dump(false);require(waits==1,"consumed queue does not wait again"); + std::puts("PASS actual dump readback output, burst deferral, wait and cleanup"); +} diff --git a/tests/run_all.py b/tests/run_all.py index 2993de8bf..191f10f6b 100644 --- a/tests/run_all.py +++ b/tests/run_all.py @@ -41,6 +41,7 @@ "tests/test_capture_copy_runs.py": ["--cxx", "{cxx}"], "tests/test_flash_sampling.py": ["--cxx", "{cxx}"], "tests/test_capture_dimensions.py": ["--cxx", "{cxx}", "--headers", "{vulkan_headers}"], + "tests/test_dump_readback.py": ["--cxx", "{cxx}", "--headers", "{vulkan_headers}"], "tests/test_ui_texture_upload.py": ["--cxx", "{cxx}", "--headers", "{vulkan_headers}"], "tests/test_multiwait_wake.py": ["--cxx", "{cxx}"], "tests/test_parallel_rb_refresh.py": ["--cxx", "{cxx}"], diff --git a/tests/test_dump_readback.py b/tests/test_dump_readback.py new file mode 100644 index 000000000..d4dff3cd6 --- /dev/null +++ b/tests/test_dump_readback.py @@ -0,0 +1,56 @@ +"""Exercise actual dump copies, host visibility and typed consumers without a GPU.""" +import argparse +from pathlib import Path +import os +import subprocess +import tempfile + +ROOT=Path(__file__).resolve().parents[1] + + +def block(s,marker): + start=s.index(marker) + at=s.index('{',start)+1 + depth=1 + while depth: + depth+=(s[at]=='{')-(s[at]=='}') + at+=1 + return s[start:at] + + +def main(): + p=argparse.ArgumentParser(description=__doc__) + p.add_argument('--source',type=Path,default=ROOT) + p.add_argument('--headers',type=Path,required=True) + p.add_argument('--cxx',default='g++') + p.add_argument('--sanitize',action='store_true') + a=p.parse_args() + source=(a.source/'gpu/vk/vk_dump.cpp').read_text(encoding='utf-8') + source='\n'.join(l for l in source.splitlines() if not l.startswith('#include')) + texture=(a.source/'gpu/vk/vk_texture.cpp').read_text(encoding='utf-8') + replay=(a.source/'gpu/vk/vk_replay.cpp').read_text(encoding='utf-8') + caller='if (!burst_open && dump_pending()) wait_value(value), dump_write();' + assert caller in replay + fixture=(ROOT/'tests/dump_readback_test.cpp').read_text(encoding='utf-8') + fixture=fixture.replace('/* FULL_BARRIER */',block(texture,'void full_barrier(').replace('const char* why)', 'const char* why = "test")')) + fixture=fixture.replace('/* SOURCE */',source).replace('/* CALLER */',caller) + with tempfile.TemporaryDirectory(prefix='dump-readback-') as tmp: + tmp=Path(tmp) + cpp,exe=tmp/'fixture.cpp',tmp/'fixture.exe' + cpp.write_text(fixture,encoding='utf-8') + flags=['-fsanitize=address,undefined','-fno-omit-frame-pointer'] if a.sanitize else [] + subprocess.run([a.cxx,'-std=c++20','-O1','-Wall','-Wextra','-Werror','-Wno-missing-field-initializers',*flags,'-I'+str(a.headers),str(cpp),'-o',str(exe)],check=True) + cases=['float','uint','sint','nan-inf','d16','d24','d32','depth-scaled','rgb9e5','ssbo','ssbo-tail','raw','stats','empty'] + for case in cases: + for coherent,implicit in [(1,1),(1,0),(0,1),(0,0)]: + env={k:v for k,v in os.environ.items() if not k.startswith('RX_VK_DUMP')} + env['RX_VK_DUMP_W']='16' + if case=='rgb9e5':env['RX_VK_DUMP_9E5']='1' + if case=='raw':env['RX_VK_DUMP_RAW']='1' + if case=='stats':env['RX_VK_DUMP_NOPNG']='1' + with tempfile.TemporaryDirectory(dir=tmp) as run: + subprocess.run([str(exe),case,str(coherent),str(implicit)],cwd=run,env=env,check=True,timeout=15,stdout=subprocess.DEVNULL) + print('PASS 56 actual dump readback scenarios: typed output, cache/domain controls, wait and cleanup') + + +if __name__=='__main__':main() From f445caa0609b1f4d1a9928c261280be51955a0ff Mon Sep 17 00:00:00 2001 From: Arkex-ai <279266118+Arkex-ai@users.noreply.github.com> Date: Tue, 10 Nov 2026 08:28:40 -0300 Subject: [PATCH 2/2] fix(vk): type the packed dump aspect as Vulkan flags Assisted-by: OpenAI Codex --- gpu/vk/vk_dump.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gpu/vk/vk_dump.cpp b/gpu/vk/vk_dump.cpp index fb663c92c..0feeb54b5 100644 --- a/gpu/vk/vk_dump.cpp +++ b/gpu/vk/vk_dump.cpp @@ -350,7 +350,7 @@ void dump_image(VkCommandBuffer cmd, Texture* tex, uint32_t level, uint32_t laye it.buffer_alloc, &it.map)) return; full_barrier(cmd); - const VkBufferImageCopy c{0, 0, 0, {raw_9e5 ? VK_IMAGE_ASPECT_COLOR_BIT : VK_IMAGE_ASPECT_DEPTH_BIT, level, + const VkBufferImageCopy c{0, 0, 0, {(VkImageAspectFlags)(raw_9e5 ? VK_IMAGE_ASPECT_COLOR_BIT : VK_IMAGE_ASPECT_DEPTH_BIT), level, std::min(layer, tex->layers - 1), 1}, {0, 0, 0}, {it.w, it.h, 1}}; vkCmdCopyImageToBuffer(cmd, tex->image, VK_IMAGE_LAYOUT_GENERAL, it.buffer, 1, &c);