Open polygon opened 8 months ago
Hey, thanks for the contribution!
This looks great, I definitely want to merge this. However I don't have a lot of bandwidth at the moment, so it might take a bit of time to review/test everything on my side.
Is the issue with exporting the MIDI fixed, or if you have a fix what is it?
Few notes after a super quick look at the changes:
new
. Let's use modern C++ (std::unique_ptr
...) instead if possible.UIFonts::get().fontx()
. UIFonts
instead of recreating them each time with things like Font(pMONTSERRAT_BOLD).withPointHeight(18.0f)
Also you can open a PR on libonnxruntime-neuralnote
so we can review it as well.
Hey, thanks for the response. I'm also a bit short on time so before doing some of the refactors, I'd like your opinion if that would be fine:
std::unique_ptr
instead of raw pointers should not be an issue, will do thatUIFonts::get().fontx()
it would be necessary to write fontx()
but I don't think getting rid of the braces is possible. Would that be acceptable for you? The other alternative would be another patch to JUCE since you seem to be embedding the fonts anyways, so looking up the system font config is kinda unneeded, but JUCE does it anywaysFont
objects as well, should not be an issue.Regarding the drag and drop issue, JUCE has this piece of code that manages communication between the Drag&Drop processes:
void handleExternalDragAndDropStatus (const XClientMessageEvent& clientMsg)
{
if (expectingStatus)
{
expectingStatus = false;
canDrop = false;
silentRect = {};
const auto& atoms = getAtoms();
if ((clientMsg.data.l[1] & 1) != 0
&& ((Atom) clientMsg.data.l[4] == atoms.XdndActionCopy
|| (Atom) clientMsg.data.l[4] == atoms.XdndActionPrivate))
{
if ((clientMsg.data.l[1] & 2) == 0) // target requests silent rectangle
silentRect.setBounds ((int) clientMsg.data.l[2] >> 16, (int) clientMsg.data.l[2] & 0xffff,
(int) clientMsg.data.l[3] >> 16, (int) clientMsg.data.l[3] & 0xffff);
canDrop = true;
}
}
}
I am not fully understanding of the details of Drag&Drop in Linux/X11 but I found that these if-conditions are no longer satisfied once the dragged data is moved outside of the plugin window. Hence, canDrop
becomes false
and in the following code which handles what happens after the mouse button is released to end the drop:
void handleExternalDragButtonReleaseEvent()
{
if (dragging)
X11Symbols::getInstance()->xUngrabPointer (getDisplay(), CurrentTime);
if (canDrop)
{
sendExternalDragAndDropDrop();
}
else
{
sendExternalDragAndDropLeave();
externalResetDragAndDrop();
}
}
the drop operation would be canceled. This had the funny side effect that while dragging, I could even see the MIDI notes in Bitwig, but once I released the mouse button, they were gone again. My (probably) dirty fix involved replacing if (canDrop)
with if (1)
to always force the drop to complete. This makes dropping files from NeuralNote to file managers and also to Bitwig Studio work without issues. I've not yet encountered any negative side-effects of always completing the drop.
I'll open a Pull Request for libonnxruntime-neuralnote
later today once I am off work.
Hey! This is exciting, I compiled and it works. And more over this drag-and-drop "fix" looks like a key for a long standing JUCE DnD Linux bug:)
I only had to add set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)
do make the linker happy.
@polygon If any linux build is available somewhere, I would be very happy to try it.
@polygon If any linux build is available somewhere, I would be very happy to try it.
same here, I am a Linux user and I am happy to try it and give feedback.
I've made NeuralNote build under Linux and was wondering if there is interest to upstream the changes. This PR would be the first step and contains the required changes to the code and the build system. The changes I was doing are:
fonts.conf
XML file and the XML parser will use certain static strings that are not initialized at that time. I've written more about this here: https://github.com/DamRsn/NeuralNote/issues/33#issuecomment-1783775194/tmp
directory otherwise, usually taking some temp-files of other applications with itThis change also requires an accompanying change to
libonnxruntime-neuralnote
to get a Linux build there which I have also available. But first wanted to start here to gauge interest in something like this.There is another issue with external Drag&Drop not working properly in Linux. This seems to be a JUCE issue and I have a workaround available. But since this requires patching the external JUCE dependency I think that would probably be more suitable to put into the build scripts. Also, it's not strictly required for a Linux build.
Edit: Link to patched
libonnxruntime-neuralnote
: https://github.com/polygon/libonnxruntime-neuralnote/tree/linux