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

Release Blockers: Platform-Specific Issues #183

Open
5 tasks
sunjay opened this issue Jun 11, 2020 · 2 comments
Open
5 tasks

Release Blockers: Platform-Specific Issues #183

sunjay opened this issue Jun 11, 2020 · 2 comments

Comments

@sunjay
Copy link
Owner

sunjay commented Jun 11, 2020

The major rewrite in #173 was a success, but unfortunately, I can't make a new release yet because of some platform-specific bugs in some of the crates we now depend on. Open source only keeps working because people contribute to it, so if you can help out with any of these, please do!

If there is already an issue open in the crate repository, I've linked it below. I haven't been able to open issues for everything because unfortunately I don't always have the operating system needed to find a minimal reproduction. If you do have an operating system listed below, you can help by opening an issue in the repository of the dependency and giving them a minimal example of the bug. If you want to help even further, try to fix the bug and send a PR to them!

Workaround

Until these issues are fixed, you may want to use a previous version of the crate. The one released on crates.io might not work on your system since its dependencies are quite out of date. Changing your Cargo.toml to use the following may work as a workaround for the timebeing:

# Use this particular commit of the turtle crate
turtle = {git = "https://github.com/sunjay/turtle", rev = "bf64b8333a2be1914378d8a22a4788947711182b"}

bf64b83 is the commit just before #173.

Known Issues

MacOS

Windows

There may be other issues that I haven't found yet or that I forgot to write down. If you see any, please let me know in the comments or by opening an issue!

Once all of these are fixed and the various platforms are tested one more time, I will release a new version of the crate.

Potential Unblockers

Replace Dependencies

One of the benefits of the refactor is that I've designed the codebase so we can swap out some of our libraries with much less effort than before. This includes the libraries listed above that are currently causing issues. If we get tired of waiting for bugs to get fixed, or if it looks like the bugs won't ever get fixed, this is a great option to get us to a release.

If we can replace pathfinder with something that works cross-platform, I would be up for doing that. While I believe that pathfinder is likely going to be the path forward, I am fine with replacing it in the near term as long as the replacement isn't significantly more complex to integrate into turtle. Almost all of the code that uses pathfinder is in a single file, so we should be able to swap something else in without any big refactors.

We can also potentially replace ipc-channel on MacOS only. This would involve updating ipc_protocol.rs to use whatever the replacement is. Again, I am open to doing this as long as the replacement isn't significantly more complex to integrate into turtle.

Kill the process

Some of the problems above (but not all of them?) may be solved by killing the process when the window is closed. This was one of the potentially breaking changes introduced in #173. Before #173, we always used to kill the process, so any clean up that needed to happen was always forced. Now, we only panic if the user continues to use a turtle after the window has been closed. It might be worth seeing if we can switch back to the old behaviour and then reintroduce it later when the issues above have been fixed.

The panic to be replaced with process::exit(1) is below:

pub async fn recv(&self) -> ServerResponse {
let mut receiver = self.receiver.lock().await;
receiver.recv().await
// Since this struct keeps a ref-counted copy of the senders, they can't have possibly
// been dropped at this point.
.expect("bug: client senders should not be dropped yet")
// This panic causes the program to exit if turtle commands continue after the window
// closes
.unwrap_or_else(|err| panic!("IPC response not received: {}", err))
}

@sunjay
Copy link
Owner Author

sunjay commented Jun 28, 2020

If it looks like pathfinder won't be fixed anytime soon, we can potentially replace glutin + pathfinder with druid + piet and see if that resolves things. This would probably involve figuring out how to implement a custom widget in druid and restructuring some things. That work might be worth it though given that we eventually wanted to move to a proper GUI framework anyway.

@sunjay
Copy link
Owner Author

sunjay commented Nov 16, 2020

In addition to druid, it may be worth exploring tiny-skia as a replacement for pathfinder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant