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

feat: headless rendering #1176

Closed
wants to merge 15 commits into from
Closed

feat: headless rendering #1176

wants to merge 15 commits into from

Conversation

1024bees
Copy link

@1024bees 1024bees commented Dec 30, 2021

Hey folks, this is a first best effort to add headless rendering into Iced. Alot of these changes are built up from #957

Changes visible to end users

  • Adding a Screenshot type and screenshot Commands. Screenshots can be serialized to/from pngs. the interface provided is fn take_screenshot(message_generator : Box<dyn Fn(Option<Screenshot>) -> Message>) -> Command<Message>
  • Adding offscreen rendering enabled by the headless flag in Settings
  • Adding a run_with_message_trace interface to run an application without user interaction. This interface allows end users to provide "Message Traces" that a vectors of items (Application::Message, std::time::Duration). Each Message is sent to the Application after Duration passes. After the last message is dispatched.
  • Added the headless example to show case the previously mentioned interfaces
  • Added a screenshot button to the Pokemon button (open to revert this, this was more for my own debugging).

Internal changes:

  • Adding a RunetimeArgs type to encapsulate extra args passed to crate::runtime::application::run
  • Added a VirtualCompositor trait and a VirtualCompositor trait bound to each Compositor backend (wgpu, opengl). This trait object is passed as an argument to run_command, and allows run_command to be shared across glutin and winit still.

Comments

  • I like the idea of "Message Traces" and think they're a good first effort to provide a "test-like way" to run applications. Not sure if Message Trace is the best name to describe the concept though.

  • The VirtualCompositor trait is a leaky abstraction; it broadly encapsulates behaviors from both wgpu and opengl. To be concrete, the requires_rerender, queue_screenshot, is_screenshot_queued and dequeue_screenshot are all methods that are only implemented for the wgpu compositor. These methods are required to support screenshotiting wgpu applications that are presenting windows to the user. They are required because currently, textures that are presented to the screen can NOT be directly copied from into a buffer.

This is a limitation of WGPU's (to be explicit, the issue is that Textures attached to Surfaces cannot configured to be the sources of copies) current implementation, and the folks over there said that in theory, this limitation could be addressed. As a work around, the current implimentation of screenshots on WGPU requires us to re-render the screen to a framebuffer, that is then copied into a screenshot object. If/when the limitation on textures attached to surfaces is addressed in WGPU, the screenshotting strategy (and VirtualCompositor trait) should be streamlined across wgpu and opengl compositors.

  • not sure if the location of Screenshot and RuntimeArgs data structures is correct. I put them both in iced_native, worth reviewing.

  • I would like to add a mechanism to inspect Application state once a "Message Trace" has been exhausted. One idea is that we could have run_with_message_trace return the Application, and state could be inspected from that point onwards.

Blockers

  • general code clean up, passing lints
  • Adding examples in which cargo test is called to run an application, check the differences between a "golden" png and a png created by runtime

@1024bees 1024bees changed the title headless rendering feat: headless rendering Dec 30, 2021
@1024bees
Copy link
Author

1024bees commented Jan 2, 2022

with ee2d3ec55655373767c06044513c32407deb59e4, we've added a simple test for the headless application that can be tested via cargo test!

to run, this test, you can clone this branch, cd into examples/headless and then run cargo test -- --test-threads=1.

If we do not limit test threads to 1, then we crash trying to create a winit::EventLoop because EventLoops are required to be created on the main thread

@hecrj
Copy link
Member

hecrj commented Jan 3, 2022

Hey! Thanks for giving this a shot!

There were already some existing efforts in the same direction in #957 by @derezzedex with some discussion. It'd be great to compare both approaches and finally settle on the best way to expose the offscreen functionality.

When it comes to testing, I believe there is a lot of room to build the functionality on top of the iced crate itself, maybe in a new iced-test crate. There are many cool features we could implement! From exporting / importing message histories to inspecting widget trees and interacting with them. I remember having a great time with elm-html-test, which I think could be a great source of inspiration.

@1024bees
Copy link
Author

1024bees commented Jan 3, 2022

There were already some existing efforts in the same direction in #957 by @derezzedex with some discussion. It'd be great to compare both approaches and finally settle on the best way to expose the offscreen functionality.

I agree, we should have consensus on which direction we'd like to go with offscreen rendering. To give my perspective comparing #957 and this PR:

features in #957 :

  • a feature flag to gate if an application is in headless mode
  • the first frame of an application is captured in headless mode

features in this PR:

  • a flag in iced::Settings is used to run an application in headless mode.
  • any frame may be captured via iced::window::take_screenshot. take_screenshot api may be used in either headless or "normal" operation.
    *the run_with_message_trace interface to run headless applications "programatically"

When it comes to testing, I believe there is a lot of room to build the functionality on top of the iced crate itself, maybe in a new iced-test crate. There are many cool features we could implement! From exporting / importing message histories to inspecting widget trees and interacting with them. I remember having a great time with elm-html-test, which I think could be a great source of inspiration.

This is a good point, I agree that it would be best to have all testing live as a layer "above" iced and an iced-test crate sounds like a good approach for that. I think this PR should not try to provide integration testing APIs, and instead should look to build + polish interfaces on which an iced-test crate can be built on top of.

the big question that I'd like to answer with this PR is: what API should we expose in iced to run a headless application

the two ways I think we could address this question:

  • We could build functions like run_with_message_trace into iced, that iced-test could build on top of.
  • A lightweight approach we could take is simply exposing the EventLoopProxy and the UnboundedSender<Event<Message>> used by run_instance. This would allow iced-test crate to hook into into the application runtime without having test state bleed into application logic.

There are a lot of ways to approach this though. what does everyone think?

FWIW, the run_with_message_trace interface looks like the following in practice.

pub fn main() -> iced::Result {
    let msg_trace = vec![
        (Message::IncrementPressed, Duration::new(1, 0)),
        (Message::IncrementPressed, Duration::new(2, 0)),
        (Message::IncrementPressed, Duration::new(2, 0)),
        (Message::TakeScreenshot, Duration::new(3, 0)),
    ];
    Counter::run_with_message_trace(
        Settings {
            headless: true,
            window: iced::window::Settings {
                size: (600, 600),
                ..iced::window::Settings::default()
            },
            ..Settings::default()
        },
        msg_trace,
    )
}

I think a iced-test crate could build on top of this interface in numerous ways: record message traces, provide utilities to serialize and deserialize message traces, etc. Typing this out makes it clear that a message trace should be formalized into its own type instead of being a Vec<(Message,Duration)>

@hecrj hecrj force-pushed the master branch 2 times, most recently from e6ab610 to 8b0f2e6 Compare January 19, 2022 15:04
@1024bees
Copy link
Author

I've peeled back a lot of changes originally in this commit; an updated list:

Changes visible to end users

  • Adding a Screenshot type and screenshot Commands. Screenshots can be serialized to/from pngs. the interface provided is fn take_screenshot(message_generator : Box<dyn Fn(Option<Screenshot>) -> Message>) -> Command<Message>
  • Adding offscreen rendering enabled by the headless flag in Settings

Internal changes:

  • Added a VirtualCompositor trait and a VirtualCompositor trait bound to each Compositor backend (wgpu, opengl). This trait object is passed as an argument to run_command, and allows run_command to be shared across glutin and winit still.

I'm open to removing the Screenshot changes introduced by this PR, or constraining it s.t. only headless applications may be screenshot (this would remove non-trivial overhead for wgpu).

On a related note; I've created a small iced-test crate here. This crate uses the screenshot APIs brought up by this PR, and provides an interface to run applications headlessly via cargo test. Would love to get feedback from folks on what they'd like from an iced_test crate.

* headless propegation from Application settings-> Compositor settings
* Removed the InitHeadlessBuffer; initializing a headless buffer is done
  now in Compositor instation (if the setting is set)
* refactored code for less copy and pasting (still some to clean up in
  the read read impl

current strategy is to create a buffer anytime we want a screenshot (and
we are in non-headless mode. we dont map this buffer when we instatiate
it, but it might be faster to do that
@togetherwithasteria
Copy link

I'm open to removing the Screenshot changes introduced by this PR, or constraining it s.t. only headless applications may be screenshot (this would remove non-trivial overhead for wgpu).

To be fair, if we would want to have Screenshot support in Iced, it's much better to split them in another PR. 😇

@togetherwithasteria
Copy link

As a side note, it's maybe a good solution to have them behind a screenshot feature flag, since it's not needed by most applications except for testing.

@1024bees
Copy link
Author

I think this PR was trying to do a little too much :) I spun these changes out into a third party lib earlier this year: https://github.com/1024bees/iced_test. It almost certainly out of date as I've not been focusing on iced, but it should be a starting point if anyone wants to add in testing utils (if they havent been added in).

Closing this now. Cheers!

@1024bees 1024bees closed this Nov 26, 2022
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.

3 participants