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

Let a Buffer figure out its height during set_size #70

Closed
hecrj opened this issue Feb 3, 2023 · 12 comments · Fixed by #271
Closed

Let a Buffer figure out its height during set_size #70

hecrj opened this issue Feb 3, 2023 · 12 comments · Fixed by #271
Assignees

Comments

@hecrj
Copy link
Contributor

hecrj commented Feb 3, 2023

While it is possible to use i32::MAX if there is no height limit for a Buffer, the resulting Buffer will use i32::MAX as the final dimension. This means that Buffer::visible_lines will not be very useful.

I think it'd be more useful to let layouting dictate the final height of a Buffer.

@jackpot51
Copy link
Member

@hecrj could you provide some guidance on a desired API?

@nicoburns
Copy link
Contributor

nicoburns commented Sep 7, 2023

@jackpot51 I'm not Hector, but perhaps I can help here, as I submitted #42 which I believe is more or less a duplicate of this issue (except it requests that the Buffer be able to figure out both dimensions, not just height).

(you may also wish to read the comments on that issue, as there is more discussion than here).

Background / Motivation

I think the key thing to understand here is that UI layout algorithms may want to determine their layout based on the size of an unconstrained text layout, rather than giving the text box an explicit size ahead of time.

An obvious case is laying out a vertical page within a scroll view, where you want a paragraph of text to take up however much vertical space it needs: it doesn't matter how much space that is so long as you can know how much space that ends up being. I believe it is possible to get this information out of cosmic-text currently by manually iterating of layout lines, but it's not particularly nice or easy. It would be nice if that height was calculated during layout and stored in an easy to access property.

A less obvious, but currently more problematic case is if you want to leave the width unspecified. As documented in #42 (comment), you can kind of do this by setting the width to some large value, but this breaks alignment as alignment as the alignment will be based on that large size rather than the actual with of the text. It would be much better if that width could explicitly be set to "undefined" which would then cause alignment to use the computed width.

This could be done by making set_size on Buffer take Option<i32> rather than i32 (or perhaps Option<u32> as I don't think negative dimensions make sense).

An extra bonus for interop with CSS style layout.

(Regarding users of cosmic-text: Bevy and Floem are both using Taffy which implements this style of layout)

CSS-style layout modes actually have two separate "undefined" layout modes - "min-content" and "max-content". Specifically for horizontal text, these should be thought of as constraints on the width of the laid out text:

  • The max-content layout is the intuitive "undefined" layout where text doesn't wrap at all unless an explicit new line character is encountered. The max-content width is the width of the longest line in that layout.
  • The min-content layout is the layout where the text is wrapped at the "min-content width". The min-content width is the width of the largest unbreakable glyph run in the Buffer (where what is considered "unbreakable" depends on the specified Wrap mode).
  • In all cases explicit newline characters cause text to wrap onto a new line

These sizes are used as hard bounds between which the layout algorithm can decide the final size of the text-containing box.

Suggested API

Add the following struct:

struct SizeConstraint {
     MinContent,
     MaxContent,
     Definite(u32),
 }
  • Make Buffer have both input_width/input_height and output_width/output_height properties.
  • Make the input properties be of type SizeConstraint (see above). The output properties can be plain u32s.
  • If width is indefinite (MinContent or MaxContent) then text wraps according to rules above. This results in an ouput_width and an output_height (u32).
  • If width is Definite then text is wrapped to max(input_width, min-content width) (min-content width = "width of largest unbreakable glyph run"), which also becomes the output_width.
  • This output_height is max(input_height, computed_height) or just computed_height is the input height is indefinite.
  • Text is then aligned using:
    • if input_width=MinContent then min-content width
    • if input_width=MaxContent then max-content width
    • if input_width=Definite then max(input_width, min-content width) as the width.
  • The computed output dimensions should be easily accessible via a getter.

If you wanted to do it in a slightly simpler way, then you could have the input dimensions be Option<u32> instead of SizeConstraint (where Option::None would follow the max-content semantics above). Min-content support could always be added later if this approach was taken.

Extension: pass input dimensions to layout method(s)

Perhaps this is going out of scope a little, but it might be nice if the "input dimensions" weren't stored on the Buffer at all, but were passed to the method that performs the layout. This is because in many layout systems, a text buffer doesn't have a single set of "input dimensions" (perhaps better thought of as "size constraints"), but may need to be sized repeatedly under multiple constraints to determine the final layout. But this change is much less important and doesn't block use of cosmic-text.

@jackpot51
Copy link
Member

Thank you @nicoburns for the description, I will look into this.

@jackpot51 jackpot51 self-assigned this Sep 8, 2023
@Imberflur
Copy link
Contributor

I've seen cases in iced text layout that the proposed SizeConstraint doesn't cover (afaict):

  1. A finite maximum width is specified but the size of the box shrinks to the size of the text.
  2. The fill width (from iced::layout::Limits) will be used if it is greater than the measured text width but less than the max width. (Although I'm not familiar with whether any arrangement of widgets could produce this case)

@nicoburns
Copy link
Contributor

nicoburns commented Sep 8, 2023

I believe (1) would be partially covered by making the output dimension in each axis:

max(min(input size, max-content size), min-content size)

Then the UI layout can use the output width.

I'm not 100% sure how that would interact with alignment though. If you want to customise whether the alignment was based on the specified width vs. the measured width then I guess you would need some kind of extra input (either a flag or a second size).

@Imberflur
Copy link
Contributor

Imberflur commented Sep 8, 2023

I somewhat feel like needing to cover all these behaviors (with output size and alignment) is a motivation for having a separate measurement operation that doesn't interact with alignment at all. Thus, allowing the user to operate on the measured dimensions with any behavior desired to determine the final bounds that they pass to a function that does alignment.

Although this might be a bit out of scope. And I guess the user can already do this by just performing full layout twice, so this could just be considered a future optimization. Also, it may not easily fit in the current Buffer API.

@hecrj
Copy link
Contributor Author

hecrj commented Sep 9, 2023

@nicoburns described the use cases very well. I also like the design proposed.

iced does not have plans to support the min-content layout approach, at least for now; so that's not a priority for us.

In iced, we are currently rolling our own logic to measure a Buffer:

https://github.com/iced-rs/iced/blob/a3489e4af960388e9f73988b88df361022a654a4/wgpu/src/text.rs#L293-L301

This is what prompted me to create this issue. The sizes we initially use for the Buffer are, as @nicoburns described, maximum boundaries; but we want the Buffer to end up with minimum ones.

@DasLixou
Copy link

Bump

Is there any ongoing work here?

@maxall41
Copy link

Bump

1 similar comment
@ElhamAryanpur
Copy link

Bump

@ElhamAryanpur
Copy link

bump

@jackpot51
Copy link
Member

jackpot51 commented Jun 12, 2024

This should be fixed in #271, which allows a Buffer to be created without a defined width and size. Buffer::layout_runs can then be used to figure out the actual size. If there are follow-up changes to request, please make a new issue.

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

Successfully merging a pull request may close this issue.

7 participants