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

Custom MeasureFunc #7730

Closed
wants to merge 8 commits into from
Closed

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Feb 17, 2023

Objective

At the moment Bevy UI has a very basic, unmodifiable MeasureFunc.

Problems:

Solution

Create a new trait MeasureNode and add a Box<dyn MeasureNode> field to CalculatedSize.
Have upsert_leaf use the boxed MeasureNode to create the MeasureFunc for the node.

Changelog

  • Added a new trait MeasureNode.
  • Added new structs ImageMeasure and BasicMeasure that implement MeasureNode.
  • Add a field to CalculatedSize called measure that takes a boxed MeasureNode.
  • upsert_leaf uses the measure of CalculatedSize to create a MeasureFunc for the node.

* Added a new trait `MeasureNode`.
* Added new structs `ImageMeasure` and `BasicMeasure` that implement `MeasureNode`.
* Add a field to `CalculatedSize` called `measure` that takes a boxed `MeasureNode`.
* `upsert_leaf` uses the `measure` of `CalculatedSize` to create a `MeasureFunc` for the node.
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets labels Feb 17, 2023
@ickshonpe ickshonpe changed the title custom measure functions Custom measure functions Feb 17, 2023
@ickshonpe ickshonpe changed the title Custom measure functions Custom MeasureFunc interface Apr 13, 2023
@ickshonpe ickshonpe changed the title Custom MeasureFunc interface Custom MeasureFunc Apr 13, 2023
@ickshonpe
Copy link
Contributor Author

This PR is the wrong approach, the whole flex module has big underlying problems I didn't understand before.

@ickshonpe ickshonpe closed this Apr 15, 2023
@TimJentzsch
Copy link
Contributor

@ickshonpe What are the underlying problems?
Did we capture them in issues already?

@ickshonpe
Copy link
Contributor Author

@ickshonpe What are the underlying problems? Did we capture them in issues already?

The major problem with this PR is that it clones and double-boxes the MeasureFunc when I think the layout algorithm should be able to access it directly from the CalculatedSize component.

There are a lot of internal performance bugs that are hard to find because they only have an impact when the UI updates and not every frame. I just published #8402 which fixes another unnecessary update bug.

@nicoburns
Copy link
Contributor

nicoburns commented Apr 17, 2023

@ickshonpe Would it be helpful if MeasureFuncs were able to borrow from their environment? I've been considering adding such a feature to Taffy, although there are a couple of possible designs:

  • MeasureFunc's could take an "environment" parameter. The user would would be pass the "environment" (which could be a reference) to compute_layout, which would just pass it in every time it called a MeasureFunc.
  • OR Individual MeasureFunc's could be replaced by a single measure func which is passed in to compute_layout and get's called with the node's id as a paramter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants