Skip to content
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.

Sign up for GitHub

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

227: Save (and restore) number of sources and first source ID in presets #229

Merged
merged 8 commits into from
Feb 27, 2025
Merged

227: Save (and restore) number of sources and first source ID in presets #229

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bf277da
wip
vberthiaume Feb 12, 2025
3867481
number of sources now recalled in preset, but somehow position is los…
vberthiaume Feb 13, 2025
b884afd
cleanup [skip-ci]
vberthiaume Feb 13, 2025
e1031ad
first source ID now saved and restored in presets
vberthiaume Feb 13, 2025
039606c
cleanup
vberthiaume Feb 17, 2025
ba93032
prComments
vberthiaume Feb 18, 2025
26a76b1
cleanup
vberthiaume Feb 20, 2025
dff62e3
fix number of source ordering bug
vberthiaume Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Source/cg_ControlGrisAudioProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,6 @@ void ControlGrisAudioProcessor::setNumberOfSources(int const numOfSources, bool
mPositionSourceLinkEnforcer.numberOfSourcesChanged();
mElevationSourceLinkEnforcer.numberOfSourcesChanged();

mPresetManager.numberOfSourcesChanged();

auto const positionSourceLink{ mPositionTrajectoryManager.getSourceLink() };
auto const isSymmetricLink{ positionSourceLink == PositionSourceLink::symmetricX
|| positionSourceLink == PositionSourceLink::symmetricY };
Expand Down
3 changes: 2 additions & 1 deletion Source/cg_ControlGrisAudioProcessor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ class ControlGrisAudioProcessor final
PresetsManager mPresetManager{ mFixPositionData,
mSources,
mPositionSourceLinkEnforcer,
mElevationSourceLinkEnforcer };
mElevationSourceLinkEnforcer,
mFirstSourceId };

PositionTrajectoryManager mPositionTrajectoryManager{ *this, mSources.getPrimarySource() };
ElevationTrajectoryManager mElevationTrajectoryManager{ *this, mSources.getPrimarySource() };
Expand Down
50 changes: 22 additions & 28 deletions Source/cg_ControlGrisAudioProcessorEditor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,33 +364,24 @@ void ControlGrisAudioProcessorEditor::oscStateChangedCallback(bool const state)
//==============================================================================
void ControlGrisAudioProcessorEditor::numberOfSourcesChangedCallback(int const numOfSources)
{
if (mProcessor.getSources().size() != numOfSources || mIsInsideSetPluginState) {
auto const initSourcePlacement{ mProcessor.getSources().size() != numOfSources };
auto const currentPositionSourceLink{ mPositionTrajectoryManager.getSourceLink() };
auto const symmetricLinkAllowed{ numOfSources == 2 };
mSectionTrajectory.setSymmetricLinkComboState(symmetricLinkAllowed);
if (!symmetricLinkAllowed) {
auto const isCurrentPositionSourceLinkSymmetric{ currentPositionSourceLink == PositionSourceLink::symmetricX
|| currentPositionSourceLink
== PositionSourceLink::symmetricY };
if (isCurrentPositionSourceLinkSymmetric) {
mProcessor.setPositionSourceLink(PositionSourceLink::independent,
SourceLinkEnforcer::OriginOfChange::user);
}
}

mSelectedSource = {};
mProcessor.setNumberOfSources(numOfSources);
mSectionGeneralSettings.setNumberOfSources(numOfSources);
mSectionTrajectory.setNumberOfSources(numOfSources);
mSectionSourceSpan.setSelectedSource(&mProcessor.getSources()[mSelectedSource]);
mPositionField.refreshSources();
mElevationField.refreshSources();
mSectionSourcePosition.setNumberOfSources(numOfSources, mProcessor.getFirstSourceId());
if (initSourcePlacement) {
sourcesPlacementChangedCallback(SourcePlacement::leftAlternate);
}
auto const currentPositionSourceLink{ mPositionTrajectoryManager.getSourceLink() };
auto const symmetricLinkAllowed{ numOfSources == 2 };
mSectionTrajectory.setSymmetricLinkComboState(symmetricLinkAllowed);
if (!symmetricLinkAllowed) {
auto const isCurrentPositionSourceLinkSymmetric{ currentPositionSourceLink == PositionSourceLink::symmetricX
|| currentPositionSourceLink == PositionSourceLink::symmetricY };
if (isCurrentPositionSourceLinkSymmetric)
mProcessor.setPositionSourceLink(PositionSourceLink::independent, SourceLinkEnforcer::OriginOfChange::user);
}

mSelectedSource = {};
mProcessor.setNumberOfSources(numOfSources);
mSectionGeneralSettings.setNumberOfSources(numOfSources);
mSectionTrajectory.setNumberOfSources(numOfSources);
mSectionSourceSpan.setSelectedSource(&mProcessor.getSources()[mSelectedSource]);
mPositionField.refreshSources();
mElevationField.refreshSources();
mSectionSourcePosition.setNumberOfSources(numOfSources, mProcessor.getFirstSourceId());
}

//==============================================================================
Expand Down Expand Up @@ -719,6 +710,10 @@ void ControlGrisAudioProcessorEditor::fieldSourcePositionChangedCallback(SourceI
void ControlGrisAudioProcessorEditor::positionPresetChangedCallback(int const presetNumber)
{
mProcessor.getPresetsManager().forceLoad(presetNumber);
numberOfSourcesChangedCallback(mProcessor.getSources().size());

if (auto const presetSourceId {mProcessor.getPresetsManager().getPresetSourceId(presetNumber)})
firstSourceIdChangedCallback(SourceId{*presetSourceId});

auto * parameter{ mAudioProcessorValueTreeState.getParameter(Automation::Ids::POSITION_PRESET) };
auto const newValue{ static_cast<float>(presetNumber) / static_cast<float>(NUMBER_OF_POSITION_PRESETS) };
Expand All @@ -727,9 +722,8 @@ void ControlGrisAudioProcessorEditor::positionPresetChangedCallback(int const pr
parameter->setValueNotifyingHost(newValue);

mProcessor.updatePrimarySourceParameters(Source::ChangeType::position);
if (mProcessor.getSpatMode() == SpatMode::cube) {
if (mProcessor.getSpatMode() == SpatMode::cube)
mProcessor.updatePrimarySourceParameters(Source::ChangeType::elevation);
}
}

//==============================================================================
Expand Down
4 changes: 2 additions & 2 deletions Source/cg_LinkStrategies.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class Base
}; // class Base

//==============================================================================
// only use full to recall saved positions
// only useful to recall saved positions
class PositionIndependent final : public Base
{
void computeParameters_implementation(Sources const &, SourcesSnapshots const &) override {}
Expand Down Expand Up @@ -240,7 +240,7 @@ class PositionDeltaLock final : public Base
};

//==============================================================================
// only usefuLl to recall saved positions
// only useful to recall saved positions
class ElevationIndependent final : public Base
{
void computeParameters_implementation(Sources const &, SourcesSnapshots const &) override {}
Expand Down
97 changes: 56 additions & 41 deletions Source/cg_PresetsManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ juce::String
PresetsManager::PresetsManager(juce::XmlElement & data,
Sources & sources,
SourceLinkEnforcer & positionLinkEnforcer,
SourceLinkEnforcer & elevationLinkEnforcer)
SourceLinkEnforcer & elevationLinkEnforcer,
SourceId & firstSourceId)
: mData(data)
, mSources(sources)
, mPositionLinkEnforcer(positionLinkEnforcer)
, mElevationLinkEnforcer(elevationLinkEnforcer)
, mFirstSourceId(firstSourceId)
{
}

Expand All @@ -64,6 +66,15 @@ int PresetsManager::getCurrentPreset() const
return mLastLoadedPreset;
}

std::optional<int> PresetsManager::getPresetSourceId(int presetNumber)
{
auto const presetData { getPresetData(presetNumber) };
if (!presetData || ! (*presetData)->hasAttribute("firstSourceId"))
return {};

return (*presetData)->getIntAttribute("firstSourceId");
}

//==============================================================================
bool PresetsManager::loadIfPresetChanged(int const presetNumber)
{
Expand Down Expand Up @@ -91,9 +102,16 @@ bool PresetsManager::load(int const presetNumber)

auto const * presetData{ *maybe_presetData };

SourcesSnapshots snapshots{};
if (presetData->hasAttribute ("numberOfSources"))
mSources.setSize(presetData->getIntAttribute("numberOfSources"));

if (presetData->hasAttribute("firstSourceId"))
mFirstSourceId = SourceId{ presetData->getIntAttribute("firstSourceId") };

// Store the preset's source positions in a new SourcesSnapshots
SourcesSnapshots snapshots;
for (auto & source : mSources) {
SourceSnapshot snapshot{};
SourceSnapshot snapshot;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

est-ce qu'on est surs que ça change rien ? ça peut avoir un impact si le type est "plain-old-data".

ex.: https://gcc.godbolt.org/z/nzqbPvvv8

ici on voit que dans le premier cas en haut, la mémoire est renvoyée directement de la fonction; dans le second cas elle se fait zeroed avant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wow ok, bien vu!

auto const index{ source.getIndex() };
auto const xPosId{ getFixedPosSourceName(FixedPositionType::initial, index, 0) };
auto const yPosId{ getFixedPosSourceName(FixedPositionType::initial, index, 1) };
Expand All @@ -107,8 +125,8 @@ bool PresetsManager::load(int const presetNumber)
snapshot.position = position;
auto const zPosId{ getFixedPosSourceName(FixedPositionType::initial, index, 2) };
if (presetData->hasAttribute(zPosId)) {
auto const inversedNormalizedElevation{ static_cast<float>(
presetData->getDoubleAttribute(getFixedPosSourceName(FixedPositionType::initial, index, 2))) };
auto const elevation {getFixedPosSourceName(FixedPositionType::initial, index, 2)};
auto const inversedNormalizedElevation{ static_cast<float>(presetData->getDoubleAttribute(elevation)) };
snapshot.z = MAX_ELEVATION * (1.0f - inversedNormalizedElevation);
}
}
Expand All @@ -119,25 +137,24 @@ bool PresetsManager::load(int const presetNumber)
}
}

//load the snapshots into the enforcers, which are references to the ControlGrisAudioProcessor members
mPositionLinkEnforcer.loadSnapshots(snapshots);
if (mSources.getPrimarySource().getSpatMode() == SpatMode::cube) {
if (mSources.getPrimarySource().getSpatMode() == SpatMode::cube)
mElevationLinkEnforcer.loadSnapshots(snapshots);
}

auto const xTerminalPositionId{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 0) };
auto const yTerminalPositionId{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 1) };
auto const zTerminalPositionId{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 2) };

// position the first source
juce::Point<float> terminalPosition;
if (presetData->hasAttribute(xTerminalPositionId) && presetData->hasAttribute(yTerminalPositionId)) {
juce::Point<float> const inversedNormalizedTerminalPosition{
static_cast<float>(presetData->getDoubleAttribute(xTerminalPositionId)),
static_cast<float>(presetData->getDoubleAttribute(yTerminalPositionId))
};
auto const inversedTerminalPosition{ inversedNormalizedTerminalPosition * 2.0f
- juce::Point<float>{ 1.0f, 1.0f } };
terminalPosition
= juce::Point<float>{ inversedTerminalPosition.getX(), inversedTerminalPosition.getY() * -1.0f };
auto const inversedTerminalPosition{ inversedNormalizedTerminalPosition * 2.0f - juce::Point<float>{ 1.0f, 1.0f } };
terminalPosition = juce::Point<float>{ inversedTerminalPosition.getX(), inversedTerminalPosition.getY() * -1.0f };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

j'ai l'impression qu'il y a pas mal de petits changements de formatting qui inviteraient à faire une MR globale de clang-format, qu'en penses-tu ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(à noter qu'il faut vraiment la faire à un moment ou tout est mergé et il n'y a rien d'autre en cours sinon ça va être l'enfer)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha ouais 💯

} else {
terminalPosition = snapshots.primary.position;
}
Expand All @@ -146,8 +163,7 @@ bool PresetsManager::load(int const presetNumber)
if (mSources.getPrimarySource().getSpatMode() == SpatMode::cube) {
Radians elevation;
if (presetData->hasAttribute(zTerminalPositionId)) {
auto const inversedNormalizedTerminalElevation{ static_cast<float>(
presetData->getDoubleAttribute(zTerminalPositionId)) };
auto const inversedNormalizedTerminalElevation{ static_cast<float>(presetData->getDoubleAttribute(zTerminalPositionId)) };
elevation = MAX_ELEVATION * (1.0f - inversedNormalizedTerminalElevation);
} else {
elevation = snapshots.primary.z;
Expand All @@ -157,6 +173,9 @@ bool PresetsManager::load(int const presetNumber)
}
}
mLastLoadedPreset = presetNumber;

// send a change message, which will end up calling SourceLinkEnforcer::enforceSourceLink()
// to position the secondary sources
sendChangeMessage();

return true;
Expand Down Expand Up @@ -185,17 +204,21 @@ std::optional<juce::XmlElement *> PresetsManager::getPresetData(int const preset
std::unique_ptr<juce::XmlElement> PresetsManager::createPresetData(int const presetNumber) const
{
// Build a new fixed position element.
auto result{ std::make_unique<juce::XmlElement>("ITEM") };
result->setAttribute("ID", presetNumber);
auto preset{ std::make_unique<juce::XmlElement>("ITEM") };
preset->setAttribute("ID", presetNumber);

auto const & positionSnapshots{ mPositionLinkEnforcer.getSnapshots() };
auto const & elevationsSnapshots{ mElevationLinkEnforcer.getSnapshots() };

//save the number and initial position of all sources
SourceIndex const numberOfSources{ mSources.size() };
preset->setAttribute("numberOfSources", numberOfSources.get());
preset->setAttribute("firstSourceId", mFirstSourceId.get());

for (SourceIndex sourceIndex{}; sourceIndex < numberOfSources; ++sourceIndex) {
auto const xName{ getFixedPosSourceName(FixedPositionType::initial, sourceIndex, 0) };
auto const yName{ getFixedPosSourceName(FixedPositionType::initial, sourceIndex, 1) };
auto const zName{ getFixedPosSourceName(FixedPositionType::initial, sourceIndex, 2) };
auto const curSourceX{ getFixedPosSourceName(FixedPositionType::initial, sourceIndex, 0) };
auto const curSourceY{ getFixedPosSourceName(FixedPositionType::initial, sourceIndex, 1) };
auto const curSourceZ{ getFixedPosSourceName(FixedPositionType::initial, sourceIndex, 2) };

auto const position{ positionSnapshots[sourceIndex].position };
auto const elevation{ elevationsSnapshots[sourceIndex].z };
Expand All @@ -206,30 +229,28 @@ std::unique_ptr<juce::XmlElement> PresetsManager::createPresetData(int const pre
auto const mirroredNormalizedPosition{ (mirroredPosition + juce::Point<float>{ 1.0f, 1.0f }) / 2.0f };
auto const inversedNormalizedElevation{ 1.0f - normalizedElevation };

result->setAttribute(xName, mirroredNormalizedPosition.getX());
result->setAttribute(yName, mirroredNormalizedPosition.getY());
result->setAttribute(zName, inversedNormalizedElevation);
preset->setAttribute(curSourceX, mirroredNormalizedPosition.getX());
preset->setAttribute(curSourceY, mirroredNormalizedPosition.getY());
preset->setAttribute(curSourceZ, inversedNormalizedElevation);
}

auto const xName{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 0) };
auto const yName{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 1) };
auto const zName{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 2) };
//TODO VB: is terminal the same as final?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pas sûr qu'on veuille garder ces commentaires dans la version finale ^^'

//save the terminal position of only the first source
auto const firstSourceX{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 0) };
auto const firstSourceY{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 1) };
auto const firstSourceZ{ getFixedPosSourceName(FixedPositionType::terminal, SourceIndex{ 0 }, 2) };

// For some legacy reason, we store a normalized value with inversed Y and elevation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kinda makes sense, a lot of spatialization systems work like this

auto const position{ mSources.getPrimarySource().getPos() };
juce::Point<float> const mirroredPosition{
position.getX(),
position.getY() * -1.0f
}; // For some legacy reason, we store a normalized value with inversed Y.
juce::Point<float> const mirroredPosition{ position.getX(), position.getY() * -1.0f };
auto const inversedNormalizedPosition{ (mirroredPosition + juce::Point<float>{ 1.0f, 1.0f }) / 2.0f };
auto const inversedNormalizedElevation{
1.0f - mSources.getPrimarySource().getNormalizedElevation().get()
}; // Same this happens with elevation.
auto const inversedNormalizedElevation{ 1.0f - mSources.getPrimarySource().getNormalizedElevation().get() };

result->setAttribute(xName, inversedNormalizedPosition.getX());
result->setAttribute(yName, inversedNormalizedPosition.getY());
result->setAttribute(zName, inversedNormalizedElevation);
preset->setAttribute(firstSourceX, inversedNormalizedPosition.getX());
preset->setAttribute(firstSourceY, inversedNormalizedPosition.getY());
preset->setAttribute(firstSourceZ, inversedNormalizedElevation);

return result;
return preset;
}

//==============================================================================
Expand Down Expand Up @@ -265,12 +286,6 @@ bool PresetsManager::deletePreset(int const presetNumber) const
return true;
}

//==============================================================================
void PresetsManager::numberOfSourcesChanged()
{
// TODO
}

//==============================================================================
std::array<bool, NUMBER_OF_POSITION_PRESETS> PresetsManager::getSavedPresets() const
{
Expand Down
6 changes: 4 additions & 2 deletions Source/cg_PresetsManager.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class PresetsManager final : public juce::ChangeBroadcaster
Sources & mSources;
SourceLinkEnforcer & mPositionLinkEnforcer;
SourceLinkEnforcer & mElevationLinkEnforcer;
SourceId & mFirstSourceId;

public:
//==============================================================================
Expand All @@ -57,16 +58,17 @@ class PresetsManager final : public juce::ChangeBroadcaster
PresetsManager(juce::XmlElement & data,
Sources & sources,
SourceLinkEnforcer & positionLinkEnforcer,
SourceLinkEnforcer & elevationLinkEnforcer);
SourceLinkEnforcer & elevationLinkEnforcer,
SourceId & firstSourceId);
//==============================================================================
[[nodiscard]] int getCurrentPreset() const;
[[nodiscard]] std::array<bool, NUMBER_OF_POSITION_PRESETS> getSavedPresets() const;

std::optional<int> getPresetSourceId(int presetNumber);
bool loadIfPresetChanged(int presetNumber);
bool forceLoad(int presetNumber);
void save(int presetNumber) const;
[[nodiscard]] bool deletePreset(int presetNumber) const;
void numberOfSourcesChanged();

private:
//==============================================================================
Expand Down
2 changes: 1 addition & 1 deletion Source/cg_SectionPositionPresets.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ void PresetButton::internalClickCallback(juce::ModifierKeys const & mods)
setToggleState(false, juce::NotificationType::dontSendNotification);
mLoaded = mSaved = false;
} else if (mSaved) {
TextButton::internalClickCallback(mods);
TextButton::internalClickCallback(mods); //this will cause PresetButton::clicked() to be called right above
}
refresh();
}
Expand Down
9 changes: 9 additions & 0 deletions Source/cg_SourceSnapshot.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ struct SourcesSnapshots {
SourceSnapshot & operator[](SourceIndex const index);
int size() const { return secondaries.size() + 1; }

juce::String toString ()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this method can be marked const

{
juce::String ret;
ret += primary.position.toString() + ", " + primary.z.toString() + "; ";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hopefully one day juce supports std::format :D with it it would be:

ret += std::format("{}, {}; ", primary.position, primary.z);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right... another thing is we'd need to change the c++ version in the projucer. I'll make a note of it

for (auto const & snapshot : secondaries)
ret += snapshot.position.toString() + ", " + snapshot.z.toString() + "; ";
return ret;
}

private:
JUCE_LEAK_DETECTOR(SourceSnapshot)
}; // class SourcesSnapshots
Expand Down