ardera / flutter-pi

A light-weight Flutter Engine Embedder for Linux Embedded that runs without X11 or Wayland.
MIT License
1.62k stars 162 forks source link

Use specific display output #201

Open shahinv opened 3 years ago

shahinv commented 3 years ago

I could not find any specific way to control display output (DSI/HDMI0/HDMI1). As far as I know display output could be controlled through dispmanx display id:

0 = Default display 2 = HDMI0 (Pi4) 4 = Touchscreen 5 = HDMI (Pi3-) 6 = Select non-default display 7 = HDMI1 (Pi4)

ardera commented 3 years ago

Yeah right now flutter-pi just uses the first display it finds, though it uses KMS not dispmanx (dispmanx is deprecated AFAIK)

Right now it's pretty easy to workaround, so lower priority, but once I work on it the Qt EGLFS config file (https://doc.qt.io/qt-5/embedded-linux.html) could serve as a template

shahinv commented 3 years ago

Thank you for the help.

I take a look at the source code and could not recognize where display selection happen. Can you share a tip on the workaround?

ardera commented 3 years ago

Sure, output selection happens here: https://github.com/ardera/flutter-pi/blob/e60ccd19d753674dc9481d87a073dd7e9c9bf4f9/src/flutter-pi.c#L1267

.

That loop will just select the first connected connector. If you want a specific connector like HDMI you can just skip all non-HDMI connectors.

Though I don't know how to differentiate HDMI0 and HDMI1 via KMS. But the modetest utility from libdrm may come in useful for finding that out.

You can maybe differentiate them via the PATH property, or via the connector_type_id. (That's at least how modeset determines the connector name)

For example

for_each_connector_in_drmdev(flutterpi.drm.drmdev, connector) {
  if (connector->connector->connection == DRM_MODE_CONNECTED && connector->connector->connector_type == DRM_MODE_CONNECTOR_HDMIA) {
    // only update the physical size of the display if the values
    //   are not yet initialized / not set with a commandline option
    if ((flutterpi.display.width_mm == 0) || (flutterpi.display.height_mm == 0)) {
      if ((connector->connector->mmHeight % 10 == 0) &&
          (connector->connector->mmWidth % 10 == 0)) {
        // don't change anything.
      } else {
        flutterpi.display.width_mm = connector->connector->mmWidth;
        flutterpi.display.height_mm = connector->connector->mmHeight;
      }
    }

    break;
  }
}
Taha-Firoz commented 2 years ago

Yeah right now flutter-pi just uses the first display it finds, though it uses KMS not dispmanx (dispmanx is deprecated AFAIK)

Right now it's pretty easy to workaround, so lower priority, but once I work on it the Qt EGLFS config file (https://doc.qt.io/qt-5/embedded-linux.html) could serve as a template

🤣 I had the same problem on my pi and I was only able to debug it by looking into QT's EGLFS config file. This is exactly what I want to add into this embedder, a config file that's an alternative to the command line params. If you're interested in having something like that merged into the project I'll be more than happy to help because I've already extensively studied QT's KMS config file implementation.

ardera commented 2 years ago

@Taha-Firoz Sounds awesome! Though if you do that, I think it's a good idea to write that stuff on top of the feature/compositor-ng branch. That's a huge refactor/rewrite of the display infra I've been working on, I just want to get it up to feature parity with master branch (mouse support is still missing for example) before I merge it. Don't want to add to many changes to master now since all that has to be added to feature/compositor-ng as well.

My plan was to add some kind of compositor_builder (a compositor is just the whole set of native windows, plus some global stuff) where you can just step-for-step pass all the json settings and at the end it gives you ready-to-use set of native windows.

i.e. assuming this json (just an example:

{
  "enableHwCursor": true,
  "windows": [
    {
      "drmdev": "/dev/dri/card0",
      "mode": "1920x1080@60p",
      "dimensions": "160x90",
      "framebufferSize": "960x540",
      "pixelformat": "RGB565"
    },
    { ... }
  ]
}

the calls to compositor_builder could look like this:

compositor_builder_set_enable_hw_cursor(builder, true);

compositor_builder_start_window(builder);
compositor_builder_set_drmdev_path(builder, "/dev/dri/card0");
compositor_builder_set_mode_str(builder, "1920x1080@60p");
compositor_builder_set_physical_dimensions_str(builder, "160x90");
compositor_builder_set_framebuffer_size_str(builder, "960x540");
compositor_builder_set_pixfmt_str(builder, "RGB565");

// ...

Or maybe you have a better idea. Bonus points if we could expose a similar interface via cmdline and then make commandline arg parsing and json file parsing both use the compositor_builder for setting up the display pipeline.

Taha-Firoz commented 2 years ago

Your design seems to be a lot similar to ivi-homescreen json configuration file implementation. Since Joel is interested in eventually pulling flutter-pi in for ivi-homescreen's DRM implementation do you think we should just take the implementation from there and modify it.

Ivi-homescreen does use paths to automatically pick up config files from instead of passing their paths in as command line parameters, I think the simplest approach should be taken. Only allowing either a config file (path given via command line) or a command line configurations to used at a time, this would allow a majority of flutter-pi's command line argument processing to stay the same and we can add in the config file module separately.

I do enjoy QT's implementation of a kms conf file, very to the point

{
    "device": "/dev/dri/card1",
    "hwcursor" : false,
    "outputs": [
        { "name": "DSI1", "mode": "800x1280", "primary": true },
        { "name": "Composite1", "mode": "off" } 
    ]
}
ardera commented 2 years ago

Your design seems to be a lot similar to ivi-homescreen json configuration file implementation. Since Joel is interested in eventually pulling flutter-pi in for ivi-homescreen's DRM implementation do you think we should just take the implementation from there and modify it.

If you can reuse code that's great, but not sure we can take from ivi-homescreen bc Apache 2.0 license is slightly more restrictive than MIT AFAIK. Other than that, C-code should be preferred when possible (though if you want to write in C++ we can talk about that 😄). We can try to stay compatible to ivi-homescreens config though so pulling in flutter-pi is not that much of a breaking change.

Only allowing either a config file (path given via command line) or a command line configurations to used at a time, this would allow a majority of flutter-pi's command line argument processing to stay the same and we can add in the config file module separately.

I think that approach is fine at the start, but IMO ultimately the cmdline arg processing should be modified to rely on the new configuration module as well. It's not like that's a huge change, the cmdline processing function is only a few lines :) And I much rather have a single, well-used codepath.

Taha-Firoz commented 2 years ago

Other than that, C-code should be preferred when possible (though if you want to write in C++ we can talk about that 😄)

I prefer Rust but since we aren't going down that rabbit hole I'll try my hand with C, the only annoying part might be working with json in C instead of C++.

If you can reuse code that's great, but not sure we can take from ivi-homescreen bc Apache 2.0 license is slightly more restrictive than MIT AFAIK.

Yes I saw that but I'm sure our C implementation will be different enough not get us into any trouble.

What are you using for parsing json in C, futter-elinux is using Tencent rapidJson.

ardera commented 2 years ago

What are you using for parsing json in C, futter-elinux is using Tencent rapidJson.

I'm using jsmn.h, it's a header-only library, though tbh calling it a parser is probably an overstatement, it's more like a tokenizer.

But if you feel like using something nicer for json handling, I'd be open for that as well.

Taha-Firoz commented 2 years ago

Basically I've used flutter-pi as the first embedder for my project and in between moved over to flutter-elinux where I seem to be having a similar conversation on. There are certain problems with flutter-elinux that have come up which is pushing to move back to flutter-pi however the lack of control over DKMS for both projects is a major problem for me which is why I want to work on it.

However as a disclaimer since I've spent a lot of time going through and working with the flutter-elinux code base, I am heavily influenced by it.

Taha-Firoz commented 2 years ago

Is this library cJSON fine? I'll use ivi-homescreens config as reference?

ardera commented 2 years ago

@Taha-Firoz yes, that's fine by me. (Both things.) I could also migrate the platform channel code to cJSON too, seems nicer than jsmn.

Taha-Firoz commented 2 years ago

Checkout my fork should I create a PR to get a review on what I've done or should we coordinate on issues for each review?

I just added basic config file parsing based on the template you sent, tbh I didn't really do anything I just pasted the example and copilot wrote the whole thing for me 😅

ardera commented 2 years ago

You can create a draft PR if you want, so we can coordinate the work better :) Or you get your prototype going first, how you prefer.

Looks pretty good already tbh, it's nice that you follow the rest of the flutter-pi codestyle :D Did copilot really write all that? That's amazing, I should get it too

Btw one small thing I've spotted: https://github.com/Taha-Firoz/flutter-pi/blob/538fc927c162586cf0d6d98f84b321f3fd1d3dc3/src/config.c#L179-L188

you can use

char *newpath;
ok = asprintf(&newpath, "%s/.config/", path);

there, it'll do the correct allocation for you. (May need to #define _GNU_SOURCE for that) (Though you can't do asprintf(&path, "%s/.config/", path); because the args are marked as restrict)

Mostly I try to do any declarations at the top and sort them in decending length, so

struct configuration *c;
char *path;
int ok;

That's probably a bit pedantic but I think it helps with readability.