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

proposal for custom layout solvers #648

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Niedzwiedzw
Copy link

DRAFT

Objective

There's been some talk about providing a way to define your own layouts, this would enable this

@nicoburns
Copy link
Collaborator

I'm definitely open to a feature like this, however:

  • Why have you introduced so many Boxs? I think it should be possible to implement this without boxing.
  • I think we'll want Display::Custom to work like measure functions and be a single function passed to a compute_layout_* method, for the same reason that measure functions work like this: it allows them to borrow external context. I think the main difference between this and measure functions is that custom layout would need to be able to size and position children.
  • We should think about whether we need both measure functions and custom layouts or whether this should replace measure functions.
  • Not sure what I think about the "name" parameter. It seems fine, but it may be better to replace it with an ID or just rely on the node context.

@Niedzwiedzw
Copy link
Author

Niedzwiedzw commented Apr 19, 2024

Why have you introduced so many Boxs? I think it should be possible to implement this without boxing.

yeah thing is this is the only way to make that trait a trait object and I didn't want to make Display generic over Tree: LayoutPartialTree cause that way Style would also need to be generic over that etc...

@nicoburns
Copy link
Collaborator

Why have you introduced so many Boxs? I think it should be possible to implement this without boxing.

yeah thing is this is the only way to make that trait a trait object and I didn't want to make Display generic over Tree: LayoutPartialTree cause that way Style would also need to be generic over that etc...

Ah, I see. I think this can be solved by making the custom layout function something that is passed in to the call to layout() similar to layout_with_measure. This means that the type only needs to exist in the function signature (and the TaffyView struct) and can be implemented as a generic (and as a bonus allows that function to (mutably) borrow external data if it wants to). Measure functions used to be stored within the Taffy struct similarly to your implementation here but this was changed for these same reasons.

@alice-i-cecile alice-i-cecile added enhancement New feature or request controversial This work requires a heightened standard of review due to implementation or design complexity labels May 30, 2024
@Niedzwiedzw
Copy link
Author

@nicoburns are there any news on this? I'd happily modify this to conform to the API you describe, but question is would it go upstream :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controversial This work requires a heightened standard of review due to implementation or design complexity enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants