-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a void * user cookie to SoOutputReallocCB #498
Comments
This would break the ABI and needs a major version bump to 5. What's the use case for this change, as SoOutputReallocCB is only called in SoOutput_Writer.cpp to reallocate memory? |
Actually, my initial statement was not correct: Even if the default parameter for the My pain point with the current API is that it enforces me to make a copy of the complete scene in order to serialize it into a custom container. I've followed the example code given in SoOutput.cpp for #include <gsl/pointers>
#include <boost/intrusive_ptr.hpp>
#include <algorithm>
#include <cstddef>
#include <cstdlib>
#include <vector>
static std::size_t bufferSize = 0;
static std::byte * buffer;
std::vector<std::byte> serializeGraphs(gsl::not_null<boost::intrusive_ptr<SoSeparator>> root)
{
bufferSize = 1024;
buffer = static_cast<std::byte *>(std::malloc(bufferSize));
auto reallocCallback = [](void * bufptr, std::size_t newSize) -> void *
{
buffer = static_cast<std::byte *>(std::realloc(bufptr, newSize));
bufferSize = newSize;
return buffer;
};
SoOutput output;
output.setBuffer(buffer, bufferSize, reallocCallback);
SoWriteAction wa(&output);
wa.apply(root.get().get());
std::vector<std::byte> serializedGraphs(bufferSize);
std::copy_n(buffer, bufferSize, std::begin(serializedGraphs));
std::free(buffer);
return serializedGraphs;
}
I have to copy the raw buffer into the vector. The example code in SoOutput.cpp also creates a copy when constructing a SbString out of the buffer: SbString s(buffer);
free(buffer);
return s; Using an additional void * parameter would allow me to stream directly into the vector without any overhead: #include <gsl/pointers>
#include <boost/intrusive_ptr.hpp>
#include <cstddef>
#include <vector>
std::vector<std::byte> serializeGraphs(gsl::not_null<boost::intrusive_ptr<SoSeparator>> root)
{
const std::size_t initialeSize = 1024;
std::vector<std::byte> serializedGraphs(initialeSize);
auto reallocCallback = [](void * bufptr, std::size_t newSize, void * userData) -> void *
{
auto * castedSerializedGraphs = static_cast<std::vector<std::byte> *>(userData);
castedSerializedGraphs->resize(newSize);
return castedSerializedGraphs->data();
};
SoOutput output;
output.setBuffer(buffer, initialeSize, reallocCallback, &serializedGraphs);
SoWriteAction wa(&output);
wa.apply(root.get().get());
return serializedGraphs;
} I would prefer the second solution as I am currently looking for opportunities to employ custom memory allocators for One possible way to retain API and ABI compatibility would be to implement a new function and deprecate the old one. This would allow users a smooth transition without any breaking change: typedef void * SoOutputReallocCB2(void * ptr, size_t newSize, void * userData);
virtual void setBuffer2(void * bufPointer, size_t initSize,
SoOutputReallocCB2 * reallocFunc, void * userData, int32_t offset = 0) This would result in quite some code duplication and maybe a version bump with API breakage would be a better solution. |
Can you use a |
Some callbacks in the library, such as
SoErrorCB
, already have avoid * data
parameter, but other callbacks are missing this parameter currently.I'd like to propose to add a
void * data
parameter to the reallocation callbackSoOutputReallocCB
defined in SoOutput.h:This parameter could be made NULL by default if API backwards-compatibility is desired.
I could prepare a PR with the necessary changes.
The text was updated successfully, but these errors were encountered: