diff --git a/devices/rtx/device/material/MDL.cpp b/devices/rtx/device/material/MDL.cpp index d4fb5e3ee..32bcf4a7d 100644 --- a/devices/rtx/device/material/MDL.cpp +++ b/devices/rtx/device/material/MDL.cpp @@ -229,6 +229,13 @@ void MDL::syncSource() std::optional materialName; if (hasParam("materialName")) materialName = getParamString("materialName", "main"); + // A subclass that generated its source THIS flush hands it over directly — + // the params it staged are invisible to the getters above until next flush. + if (m_sourceHandoff) { + sourceType = m_sourceHandoff->sourceType; + source = m_sourceHandoff->source; + materialName = m_sourceHandoff->materialName; + } // A change to any selection input triggers a full material reload. if (source == m_source && sourceType == m_sourceType @@ -244,7 +251,7 @@ void MDL::syncSource() const bool isMdle = libmdl::endsWith(source, ".mdle"); if (sourceType == "code") { - if (!hasParam("source")) { + if (!hasParam("source") && !m_sourceHandoff) { reportMessage(ANARI_SEVERITY_ERROR, "MDL::syncSource(): sourceType 'code' requires a 'source' parameter"); } else { diff --git a/devices/rtx/device/material/MDL.h b/devices/rtx/device/material/MDL.h index 29ba263c5..0359d5b4d 100644 --- a/devices/rtx/device/material/MDL.h +++ b/devices/rtx/device/material/MDL.h @@ -71,6 +71,25 @@ struct MDL : public Material // Handle argument block update void syncParameters(); + protected: + // Source handoff for subclasses that GENERATE their MDL source during + // commitParameters (MaterialX). This is the ONLY channel for the generated + // code — subclasses must NOT write it into source/sourceType/materialName: + // under helium snapshot-commit staged writes are invisible to the getters + // syncSource uses until the NEXT flush (the raw document path would compile + // as MDL code), and overwriting `source` destroys the app's document for + // later retranscodes. The app's params stay authoritative; subclasses + // re-populate this override every commit and syncSource prefers it when + // set. materialName nullopt = no selection (mirrors an absent + // "materialName" param). + struct SourceHandoff + { + std::string sourceType; + std::string source; + std::optional materialName; + }; + std::optional m_sourceHandoff; + private: MaterialGPUData gpuData() const override; std::map m_parameterMap; diff --git a/devices/rtx/device/material/MaterialX.cpp b/devices/rtx/device/material/MaterialX.cpp index f33032d53..f13a03166 100644 --- a/devices/rtx/device/material/MaterialX.cpp +++ b/devices/rtx/device/material/MaterialX.cpp @@ -66,9 +66,10 @@ MaterialX::~MaterialX() bool MaterialX::needsRetranscode() { - // A successful transcode overwrites source/sourceType/materialName with the - // generated MDL handoff (sourceType="code"). Recognize our own write so it is - // not mistaken for new user input; otherwise distinguish the app's values. + // The MDL handoff never overwrites source/sourceType/materialName (see + // commitParameters step 4) — the app's params stay authoritative here. The + // "recognize our own write" branches below remain only as tolerance for + // apps that stage 'code'/generated values directly. auto liveSource = getParamString("source", ""); auto liveSourceType = getParamString("sourceType", "documentFile"); @@ -94,7 +95,11 @@ bool MaterialX::needsRetranscode() // `source` carries the generated MDL after our handoff; recognize that so our // own output is not re-read as a user document. - std::string userPath = (liveSource == m_generatedSource) ? m_userPath : liveSource; + // Empty-vs-empty must not match: an app clearing `source` during an outage + // (m_generatedSource also "" after a FREEZE) is an unset, not our write. + std::string userPath = (!liveSource.empty() && liveSource == m_generatedSource) + ? m_userPath + : liveSource; std::optional userSelected; if (hasParam("materialName")) { @@ -278,23 +283,25 @@ void MaterialX::commitParameters() if (desired != m_texturedOrigins) transcode(desired); - // 4. Re-apply the MDL handoff on EVERY commit, not only when we re-transcoded. - // An app re-setting `source` to the .mtlx path (or helium re-staging the - // object) would otherwise leave the base reading a path as MDL code and - // silently fall back to diffuse. MDL::syncSource has its own content-based - // change detection, so an unchanged handoff is a cheap no-op (no recompile). - setParam("sourceType", ANARI_STRING, "code"); - setParam("source", ANARI_STRING, m_generatedSource.c_str()); - if (!m_generatedName.empty()) { - setParam("materialName", ANARI_STRING, m_generatedName.c_str()); - } else if (m_userSelected) { - // Fallback active (no generated material): keep the USER's selection in - // the param. Removing it would make the next needsRetranscode misread the - // absence as "user unset" and recover on the document's first material. - setParam("materialName", ANARI_STRING, m_userSelected->c_str()); - } else { - removeParam("materialName"); - } + // 4. Re-apply the MDL handoff on EVERY commit, not only when we + // re-transcoded, via the direct m_sourceHandoff — NEVER by overwriting the + // source/sourceType/materialName params. Under helium snapshot-commit, + // staged param writes are invisible to same-flush getters (the raw .mtlx + // path would compile as MDL code and fall back to diffuse), and an + // overwritten `source` would destroy the user's document for later + // retranscodes (the snapshot lags a flush behind m_generatedSource, so + // "recognize our own write" comparisons misfire after an outage FREEZE). + // The app's params stay authoritative; MDL::syncSource has its own + // content-based change detection, so an unchanged handoff is a cheap + // no-op (no recompile). + SourceHandoff handoff; + handoff.sourceType = "code"; + handoff.source = m_generatedSource; + if (!m_generatedName.empty()) + handoff.materialName = m_generatedName; + else if (m_userSelected) + handoff.materialName = *m_userSelected; + m_sourceHandoff = std::move(handoff); // 5. Route clean params -> value/texture args, then delegate. While the // fallback is active (no generated material) routing would CONSUME clean diff --git a/devices/rtx/device/material/MaterialX.h b/devices/rtx/device/material/MaterialX.h index 7f4987acd..5d5e118a6 100644 --- a/devices/rtx/device/material/MaterialX.h +++ b/devices/rtx/device/material/MaterialX.h @@ -53,9 +53,11 @@ struct MaterialX : public MDL private: // True if the user's source, sourceType, selection, or (for documentFile) the // file's mtime changed since the last successful transcode. Updates the cached - // user inputs as a side effect. Must distinguish the user's - // `source`/`sourceType`/`materialName` from the generated MDL + "code" we write - // back into those same params (see commitParameters). + // user inputs as a side effect. The app's `source`/`sourceType`/`materialName` + // params are never overwritten (the generated MDL travels via the + // MDL::SourceHandoff — see commitParameters), so the live params read here are + // authoritative user input; the "recognize our own write" branches remain only + // as tolerance for apps staging 'code'/generated values directly. bool needsRetranscode(); // Invoke the transcoder and update all generated state. @@ -81,8 +83,8 @@ struct MaterialX : public MDL // first commit always mismatch. uint64_t m_distributionGeneration{~uint64_t(0)}; - std::string m_generatedSource; // MDL written into the `source` param - std::string m_generatedName; // name written into the `materialName` param + std::string m_generatedSource; // MDL handed to the base via SourceHandoff + std::string m_generatedName; // material name handed via SourceHandoff std::vector m_materialNames; std::vector m_materialNamePtrs; std::vector m_textureInputNames; diff --git a/devices/rtx/device/material/PhysicallyBasedMDL.cpp b/devices/rtx/device/material/PhysicallyBasedMDL.cpp index c5589867c..864e87a6f 100644 --- a/devices/rtx/device/material/PhysicallyBasedMDL.cpp +++ b/devices/rtx/device/material/PhysicallyBasedMDL.cpp @@ -99,25 +99,37 @@ void PhysicallyBasedMDL::commitParameters() translateAndRemoveParameter("iridescence"sv); translateAndRemoveParameter("iridescenceThickness"sv); - // Capture the emissive binding AFTER translation, from the persistent - // post-translate keys: the translation consumes the pre-translate `emissive` - // key at its first commit, so reading that key here would zero the capture — - // and silently drop the Geometry Light — on any later commit that doesn't - // re-set `emissive`. The keys are mutually exclusive post-translate; the - // sampler gate mirrors the material's own texture-over-value precedence. + // Capture the emissive binding by mirroring the translation above. Under + // helium snapshot-commit, getters read the snapshot taken at + // anariCommitParameters() while setParam*() writes staging — so the + // post-translate keys the translation just wrote are INVISIBLE to getters + // until the next flush. A freshly set pre-translate `emissive` (visible in + // this flush's snapshot) therefore fully determines the binding; only when + // absent do the post-translate keys persisted by earlier flushes apply — + // which also keeps the capture alive on later commits that don't re-set + // `emissive` (the translation consumed the pre-translate key). // Constant iff a nonzero inline color is bound; a bound sampler goes through // the EDF path with a live sampler-mean Pick Power (see emissionAverage). // Note: unsetting `emissive` leaves the last translated key in place — a // pre-existing translation-lifecycle trait shared by the MDL argument block, // so the light stays consistent with the still-glowing surface. { - m_emissiveSampler = getParamObject("emissive.texture"); vec3 radiance(0.f); - if (!m_emissiveSampler) { - vec4 v(0.f); - getParam("emissive.value", ANARI_FLOAT32_VEC4, &v); - getParam("emissive.value", ANARI_FLOAT32_VEC3, &v); - radiance = vec3(v); + if (auto any = getParamDirect("emissive"); any.type() != ANARI_UNKNOWN) { + m_emissiveSampler = + any.type() == ANARI_SAMPLER ? any.getObject() : nullptr; + if (any.type() == ANARI_FLOAT32_VEC3) + radiance = any.get(); + else if (any.type() == ANARI_FLOAT32_VEC4) + radiance = vec3(any.get()); + } else { + m_emissiveSampler = getParamObject("emissive.texture"); + if (!m_emissiveSampler) { + vec4 v(0.f); + getParam("emissive.value", ANARI_FLOAT32_VEC4, &v); + getParam("emissive.value", ANARI_FLOAT32_VEC3, &v); + radiance = vec3(v); + } } m_emissionRadiance = radiance; m_emissionIsConstant = glm::any(glm::greaterThan(radiance, vec3(0.f))); diff --git a/devices/rtx/device/material/PhysicallyBasedMDL.h b/devices/rtx/device/material/PhysicallyBasedMDL.h index 0d6612800..03d16b1e9 100644 --- a/devices/rtx/device/material/PhysicallyBasedMDL.h +++ b/devices/rtx/device/material/PhysicallyBasedMDL.h @@ -48,9 +48,10 @@ struct PhysicallyBasedMDL : public MDL private: void translateAndRemoveParameter(std::string_view paramName); - // Captured from the post-translate `emissive.value`/`emissive.texture` keys - // (the pre-translate `emissive` key is consumed by the translation at first - // commit, so it cannot be re-read on later commits; no MDL introspection). + // Captured by mirroring the parameter translation (see commitParameters): + // a freshly committed pre-translate `emissive` key determines the binding; + // the post-translate `emissive.value`/`emissive.texture` keys cover later + // commits that don't re-set it (no MDL introspection). // A nonzero constant is a Geometry Light with an exact Pick Power; a bound // sampler is one with the live sampler-mean as Pick Power, the device // evaluating the compiled EDF at the synthetic next-event hit (ADR 0006). diff --git a/devices/rtx/device/mdl/SamplerRegistry.cpp b/devices/rtx/device/mdl/SamplerRegistry.cpp index 2710abeff..eca580ade 100644 --- a/devices/rtx/device/mdl/SamplerRegistry.cpp +++ b/devices/rtx/device/mdl/SamplerRegistry.cpp @@ -285,6 +285,12 @@ Sampler *SamplerRegistry::loadFromDDS( image2d->setParam("size", U64Vec2(dds->header.width, dds->header.height)); array1d->refDec(helium::PUBLIC); + // Registry-internal objects never pass through anariCommitParameters(), + // so nothing ever captures their committed snapshot — a later buffered + // re-commit (change-observer notification) would run under a + // ReadCommittedScope against an EMPTY snapshot and lose every parameter. + // Mirror staging into the snapshot explicitly. + image2d->snapshotParameters(); image2d->commitParameters(); image2d->finalize(); tex = image2d; @@ -317,6 +323,7 @@ Sampler *SamplerRegistry::loadFromDDS( image2d->setParam("image", array2d); array2d->refDec(helium::PUBLIC); + image2d->snapshotParameters(); // see the compressed-path comment above image2d->commitParameters(); image2d->finalize(); tex = image2d; @@ -397,6 +404,7 @@ Sampler *SamplerRegistry::loadFromImage( // copy just doubles this texture's VRAM footprint. auto image2d = new Image2D(m_deviceState); image2d->setParam("image", array2d); + image2d->snapshotParameters(); // see the compressed-path comment above image2d->commitParameters(); image2d->finalize(); array2d->refDec(helium::PUBLIC); @@ -477,6 +485,7 @@ Sampler *SamplerRegistry::loadFromTextureDesc( array3d->uploadArrayData(); auto image3d = new Image3D(m_deviceState); image3d->setParam("image", array3d); + image3d->snapshotParameters(); // see the compressed-path comment above image3d->commitParameters(); image3d->finalize(); array3d->refDec(helium::PUBLIC); diff --git a/devices/rtx/device/sampler/CompressedImage2D.cpp b/devices/rtx/device/sampler/CompressedImage2D.cpp index 6a4b2e3d5..a603bfe2c 100644 --- a/devices/rtx/device/sampler/CompressedImage2D.cpp +++ b/devices/rtx/device/sampler/CompressedImage2D.cpp @@ -76,7 +76,6 @@ void CompressedImage2D::finalize() } const ANARIDataType format = m_image->elementType(); - m_cuArray = {}; if (format != ANARI_UINT8 && format != ANARI_INT8) { reportMessage(ANARI_SEVERITY_WARNING, "invalid texture type encountered in CompressedImage2D sampler (%s). Must be ANARI_UINT8 or ANARI_INT8.", @@ -89,6 +88,10 @@ void CompressedImage2D::finalize() m_imageLastUpdated = imageDataModified; cleanup(); + // Zero the handle only when actually rebuilding: an equal-stamp + // re-finalize (buffered re-commit) must keep the live array — zeroing it + // here used to leak it (cleanup() would later free a null handle). + m_cuArray = {}; cudaChannelFormatKind channelFormatKind; diff --git a/devices/rtx/device/world/World.cpp b/devices/rtx/device/world/World.cpp index 5417218eb..2a4030eac 100644 --- a/devices/rtx/device/world/World.cpp +++ b/devices/rtx/device/world/World.cpp @@ -92,6 +92,10 @@ World::World(DeviceGlobalState *d) m_zeroInstance = new Instance(d); m_zeroInstance->setParamDirect("group", m_zeroGroup.ptr); + // Internal objects never pass through anariCommitParameters(): mirror + // staging into the committed snapshot so a later buffered re-commit + // (running under a ReadCommittedScope) doesn't read an empty one. + m_zeroInstance->snapshotParameters(); m_zeroInstance->commitParameters(); m_zeroInstance->finalize(); @@ -176,7 +180,14 @@ void World::finalize() m_zeroGroup->removeParam("light"); m_zeroInstance->setParam("id", getParam("id", ~0u)); + m_zeroInstance->snapshotParameters(); // internal object, see World() + // Re-commit explicitly: the zero instance observes nothing, so no buffered + // re-commit would ever consume the staged id (it stayed at the ctor default + // before this). + m_zeroInstance->commitParameters(); + m_zeroInstance->finalize(); + m_zeroGroup->snapshotParameters(); // internal object, see World() m_zeroGroup->commitParameters(); m_zeroGroup->finalize();