On 07.10.2016 12:08, Pekka Paalanen wrote:
> On Fri, 30 Sep 2016 23:25:29 +0200
> Armin Krezović <[email protected]> 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ć <[email protected]>
>> ---
>>  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.

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to