Remove ValueBuffer

[Rossmaxx]

supersedes #7170 which got closed because of a mess up on @Snowiiii's side. I recovered their commits using git fetch upstream pull/7170/head:valuebuffer-removal.

ValueBuffer is just and wrapper around std::vector, but we dont use any Special methods from it so we should use std::vector instead

Copied description from original PR.

[DomClark]

@sakertooth's comment at #7170 (comment) still applies. While passing a pointer to a std::vector<float> doesn't leave us any worse off than passing a pointer to a ValueBuffer, if we actually want to get any advantage from this change we ought to do it properly.

Also, having thought about it some more, I wonder if we can repurpose it in some way to avoid the endless repeating of code like sampleExactBuffer ? sampleExactBuffer[i] : fixedValue.

[Rossmaxx]

@sakertooth's comment at #7170 (comment) still applies. While passing a pointer to a std::vector doesn't leave us any worse off than passing a pointer to a ValueBuffer, if we actually want to get any advantage from this change we ought to do it properly.

I've addressed this one a week ago and am waiting for a follow up review.

Also, having thought about it some more, I wonder if we can repurpose it in some way to avoid the endless repeating of code like sampleExactBuffer ? sampleExactBuffer[i] : fixedValue.

we could but how exactly? I guess we have to write a simple helper function for it.

[sakertooth]

I would add a member function valueAt to AutomatableModel that takes an index as a parameter. It will check if the buffer is not nullptr, if it is not, then it accesses the buffer at the given index. If it is, then it will use the fixed value and return the result of AutomatableModel::value. My review is based on assuming this function was added.

(In AutomatableModel.cpp somewhere):

float AutomatableModel::valueAt(size_t index)
{
     const auto buffer = valueBuffer();
     return buffer ? buffer->data[index] : value();
}

Once you end up replacing calls from value to valueAt, make sure to keep the overall expression the same and just change the value part.

NB: Since AutomatableModel::valueBuffer seems to be a non-trivial function, it might be worthwhile to see if there's any performance impact after applying these changes since it will be called more frequently.

[Rossmaxx]

If what i understand is right, you are asking me to create a helper function to handle the code repetition and modify the code accordingly. But i see one problem in the helper function.

float AutomatableModel::valueAt(size_t index)
{
     const auto buffer = valueBuffer();
     return buffer ? buffer->data[index] : value();
}

We create a new buffer and check for the buffer immediately. Which might return true in all cases, so the fixed value might not get accessed. Also the new buffer might have a value different from the old one. Atleast that's what i understand. Is there anything else going on which I missed?

[Rossmaxx]

@messmerd I did apply assert like you said before but i can't find the thread where you said to apply the assert. Can you cross paste the reference you pasted there?

[Rossmaxx]

I am seriously thinking of removing this function and revert back to the state where everything was working. I did try something now. I'll see anyway.

[sakertooth]

Looking at this now, I realized it would be better to redesign the sample-exactness system rather than just remove ValueBuffer, thus replacing complex uses of ValueBuffer directly with something easier to work with. I'm planning to do some work towards this.