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

Add iOS External Screen support #484

Closed
wants to merge 2 commits into from

Conversation

conath
Copy link
Contributor

@conath conath commented Oct 9, 2020

This is currently very WIP! This is eventually intended to implement #220.

Current status: if an external screen is connected, the VM display will be moved there. This breaks touch and cursor input to the VM, which will require separating the touch handling from the MTKView to fix. As a 'band aid' fix, I have redirected touch handling to the VM container view (VMDisplayMetalViewController+Touch.m), which enables a small portion of the screen to be used to control the VM pointer.

Help needed with mirroring mode: I have attempted to implement a display mirroring mode by introducing a second MTKView on a new VMExternalDisplayMetalViewController.swift and setting its UTMRenderer, sourceDisplay and sourceInput the same as the internal ones. The relevant code is in ExternalScreenController.swift line 126. Unfortunately with regards to the UTMRenderer and MTKView, I have no idea what I'm doing. So if you use this mode by uncommenting it, that currently results in a blank screen.

@conath conath mentioned this pull request Oct 9, 2020
@osy
Copy link
Contributor

osy commented Oct 10, 2020

Yeah, I kinda regret this code but the way vmDisplay and vmInput are set is:

  1. cs_display_monitors calls the delegate method spiceDisplayCreated with CSDisplayMetal and CSInput created for the new display
  2. In UTMSpiceIO, spiceDisplayCreated sets properties on its delegate _delegate.vmDisplay and _delegate.vmInput. In order to prevent race conditions, UTMSpiceIO's setDelegate also sets those two properties.
  3. UTMSpiceIO's delegate is VMDisplayMetalViewController. In transitionToState (called by UTMVirtualMachine) for kVMStarted, the two UTMRenderSource are passed to the renderer. In order to prevent race conditions (the kVMStarted message is dropped), this is also done in viewDidLoad.

So when you copy self.vmDisplay and self.vmInput in viewDidLoad, you are likely just copying nil at this point. You either have to re-pass them at kVMStarted or set up a KVO on those two.

If I have time, I might go through and re-arch this who procedure to be more understandable. This bad design was a result of incrementally adding code as new features were introduced.

@conath
Copy link
Contributor Author

conath commented Oct 13, 2020

You are right, the values are still nil when the view loads. I changed the implementation to grab the UTMRenderSource references after the VM starts, which worked. I then tried to create a second MTKView on the external view controller with the same render source as internally. However as soon as I set the delegate in the next draw call, it crashes. I now assume that a UTMRenderSource can't be used with two MTKViews. So I abandoned that idea and focused on creating an extended display mode.

I read through some of the CoreSpice code and have a gist of what happens to create the virtual display. However I don't see anywhere where I could call a function to create another display. When a new display channel is needed that seems to be signaled by Spice on "channel-new" with type display. How can I cause that signal? Am I understanding it correctly in the first place?

@osy
Copy link
Contributor

osy commented Oct 13, 2020

Not 100% sure on this but look at requestResolution in CSDisplayMetal. It's currently setting for self.monitorID which afaik is always 0 right now. But it seems like you can pass in 1,2,...,15 to configure up to 16 displays. spice_main_channel_send_monitor_config will issue the VD_AGENT_MONITORS_CONFIG request. If the guest agent responds with MONITORS_CONFIG this is seen by cs_display_monitors in CSConnection. There is already code here to setting up a new CSDisplayMetal and CSInput but I don't think it's ever been tested. Once you have those you can create a new UTMRenderer and so on.

@conath
Copy link
Contributor Author

conath commented Oct 14, 2020

Thanks for that hint. I was able to extend the CSDisplayMetal implementation to allow me to request a resolution for a different display. However when that call happens, I get the following error printed in the console:

qemu-system: GSpice: spice_main_channel_send_monitor_config: assertion 'c->agent_connected' failed

The cs_display_monitors callback doesn't happen currently (presumably because of this assertion failure). Any way to debug that?

Worth noting is that I tried this with a Win98 VM without the spice tools installed.

@osy
Copy link
Contributor

osy commented Oct 14, 2020

That happens normally. libspice-gtk should automatically send the monitor config once the agent does connect (SPICE guest tools needs to be installed). Calling spice_main_channel_send_monitor_config is for the race condition where the agent is already connected before we called spice_main_channel_update_display_enabled.

@conath
Copy link
Contributor Author

conath commented Oct 14, 2020

Oh alright. I'll try it again with the prebuilt VM that includes the spice tools.

@conath
Copy link
Contributor Author

conath commented Oct 14, 2020

Just tried it with the "Debian 10.4 (LDXE) ARM" vm from the website, which includes SPICE tools. The output after the requestResolution call is now:

qemu-system: info: GSpice: channel-main.c:1131 main-1:0: sending new monitors config to guest
qemu-system: info: GSpice: channel-main.c:1148 main-1:0: monitor #0: 1366x1024+0+0 @ 32 bpp
qemu-system: info: GSpice: channel-main.c:1148 main-1:0: monitor #1: 1920x1080+0+0 @ 32 bpp
qemu-system: info: GSpice: channel-main.c:1066 #0 +0+0-1366x1024
qemu-system: info: GSpice: channel-main.c:1066 #1 +1366+0-1920x1080
qemu-system: info: Spice: reds.c:1158:reds_on_main_agent_monitors_config: monitors_config->num_of_monitors: 16

However cs_display_monitors is not called.

@osy
Copy link
Contributor

osy commented Oct 14, 2020

Do you get the log message received new monitors config from guest? If not, maybe Linux isn't automatically "enabling" the new display?

@conath
Copy link
Contributor Author

conath commented Oct 14, 2020

No, I don't get that message. I tried to run xrandr -q and xrandr -d :1 which both gave me unable to open display.

@osy
Copy link
Contributor

osy commented Oct 14, 2020

No, I don't get that message. I tried to run xrandr -q and xrandr -d :1 which both gave me unable to open display.

sudo ?

@conath
Copy link
Contributor Author

conath commented Oct 14, 2020

CF6539FA-B4E5-4B6B-ADD8-F2A0AF3C5CD0

Same with sudo

@osy
Copy link
Contributor

osy commented Oct 14, 2020

Just to be clear, auto-resolution changes do work? (i.e. rotating the screen)

@conath
Copy link
Contributor Author

conath commented Oct 14, 2020

No, that isn‘t working. In VM Display settings, Fit to screen is enabled, Retina mode is disabled. I‘m on iPad Pro 12.9 2nd gen, jailbroken on iOS 13.3. The VM is the "Debian 10.4 (LDXE) ARM" from the website (on the website it says that auto resolution change is not supported due to a bug, is that up to date?)

Maybe you could try this branch on one of your devices & VMs?

@osy
Copy link
Contributor

osy commented Oct 19, 2020

Still trying to figure out why the second display won't show up but I've ran into a couple of other issues:

I've gotten rare crashes in drawRegion. The cause may be that _canvasData is assumed to be locked by _drawLock. However, it is valid for libspice to return the same _canvasData to two different monitors (they could share a canvas). Therefore either the _drawLock has to be shared as well or a separate lock should be created.

Also, it seems like when I rotate the iPad (simulator) or use split view, it no longer sends the right resolution to SPICE.

@osy
Copy link
Contributor

osy commented Oct 19, 2020

Oh. Duh. We only have one VGA device.

https://people.freedesktop.org/~teuf/spice-doc/html/ch02s07.html

@conath
Copy link
Contributor Author

conath commented Oct 22, 2020

I tried the suggestion from the website you linked to, which was to add the GEMU argument “-vga qxl -device qxl”.

That results in the following error message when the VM starts:
1964A8E9-62FC-4944-9F23-D7FA145137B4

Edit: #303 seems related: “Add vga option (cirrus / std / vmware / qxl)”

@brunocastello
Copy link

Hi I am just curious. What's the current status on this? Is it working? If yes, how does it work, what are the current caveats, issues, supported VM OSes, requirements... ? A Samsung Dex like experience running a VM on my iPhone would be amazing IMO.

@conath
Copy link
Contributor Author

conath commented Apr 28, 2021

What's the current status on this?

It's not working and needs further development to get to a working state.

@osy
Copy link
Contributor

osy commented Apr 28, 2021

The rework in CocoaSpice on the dev branch should solve some of the issues but also require a lot of refactoring to be done.

@osy osy force-pushed the master branch 3 times, most recently from 9c70901 to cde596e Compare May 23, 2021 23:14
@conath conath added help wanted Extra attention is needed qemu QEMU related labels Oct 20, 2021
@osy
Copy link
Contributor

osy commented Aug 5, 2022

External displays have been implemented in 4.0.0

@osy osy closed this Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed qemu QEMU related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants