Refactor the weston_output init/fini/enable/disable and weston_compositor_add/remove_output mess
The following discussion from !637 (merged) should be addressed:
-
@afrantzis started a discussion: (+2 comments) The fix looks good, but ideally weston_output would be fully initialized with sane/valid values when created. The current partial initialization split between creation and enablement makes it more difficult to reason about what's valid under which scenarios. Although ensuring full valid initialization is outside the scope of this MR, perhaps we could take a first step here and move
wl_list_init(&output->paint_node_{z_order_}list)
fromweston_output_enable()
toweston_output_init()
?
Copying my reply:
That would be a bigger refactoring, as many fields already in weston_output
follow the enable/disable life cycle instead of init/fini life cycle. See weston_output_enable()
. These are mostly things that should semantically remain unused when the output is not enabled, i.e. their semantic life time matches enable/disable better than init/fini.
But I tend to agree. We could have more asserts to ensure that fields were properly unused at enable time and cleared at disable time. It just won't crash at the site of illegal access but at check time. Or, split weston_output
into the base struct and a dynamically allocated enabled struct.
It doesn't help that much of what weston_output_enable()
does is undone in weston_compositor_remove_output()
.
I think the big fat comment in weston_output_disable()
about turning non-enabled outputs off is also out-of-date nowadays. All this mess is inherited from the times when backends created outputs directly for all "connectors" they could light up, before weston_head
was introduced.
All that stuff could use a serious refactoring.