KomputeProject / kompute

General purpose GPU compute framework built on Vulkan to support 1000s of cross vendor graphics cards (AMD, Qualcomm, NVIDIA & friends). Blazing fast, mobile-enabled, asynchronous and optimized for advanced GPU data processing usecases. Backed by the Linux Foundation.
http://kompute.cc/
Apache License 2.0
1.94k stars 146 forks source link

can not use kp::Algorithm::setPushConstants(const std::vector<T>& pushConstants) in v0.8.1 #297

Closed cracy3m closed 7 months ago

cracy3m commented 2 years ago

kp::Algorithm::setPushConstants(const std::vector& pushConstants) can not compile success.

compiler context : windows 10 x64, VS2017 x64

    template<typename T>
    void setPushConstants(const std::vector<T>& pushConstants)  // specify to const vector, and `data()` return const T *
    {
        uint32_t memorySize = sizeof(decltype(pushConstants.back()));
        uint32_t size = pushConstants.size();

        this->setPushConstants(pushConstants.data(), size, memorySize); // here invoke non constant version function
    }

// only provide non constant version 

 /**
     * Sets the push constants to the new value provided to use in the next
     * bindPush() with the raw memory block location and memory size to be used.
     *
     * @param data The raw data point to copy the data from, without modifying
     * the pointer.
     * @param size The number of data elements provided in the data
     * @param memorySize The memory size of each of the data elements in bytes.
     */
    void setPushConstants(void* data, uint32_t size, uint32_t memorySize) // <- void * data, so can not compile 
    {

        uint32_t totalSize = memorySize * size;
        uint32_t previousTotalSize =
          this->mPushConstantsDataTypeMemorySize * this->mPushConstantsSize;

        if (totalSize != previousTotalSize) {
            throw std::runtime_error(fmt::format(
              "Kompute Algorithm push "
              "constant total memory size provided is {} but expected {} bytes",
              totalSize,
              previousTotalSize));
        }
        if (this->mPushConstantsData) {
            free(this->mPushConstantsData);
        }

        this->mPushConstantsData = malloc(totalSize);
        memcpy(this->mPushConstantsData, data, totalSize);
        this->mPushConstantsDataTypeMemorySize = memorySize;
        this->mPushConstantsSize = size;
    }
axsaucedo commented 2 years ago

Thank you for creating the issue @cracy3m - could you share the logs of the compile error? It may actually be due to the version of the headers, but we can have more visibility depending on the logs

cracy3m commented 2 years ago

Thank you for creating the issue @cracy3m - could you share the logs of the compile error? It may actually be due to the version of the headers, but we can have more visibility depending on the logs

logs

.\kompute\src\include\kompute\Algorithm.hpp:199: error: C2664: “void kp::Algorithm::setPushConstants(void *,uint32_t,uint32_t)”: can not convert arg 1 from “const _Ty *” to “void *”
        with
        [
            _Ty=float
        ]
.\kompute\src\include\kompute/Algorithm.hpp(199): note:  Conversion missing qualifier
cracy3m commented 2 years ago

@axsaucedo Is it possible that member var of class in Komputelib change private to protect (for subclass access )? I try to use Kompute src to do some gpgpu calculations without modify the lib source; I think some functions can be easily implemented only by subclassing, such as uniform buffer. If I can't access the member variables(kompute lib class) from subclass , I think I must only create Vulkan commandbuffer、pipeline... by myself

axsaucedo commented 7 months ago

Is this still an issue on the latest version?

cracy3m commented 7 months ago

Sorry, I don't have time to try the latest version recently. Maybe it would be better to close this issue.

axsaucedo commented 7 months ago

Ok - closing issue until reported on latest version