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

Keyboard hook not unregistered because it is called on every patch change #427

Closed
sagantech opened this issue Jan 31, 2019 · 1 comment · Fixed by #501
Closed

Keyboard hook not unregistered because it is called on every patch change #427

sagantech opened this issue Jan 31, 2019 · 1 comment · Fixed by #501

Comments

@sagantech
Copy link
Contributor

Describe the bug
Leaking hooks is not a good idea

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'Surge'
  2. Click on preset and select one
  3. Repeat
  4. Keyboard hooks are leaked, one for each patch change

Expected behavior
No Leaks. (BTW There are many memory leaks as well). Depending on the implementation, no crashing later when hook is called.

Additional context
In SurgeGUIEditor::openOrRecreateEditor This function is called:

frame->registerKeyboardHook(this);

Simple fix is to move to SurgeGUIEditor::open and not call this function from SurgeGUIEditor::openOrRecreateEditor.

baconpaul added a commit to baconpaul/surge that referenced this issue Feb 4, 2019
The onKeyDown handler was reading the wrong part of the data
structure, so keypresses didn't navigate patches. Moreover, the
handler was over-registered. Finally, once those problems were
fixes, make + and - zoom and unzoom the UI.

Closes surge-synthesizer#496 Keyboard Hook doesn't work
Closes surge-synthesizer#479 +/- for zoom
Closes surge-synthesizer#427 Keyboard hook over-registered
baconpaul added a commit to baconpaul/surge that referenced this issue Feb 4, 2019
The onKeyDown handler was reading the wrong part of the data
structure, so keypresses didn't navigate patches. Moreover, the
handler was over-registered. Finally, once those problems were
fixes, make + and - zoom and unzoom the UI.

Closes surge-synthesizer#496 Keyboard Hook doesn't work
Closes surge-synthesizer#479 +/- for zoom
Closes surge-synthesizer#427 Keyboard hook over-registered

Due to an error in windows VSTGUI this doesn't fully work on
windows, with the keyboard zoom not being supported there.
See issue surge-synthesizer#500 for a description of why.
@baconpaul
Copy link
Collaborator

Thanks

baconpaul added a commit to baconpaul/surge that referenced this issue Jul 10, 2019
The onKeyDown handler was reading the wrong part of the data
structure, so keypresses didn't navigate patches. Moreover, the
handler was over-registered. Finally, once those problems were
fixes, make + and - zoom and unzoom the UI.

Closes surge-synthesizer#496 Keyboard Hook doesn't work
Closes surge-synthesizer#479 +/- for zoom
Closes surge-synthesizer#427 Keyboard hook over-registered

Due to an error in windows VSTGUI this doesn't fully work on
windows, with the keyboard zoom not being supported there.
See issue surge-synthesizer#500 for a description of why.


Former-commit-id: e0ff13f2588bbfec2ece60f9217e17522756d0fa [formerly 5aef139]
Former-commit-id: 9e8d3af55f3c457372964310ba83c19e29da1224
Former-commit-id: f8a51113f2f5e6d738efa060371ab5fbd4cf095a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants