Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Widget life-cycle model - bug - Stack calls size_rules without set_rect on inactive children #486

Closed
dhardy opened this issue Mar 23, 2025 · 2 comments

Comments

@dhardy
Copy link
Collaborator

dhardy commented Mar 23, 2025

A bug is observable in #485. Example output from examples/gallery.rs in debug mode (widget varies but is usually a ScrollBar under a ScrollLabel, i.e. an auto-hiding ScrollBar):

thread 'main' panicked at crates/kas-core/src/core/data.rs:82:13:
WidgetStatus of #1002110101: require SetRect, found SizeRulesY

The cause does not appear to be new:

  1. <Stack as Layout>::size_rules may call size_rules on in-active children
  2. <Stack as Layout>::set_rect only calls set_rect on the active child
  3. A requested timed update is delivered to a child of a formerly active page, resulting in calling Events::handle_event on a child which has not had set_rect called (since the last call to size_rules)

This violates the widget state model: set_rect should be called after size_rules before most other methods such as event_handler. Some possible solutions:

  • Do not require re-calling set_rect after size_rules if formerly called. This is an incomplete fix since in theory nothing prevents delivery of events to a Stack page which has not yet been made active (and thus never had set_rect called).
  • Let Stack more fully track the state of children and avoid delivering events to children which are not active. This opens another potential issue: a former child will not receive events such as Event::lost_nav_focus which could in theory cause issues in widget code; this may be fixable by re-configuring a child each time it is made active but (a) this is otherwise unnecessary work and (b) this is an under-specified part of the widget model (though ListView does exactly this when re-assigning its child widgets to a new data entry).
  • Stack could avoid querying size_rules on inactive children. Undesirable.
  • Stack could call set_rect on each child it called size_rules on. This seems unnecessary.
@dhardy
Copy link
Collaborator Author

dhardy commented Mar 23, 2025

The more general problems here are:

The widget life-cycle is underspecified

  • Should all widgets assume that all in-progress events are cancelled when re-configured? Perhaps yes (due to the way ListView works currently, though that could instead re-create widgets on re-assignment only re-using the memory allocation).

  • Should we require that set_rect is always called after size_rules before other methods? This appears unnecessary; e.g. it should be valid to query for updates to size requirements without applying any changes. No.

  • Document exactly which widget state transitions are valid.

Poor enforcement of widget life-cycle

The enum WidgetStatus (debug only) panics on invalid state transitions (as demonstrated above). But we do not:

  • Enforce correct call order. This would require that children be managed through kas-core code instead of directly accessible to custom widgets.
  • Enforce anything with regards to direct method calls to child widgets. Several widgets do make use of this, e.g. CheckButton is a wrapper around CheckBox which directly calls some inner methods; outright preventing this would make it harder to re-use components.
  • Enforce access to state which should not yet be available. A cleaner design might use a Finite State Machine where user state is stored in an enum, but this would be messy to enforce on user-defined widgets (especially considering we have so many valid states: initial, configured, horizontal size queried, ...). Another possible cleaner design would be to make size_rules take &self (i.e. not consider it a state transition) and make the Rect (or user-defined state generated by calling set_rect) available only after calling set_rect. This is not easy to integrate into Kas's widget model.

@dhardy dhardy changed the title Bug: Stack calls size_rules without set_rect on inactive children Widget life-cycle model - bug - Stack calls size_rules without set_rect on inactive children Mar 23, 2025
@dhardy
Copy link
Collaborator Author

dhardy commented Mar 24, 2025

#485 has been updated to fix the observable bug and to clarify that configure, size_rules and set_rect may be called repeatedly without expectation of further actions. One related fix is included, namely that should a List/Splitter/Stack widget be reconfigured with a new id, we now ensure children also get a new (descendant) id.

Event handling code was checked to look for any assumptions that some event such as PressEnd or PopupClosed would eventually be received (which might not be the case when a widget gets reconfigured with a new id), but mostly this does not appear to be an issue. Thus, the un-ticked item above is not formally solved here but at least does not appear to be an issue in existing code.

@dhardy dhardy closed this as completed Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant