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

modernization update #4

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

danomatika
Copy link
Contributor

@danomatika danomatika commented Dec 5, 2019

This PR a general modernization update for building the app for macOS 10.10+ as well as 64 bit builds for 10.15+.

In also integrates existing PRs #1 and #2 as well as a commit from one of the forks. I also took the liberty of updating the (meta) documentation to markdown.

I find this application extremely useful and would very much like to see an official release of this proposed 1.0.4 version as a 64 bit build. It's obvious the design is solid and the CoreMIDI API is stable as MIDI Patchbay has worked just fine over the last 10 years. Hopefully this afternoon of work will ensure that it does for the next 10. :)

EDIT: 64 bit version 1.04 test builds are here: http://docs.danomatika.com/releases/midipatchbay/

NOTE: For attribution, I should probably listed as "Dan Wilcox (ZKM | Hertz-lab)" to include the fact this work was done within my normal job.

This was referenced Dec 5, 2019
@notahat
Copy link
Owner

notahat commented Dec 5, 2019

@danomatika You're a champion! I'll try to have a look this weekend and get an official release out. I'll add you to the credits and give you a mention on the website too.

@danomatika
Copy link
Contributor Author

danomatika commented Dec 6, 2019

Thanks. It's mainly a community effort I pulled together: see the attribution in the proposed 1.0.4 version at the end of the readme.

There are still two points which are in question:

  1. I noticed the UI in the repo is slightly different from the last release (1.0.3). I find the release version to be a better layout, but I wasn't sure enough about the design decisions involved to change it directly.
  • placement of the Input & Outputs
  • placement of Add Patch button and additional Remove Patch button
  • 1.0.3 opens with Notes tab selected by default instead of Channels tab

In the repo:
Screen Shot 2019-12-06 at 11 19 08 AM

Version 1.0.3:
Screen Shot 2019-12-06 at 11 19 22 AM

  1. There is still a deprecation warning for the custom TableView Enter key handling which was non-trivial to change.

I will make proposed implications for both of these issues in feature branches and post back.

@danomatika
Copy link
Contributor Author

Oh, and ARC modernization is needed as well.

@danomatika danomatika mentioned this pull request Dec 6, 2019
@danomatika
Copy link
Contributor Author

danomatika commented Dec 6, 2019

Recent work:

  • UI updates are in a separate branch: UI updates #5.
  • The custom table view enter key handling does not appear to be needed any longer as the selection remains after an edit is finished. It is commented out for now which solves the deprecation warning.
  • ARC transition is done: mainly removing retains/releases and setting __bridge casts.
  • I noticed that endpoint switching was not saved after leaving the endpoint panel and the logic was commented out. This has been re-enabled with a check to make sure selecting the same endpoint does not add an undo action.
  • There are now two build schemes: Debug and Release

EDIT: That should it for now. I think everything is covered, unless bugs or more inconsistencies with the 1.0.3 release version are found.

@danomatika
Copy link
Contributor Author

Also, there is a new test build with the UI updates.

@notahat
Copy link
Owner

notahat commented Dec 7, 2019

Oh, and ARC modernization is needed as well.

Yeah, I'm going through the changes, and I'm suspicious of all the memory management stuff. The fun is that CoreMIDI doesn't correctly follow Apple's own memory management rules, so I'm not sure how it's gonna interact with ARC. Gonna have to do some more reading and testing.

@XENONChromatic
Copy link

I am happy to continue testing as needed. The current version (RC2 I guess it'd be called?) seems to run fine, and I didnt see any noticeable performance issues in terms of memory/CPU usage, although I also wasnt looking for such things.

@danomatika
Copy link
Contributor Author

danomatika commented Dec 8, 2019 via email

@danomatika
Copy link
Contributor Author

@notahat I'm happy about #6. This PR is definitely more quick triage as I went quickly without a deep understanding of the code. If it's helpful in the end, great.

@EricGeorge
Copy link

EricGeorge commented Dec 31, 2019

@danomatika - so glad you did this. I too was slowing trying to bring this up to date and super glad to see the activity here. I did some quick testing on the test build and it seems to work well on 10.15.2. Happy to continue testing as needed until merged.

@mmontag
Copy link

mmontag commented Dec 11, 2020

Can this be merged? Or is it now obsolete?

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 this pull request may close these issues.

7 participants