engine icon indicating copy to clipboard operation
engine copied to clipboard

Build a GTK4 version of libflutter_gtk

Open robert-ancell opened this issue 1 year ago • 1 comments

Since only recent versions of GTK4 will work with Flutter we need to continue to support GTK3. This draft PR is the first stage of this and compiles two versions of the libflutter_gtk library to support both 3 and 4. The GTK4 library won't be useful at this point, that is being tackled in https://github.com/flutter/engine/pull/50960. I expect it will make sense to break out classes from FlView so we can more easily handle both GTK3 and 4 without too many confusing ifdefs.

At this point the first thing that needs confirming is this is a suitable way of building two libraries using gn, and making those rules as clear and concise as possible (they currently have a lot of copied code).

robert-ancell avatar Jun 13 '24 04:06 robert-ancell

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

flutter-dashboard[bot] avatar Jun 13 '24 04:06 flutter-dashboard[bot]

@chinmaygarde would love some help on how the gn rules should look!

robert-ancell avatar Jul 03 '24 23:07 robert-ancell

I highly recommend having the GN reference handy for this as that's what I am using throughout this reply. You can either access it online at https://gn.googlesource.com/gn/+/main/docs/reference.md or gn help at the command line.

It seems like you want to build a source set multiple times. Each with a different config applied to it. Besides the different config (and perhaps the name of the output artifacts), a lot of details seem to be the same.

Remember that a GN source_set, shared_library, executable, etc.. will only result in its sources being processed once. But you can use a template instead (gn help template) and stamp multiple targets from a template. The way the template is put together can be mostly identical to the source_set. But you will have to instantiate the template as many times as you need. In the template implementation, you’ll need to control the config based on template arguments.

Some rough pseudo code:

template("flutter_linux") {
  assert(defined(invoker.gtk_version), "The GTK version must be specified")

  shared_library(target_name) {
    # Tip: See `gn help forward_variables_from`
    if (defined(invoker.output_name)) {
      output_name = invoker.output_name
    }

    # All the common stuff goes here.
    sources = [
      "a.cc",
      "a.h",
    ]

    # This make it easier to use in string interpolation.
    gtk_version = invoker.gtk_version

    # Now, do stuff that is specific to the GTK version. I'd put everything
    # specific to a version in its own public and private configs. That way, it
    # cleaner and easier to maintain and reuse.

    # This assumes there is a :public_gtk3 and :public_gtk4 defined already.
    public_configs = [ "//path/to/config:public_gtk$gtk_version" ]

    # This assumes there is a :private_gtk3 and :private_gtk4 defined already.
    configs = [ ":private_gtk$gtk_version" ]

    # Follow the same pattern or use conditionals. Like so:
    if (gtk_version == "4") {
      deps = [ "something/only/4/needs" ]
    }
  }
}

# Now, stamp the templates

group("flutter_linux") {
  flutter_linux("gtk4") {
    gtk_version = "4"
    output_name = "flutter_linux_gtk4"
  }
  flutter_linux("gtk3") {
    gtk_version = "3"
    output_name = "flutter_linux_gtk3"
  }
  deps = [
    "gtk3",
    "gtk4",
  ]
}

chinmaygarde avatar Jul 05 '24 20:07 chinmaygarde

(PR triage): Hey @robert-ancell, thanks for your continued contributions! Is this PR still on your radar?

Piinks avatar Oct 22 '24 22:10 Piinks

Closing this, not currently working on it.

robert-ancell avatar Oct 23 '24 09:10 robert-ancell

GTK4 support dropped?

kjxbyz avatar Oct 23 '24 10:10 kjxbyz

Currently working on multi-view support in GTK3, which seems like it won't require the migration to GTK4 as we previously though.

robert-ancell avatar Oct 23 '24 11:10 robert-ancell