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

FFI Rework #558

Merged
merged 134 commits into from
Mar 5, 2020
Merged

FFI Rework #558

merged 134 commits into from
Mar 5, 2020

Conversation

jmercouris
Copy link
Member

This is the FFI branch that I've been working on.

I've changed quite a few concepts. It works on macOS too, which is great.

Tasks Left (in-exhaustive list):

  • Use buffer, interface class etc
  • Finish IPC methods, RPC -> IPC
  • Work on C -> C++ Glue code @chrisboeg

I would be interested in some preliminary feedback as I am working on this first draft. Ideally I would like to get something working, merge it onto master so that things do not diverge too far, and then we incrementally improve upon it. Interested in your early thoughts and questions @Ambrevar.

The first draft will only feature a GTK port that is cross-platform. It is tested on SBCL, with some functionality tested on CCL.

Woe is me, O Hector; woe, indeed, that to share a common lot we were
born, you at Troy in the house of Priam, and I at Thebes under the
wooded mountain of Plakos in the house of Eetion who brought me up
when I was a child – ill-starred sire of an ill-starred daughter –
would that he had never begotten me. You are now going into the house
of Hades under the secret places of the earth, and you leave me a
sorrowing widow in your house. The child, of whom you and I are the
unhappy parents, is as yet a mere infant. Now that you are gone, O
Hector, you can do nothing for him nor he for you.
@jmercouris
Copy link
Member Author

Shall I now merge while we continue to work on this?

@Ambrevar
Copy link
Member

Ambrevar commented Mar 4, 2020 via email

@Ambrevar
Copy link
Member

Ambrevar commented Mar 4, 2020 via email

@jmercouris
Copy link
Member Author

What if we had a prompt with a y-n question that one could toggle via their init file ? > Sounds like an implementation detail we would ideally hide from the user. ENTRY-POINT is only every run by the binary, so in my opinion my proposal fits both use cases without exposing anything to the user. I'm not sure if that will work, that's all (and it may be a lot of work to get it to work, when I want to move on and complete the Qt port as soon as possible).
I think you misunderstood, what I'm suggesting is super simple. I'll send a patch later.

OK :-)

@jmercouris
Copy link
Member Author

Shall I now merge while we continue to work on this?
Considering it breaks on Linux, I'd say no, let's wait until it's fixed. Your hunch about the within-gtk-loop is good, I'll have a look today. Fingers crossed and maybe we will merge on master today / this week :)

OK, I can also take a look on my VM and see if I can reproduce some of the behavior.

@jmercouris
Copy link
Member Author

I can reproduce the crashing in my VM. I will work on the C++ API for now, and get back to this if you have not already handled it.

I find myself frequently looking at the title instead of the URL
@Ambrevar
Copy link
Member

Ambrevar commented Mar 4, 2020 via email

@jmercouris
Copy link
Member Author

I’m using 3.24.12 on my machine, I don’t know how to check the VM

@Ambrevar
Copy link
Member

Ambrevar commented Mar 4, 2020 via email

@Ambrevar
Copy link
Member

Ambrevar commented Mar 4, 2020 via email

@jmercouris
Copy link
Member Author

Great!!! That’s fantastic!

It is Ubuntu, anyways if within-main-loop solves it, then I don’t need to look at it, please make it conditional so that it is only used on Linux, as that line actively makes it malfunction on macOS

We've got to use within-main-loop or else Next hangs after 10 minutes on
GNU/Linux.
@Ambrevar
Copy link
Member

Ambrevar commented Mar 4, 2020 via email

@jmercouris
Copy link
Member Author

I've just sent a patch on ambrevar/ffi-2. I haven't put a conditional yet because I'd like you to give it a try, see if you really get a problem in this case. If so, could you report the issue you are getting to cffi-gtk? If move the code around a little bit so that it should be trivial to put a conditional. Let me know! :)

I just tried it, and it did not work. This problem took me like two weeks to solve already :-D. It involves not being able to draw when not on the main thread of macOS, which is why I cannot join the main GTK loop. It has to do with an implementation detail of WebKitGTK+.

Whenever you get a chance, please put the conditional in place and I will merge onto this branch, and then onto master!

@jmercouris
Copy link
Member Author

I added a conditional, I will test on Linux, and if it works, merge onto master.

@jmercouris
Copy link
Member Author

Works for me! Just waiting for your confirmation.

@jmercouris
Copy link
Member Author

As per report to cl-cffi-gtk, I already made a bug report some time ago: crategus/cl-cffi-gtk#72

@jmercouris
Copy link
Member Author

Well, since you already fixed it, and the conditional works on my machine, merge time! :-D

by all means, this is not finished, we'll keep working on it on master, I just don't want to keep this branch open for so long

@jmercouris
Copy link
Member Author

Any objections? :-D

@Ambrevar
Copy link
Member

Ambrevar commented Mar 5, 2020 via email

@Ambrevar
Copy link
Member

Ambrevar commented Mar 5, 2020 via email

@jmercouris jmercouris merged commit 227fc6b into master Mar 5, 2020
@jmercouris
Copy link
Member Author

Yay!!! :-)

@jmercouris jmercouris deleted the ffi branch March 12, 2020 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants