Repository: google/filament
File: filament/src/ds/DescriptorSet.cpp (C++)

Code before:
    }
    mDescriptorSetHandle = driver.createDescriptorSet(layout.getHandle(), mName);
    mValid.forEachSetBit([&layout, &driver,
            dsh = mDescriptorSetHandle, descriptors = mDescriptors.data()]
            (backend::descriptor_binding_t const binding) {
        assert_invariant(layout.isValid(binding));
        if (layout.isSampler(binding)) {
            driver.updateDescriptorSetTexture(dsh, binding,
                    descriptors[binding].texture.th,
                    descriptors[binding].texture.params);
        } else {
            driver.updateDescriptorSetBuffer(dsh, binding,
                    descriptors[binding].buffer.boh,
                    descriptors[binding].buffer.offset,
                    descriptors[binding].buffer.size);
        }
    });

    // FIXME: see [b/468072646]
    //  We only validate empty descriptors at FEATURE_LEVEL_1 and above.
    //  This is because at FL0 it's expected that some descriptor won't be set. In theory, the
    //  corresponding layouts should not even contain those descriptors. However, making that change
    //  is difficult and risky, and will be done at a later time.
    //  Note: that the correct fix is actually needed to properly support FL3 once we want
    //  to take advantage of having more samplers.
    if (UTILS_UNLIKELY(driver.getFeatureLevel() > backend::FeatureLevel::FEATURE_LEVEL_0)) {
        auto const unsetValidDescriptors = layout.getValidDescriptors() & ~mValid;
        if (UTILS_VERY_UNLIKELY(!unsetValidDescriptors.empty() && !mSetUndefinedParameterWarning)) {
            unsetValidDescriptors.forEachSetBit([&](auto i) {
                LOG(WARNING) << (layout.isSampler(i) ? "Sampler" : "Buffer") << " descriptor " << i
                        << " of " << mName.c_str() << " is not set. Please report this issue.";
            });
            mSetUndefinedParameterWarning = true;
        }
    }
}

void DescriptorSet::bind(FEngine::DriverApi& driver, DescriptorSetBindingPoints const set) const noexcept {
    bind(driver, set, {});
}

void DescriptorSet::bind(FEngine::DriverApi& driver, DescriptorSetBindingPoints const set,
        backend::DescriptorSetOffsetArray dynamicOffsets) const noexcept {
    // TODO: on debug check that dynamicOffsets is large enough

    assert_invariant(mDescriptorSetHandle);

    // TODO: Make sure clients do the right thing and not change material instance parameters
    // within the renderpass. We have to comment the assert out since it crashed a client's debug
    // build.
    // assert_invariant(mDirty.none());

Review comment (suggestion):
```suggestion
    if (UTILS_LIKELY(driver.getFeatureLevel() > backend::FeatureLevel::FEATURE_LEVEL_0)) {
```

Code after:
    }
    mDescriptorSetHandle = driver.createDescriptorSet(layout.getHandle(), mName);
    mValid.forEachSetBit([&layout, &driver,
            dsh = mDescriptorSetHandle, descriptors = mDescriptors.data()]
            (backend::descriptor_binding_t const binding) {
        assert_invariant(layout.isValid(binding));
        if (layout.isSampler(binding)) {
            driver.updateDescriptorSetTexture(dsh, binding,
                    descriptors[binding].texture.th,
                    descriptors[binding].texture.params);
        } else {
            driver.updateDescriptorSetBuffer(dsh, binding,
                    descriptors[binding].buffer.boh,
                    descriptors[binding].buffer.offset,
                    descriptors[binding].buffer.size);
        }
    });

    // FIXME: see [b/468072646]
    //  We only validate empty descriptors at FEATURE_LEVEL_1 and above.
    //  This is because at FL0 it's expected that some descriptor won't be set. In theory, the
    //  corresponding layouts should not even contain those descriptors. However, making that change
    //  is difficult and risky, and will be done at a later time.
    //  Note: that the correct fix is actually needed to properly support FL3 once we want
    //  to take advantage of having more samplers.
    if (UTILS_LIKELY(driver.getFeatureLevel() > backend::FeatureLevel::FEATURE_LEVEL_0)) {
        auto const unsetValidDescriptors = layout.getValidDescriptors() & ~mValid;
        if (UTILS_VERY_UNLIKELY(!unsetValidDescriptors.empty() && !mSetUndefinedParameterWarning)) {
            unsetValidDescriptors.forEachSetBit([&](auto i) {
                LOG(WARNING) << (layout.isSampler(i) ? "Sampler" : "Buffer") << " descriptor " << i
                        << " of " << mName.c_str() << " is not set. Please report this issue.";
            });
            mSetUndefinedParameterWarning = true;
        }
    }
}

void DescriptorSet::bind(FEngine::DriverApi& driver, DescriptorSetBindingPoints const set) const noexcept {
    bind(driver, set, {});
}

void DescriptorSet::bind(FEngine::DriverApi& driver, DescriptorSetBindingPoints const set,
        backend::DescriptorSetOffsetArray dynamicOffsets) const noexcept {
    // TODO: on debug check that dynamicOffsets is large enough

    assert_invariant(mDescriptorSetHandle);

    // TODO: Make sure clients do the right thing and not change material instance parameters
    // within the renderpass. We have to comment the assert out since it crashed a client's debug
    // build.
    // assert_invariant(mDirty.none());