[PATCH weston 3/4] compositor: Add internal output object used for layout configuration

[Armin Krezović]

On 07.10.2016 12:08, Pekka Paalanen wrote:
> On Fri, 30 Sep 2016 23:25:29 +0200
> Armin Krezović <krezovic.armin at gmail.com> wrote:
> 
>> This adds weston specific output object, which contains information
>> about output's position, relative to other outputs. DRM and Windowed
>> backends code has been updated to make use of the new code and
>> parse new configuration options for a given output, if it was
>> specified.
>>
>> New configuration file options for [output] section have been added:
>>
>> left-of: Specifies which output should be on the right side of the
>> current output.
>>
>> right-of: Specifies which output should be on the left side of the
>> current output.
>>
>> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
>> ---
>>  compositor/main.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 81 insertions(+)
>>
>> diff --git a/compositor/main.c b/compositor/main.c
>> index 503016e..e379d8d 100644
>> --- a/compositor/main.c
>> +++ b/compositor/main.c
>> @@ -78,6 +78,18 @@ struct wet_compositor {
>>  	struct weston_config *config;
>>  	struct wet_output_config *parsed_options;
>>  	struct wl_listener pending_output_listener;
>> +	struct wl_list output_layout_list;
>> +};
>> +
>> +struct wet_output {
>> +	struct weston_output *output;
>> +	struct wl_listener output_destroy_listener;
>> +	struct wl_list link;
>> +
>> +	char *left_output_name;
>> +	char *right_output_name;
> 
> Hi
> 

Salut

> Rather than linking by name, would it not be easier to link by pointer
> to struct wet_output?
> 

I can try that, too.

> That means when you encounter a directive like "left-of=F5" and there
> is no output F5 yet, you would create a new struct wet_output for F5
> but leave the weston_output pointer NULL, to be filled out if F5
> actually appears.
> 
> That means you would always have a graph in memory instead of doing
> searches to find if output of this name actually exists. I'm not sure
> if that's actually simpler in the end.
> 

I'd still have to search for a wet_output matching an output when it
gets attached (implement a way of matching an unclaimed wet_output to
weston_output), and if it isn't there, I'll have to create it. That
includes a bit of complexity too. How much however, I can't say
until I try.

> Having the complete graph would allow you to "jump over" outputs that
> do not exist at the moment: e.g. order F1 -> (F2) -> F3.
> 

That could lead to waste of memory, as the output could never get attached,
and I'll have to implement a function free-ing such outputs, as I can't rely
on output destroyed signal as I currently do. Not to mention that the third
output needs to be moved when second one gets attached, so it's not really
a win-win situation, besides having a better picture in the memory.

We could accomplish that using wl_array with this implementation (instead
of wl_list that's currently used).

In the end, the implementation looks more complex than this one, and
we already configure outputs by name, so why not simply keep it?

>> +
>> +	bool position_set;
>>  };
>>  
> 
>> +	weston_config_section_get_string(section, "left-of", &out, NULL);
>> +	wet_output->right_output_name = out ? strdup(out) : NULL;
>> +	free(out);
>> +
>> +	weston_config_section_get_string(section, "right-of", &out, NULL);
>> +	wet_output->left_output_name = out ? strdup(out) : NULL;
>> +	free(out);
> 
> Yes, this looks like an intuitive approach, storing the relationship in
> inverse compared to how users write it out in weston.ini.
> 

Heh, this is my bad really. While rushing to finish this part before GSoC
deadline, I managed to invert the intended functionality. I just swapped
these two here, instead of going through the rest of the code as I was
sure the code would be changed anyways as soon as I send it for review.

It will be fixed as soon as we can agree on an approach.

> 
> Thanks,
> pq
> 

Cheers.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 837 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20161009/a2758f92/attachment.sig>