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.

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

setup dx7 when dx oscillator selected #3004

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

m-m-adams
Copy link
Collaborator

Remove the need for the custom 1 plus synth shortcut by just setting up the dx7 engine if an oscillator is set to dx7

I think we should cherry pick this since it's tiny and makes the DX7 a lot more usable

@m-m-adams m-m-adams added the cherry-pick Commit to cherry pick to release branch label Dec 2, 2024
Copy link
Contributor

github-actions bot commented Dec 2, 2024

Test Results

106 tests  ±0   106 ✅ ±0   0s ⏱️ ±0s
 16 suites ±0     0 💤 ±0 
 16 files   ±0     0 ❌ ±0 

Results for commit a1ef4f2. ± Comparison against base commit 5733f49.

@seangoodvibes seangoodvibes added this pull request to the merge queue Dec 3, 2024
Merged via the queue into community with commit aa143d9 Dec 3, 2024
6 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 3, 2024
@bfredl
Copy link
Collaborator

bfredl commented Dec 3, 2024

While I agree with the UI change, this does unfortunately not include the steps to actually setup the synth for dx7:

  • the ENV1 release should be set to 50 to let the DX7:s internal envelopes release phase play out (yes, we still do voice culling properly when the dx7 envelopes end)
  • The VEL-> master volume patch cable should not be created (or removed after the fact) as dx7 handles velocity per operator and this additional global patch cable will throw off loaded patches which are properly velocity calibrated internally.

I think we could do this before ensureDX7Patch() when dxPatch == nullptr (dx7 has not been set up yet) and the synth was not loaded from file (so it is a newly created synth).

Together with #2694 we can then completely remove the "custom 1 plus synth" hack.

bfredl added a commit to bfredl/DelugeFirmware that referenced this pull request Dec 3, 2024
bfredl added a commit to bfredl/DelugeFirmware that referenced this pull request Dec 3, 2024
bfredl added a commit to bfredl/DelugeFirmware that referenced this pull request Dec 3, 2024
follow up to SynthstromAudible#3004

- only setup dx7 on source1.
- the ENV1 release should be set to 50 to let the DX7:s internal envelopes release phase play out (yes, we still do voice culling properly when the dx7 envelopes end)
- The VEL-> master volume patch cable should not be used as dx7 handles velocity per operator and this additional global patch cable will throw off loaded patches which are properly velocity calibrated internally.
bfredl added a commit to bfredl/DelugeFirmware that referenced this pull request Dec 3, 2024
follow up to SynthstromAudible#3004

This restriction was intentional as the UI code (like the operator
sidebar) assumes only OSC1 is used as a DX7 oscillator.

We might want to improve the UI later on, but for now keep the
restriction
bfredl added a commit to bfredl/DelugeFirmware that referenced this pull request Dec 3, 2024
follow up to SynthstromAudible#3004

This restriction was intentional as the UI code (like the operator
sidebar) assumes only OSC1 is used as a DX7 oscillator.

We might want to improve the UI later on, but for now keep the
restriction
github-merge-queue bot pushed a commit that referenced this pull request Dec 3, 2024
follow up to #3004

This restriction was intentional as the UI code (like the operator
sidebar) assumes only OSC1 is used as a DX7 oscillator.

We might want to improve the UI later on, but for now keep the
restriction
github-actions bot pushed a commit that referenced this pull request Dec 3, 2024
follow up to #3004

This restriction was intentional as the UI code (like the operator
sidebar) assumes only OSC1 is used as a DX7 oscillator.

We might want to improve the UI later on, but for now keep the
restriction
@stellar-aria stellar-aria deleted the usability/make_dx7_setup_make_sense branch December 10, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick Commit to cherry pick to release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants