On Sat, Feb 27, 2016 at 01:56:53AM +0100, Benoit Gschwind wrote: > Hello Bryce, > > Thanks for the summarization, I have few comment that I share here. > > Le 26/02/2016 22:37, Bryce Harrington a écrit : > > To followup Pekka's recent libweston thread, here's the next actions it > > looks like we should take? > > > > a. Revert 5ffbfffa > > b. Land https://patchwork.freedesktop.org/patch/67547/, which covers > > the drm-backend. (Is this patch proposal good as is, or would it > > benefit from any additional review?) > > c. Defer the two alternative options for now > > https://patchwork.freedesktop.org/patch/73206/ > > > > https://patchwork.freedesktop.org/patch/[73035,73036,73037,73038,73039] > > d. Review/update wayland-backend and x11-backend to comply > > https://patchwork.freedesktop.org/patch/74553/ > > https://patchwork.freedesktop.org/patch/74504/ > > > > This establishes Giulio's "Well Defined Structs" approach for > > configuring libweston backends. This uses versioned structs for > > communicating parameters with the backends. > > > > If no one raises an objection to this plan, I can tackle (a), (b) and > > (c) myself directly. For (d), offhand it appears they at least need to > > add the structure versioning support, but might be suitable to consider > > landing after that? > > (a), (b), (c) and (d) are fine for me.
> I would add an action to sort weston API function (i.e. function that > are exposed to user(developer)) from functions that must be kept > hidden. For this action I think we should start a libweston.h (or what > ever name that look good) and the related libweston.a . Currently > implementing a compositor from weston look durty, some low level > actions require to use libwayland directly and "link" them to > weston. Another issue is that user(developer) can use any weston > internal API and some of those functions may be unsafe. It sounds like this would be suitable to do as a followup patchset? I'm sure it'll deserve some additional review and discussion. For the header file naming, in my /usr/include, the "libfoo.h" naming style is less often used than something like "libfoo/foo.h". For example, 'libdrm/drm.h'. Plain old foo.h seems to be the most common, although there's probably value in disambiguating libweston from weston. In addition to the header file, api docs would be nice too. > I do not know well where the border are but we should start to draft one > in the same manner we started with backends. > > > <side topic> > I waiting for your review for (d) and I expect to provide patch for > others back-end soon (i.e. maybe once per week until all testable > back-end are done). Will take a look today or tomorrow. > Also note that I didn't added versionning header for two main reasons: > 1. I do not know which best versionning method I should use, is one > incremental number enough ? > 2. I do not know how the user(developer) is expected to fill it properly. > > Once I get answer to this both questions I will be happy to updates > previous patch :) Is pq's feedback sufficient for progressing on this now? > </side topic> > > > > > > > > > --- > > A couple of more sophisticated solutions were proposed and evaluated, > > but it feels like our requirements here are modest, and so simpler is > > probably better. Things aren't cast in stone here and we can always > > move to something more advanced later if conditions warrant it. > > > > In mind of this, here are the assumptions that are leading us to this > > choice. In the future if these assumptions fail to hold adequately, > > then this design should be re-evaluated. > > > > 1. libweston has no stability promises, and won't for a long time, and > > it is and will remain parallel installable. We are freely able to > > redo tomorrow any bad decisions we make today. > > > > 2. New options will be appended to the the struct, which will avoid ABI > > breaks. > > > > 3. The struct is versioned, so if we do need to break ABI for some > > reason, we can and backends can thereby check and verify their > > version. > > > > 4. Configuration needs by the backends are on the tame side, mostly > > just involving basic data types (strings, ints, etc.) > > This assumption (4) is already wrong, back-end at the moment needs what > I consider complex setup, in particular to define outputs list or the > output_setup callback in drm-backend. I figured this would be the first violated. Lists of basic types (e.g. list of strings) is probably not that big of a deal. Or are these output lists containers of output structs? Anyway, we can probably hand wave one or two special cases here without too much worry. > > 5. Backend configuration is internal to the backend. End users won't > > be exposed to it, and only backend developers will need to tinker > > with these things. > > > > 6. Additions of or changes to configuration parameter definitions are > > going to be done by developers who are either part of the Wayland > > development community, or will be sending all of their changes to > > the community. > > > > 7. All libweston backends will be living in the weston repository. We > > do not provide support for third-party backends. > > > > It's probably non-fatal if we periodically violate one or two of these > > assumptions, but if we get beyond that it should be a cue to revisit our > > approach. > > > > > > So far, two other alternatives were considered; here are the key design > > differences: > > > > A. Opaque Structs > > https://patchwork.freedesktop.org/patch/73206/ > > > > This still uses structs for sharing configuration parameters but the > > structs are hidden as internal details. The backend instead uses > > function calls to fill in parameters. > > > > While this does decouple things a bit and avoid ABI breakge on > > structure definitions, this requires coding and maintaining an array > > of backend-specific functions, which bring their own API breakage > > potentials. > > > > This is essentially an incremental enhancement of the "Well Defined > > Structs" scheme, so would be straightforward to upgrade to later. > > > > > > B. Getter/Setters > > https://patchwork.freedesktop.org/patch/[73035,73036,73037,73038,73039] > > > > Establishes a key/value backend config API, with separate > > getter/setter calls for different config item data types. Backends > > then call these getters to retrieve configuration variables. > > This system includes provision for sections and default values. > > > > This is certainly a much more flexible system, but many of the same > > problems will exist when configuration parameters change. Just that > > instead of erroring at the API/ABI layer it breaks at the > > parser/configuration layer. Shifting to this lower level means > > errors may get detected later on, or may be missed entirely, where a > > broken struct will be more highly visible. > > > > Alternative A might be worth considering if we start seeing > > proliferation of backend options, or if the configuration settings start > > going beyond basic types. > > > > Alternative B could become more convenient if we need to expose options > > to end-users or if we start seeing third-party backends. > > > > Bryce > > _______________________________________________ > > wayland-devel mailing list > > wayland-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > > > > I agree with all uncommented parts. I have also comments about the last > part about alternatives, but I can't find a good way and strong > arguments to expose them. Thus, I will agree with what is written and do > not write down my immature comments; but, maybe, when they will be more > mature, I will share them. Please don't assume that those immature > comments are pro-A comments. Thanks, Bryce > Best regards > > -- > Benoit Gschwind _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
