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

Add stylo_taffy crate #88

Closed
wants to merge 1 commit into from
Closed

Conversation

nicoburns
Copy link
Contributor

This PR contains a new crate stylo_taffy which contains:

  • Type conversions between stylo types and taffy types
  • A function to convert a stylo::ComputeValues into a taffy::Style
  • A wrapper type TaffyStyloStyle<T> that wraps any T: Deref<Target = ComputedValue> and which implements Taffy's style traits, which allows it to be passed to Taffy's algorithms directly instead of Taffy's Style struct.

I believe this repository is the best place for this code to live as:

  • It allows Servo, Blitz, and anybody else who wants to combine Stylo with Taffy to share this code
  • It avoids any complications with having MIT/APACHE2.0 licensed code in the main Servo repo
  • It means that any required fixups can happen directly as part of a Stylo sync, and there is not the additional step of having to update code in the Taffy repo first.

I can submit this upstream if there is agreement that this is the best place for this code.

@mrobinson
Copy link
Member

I'm not sure this the best place to put this code. One of the big goals here is to reduce the diff with the upstream version of stylo.

@nicoburns
Copy link
Contributor Author

I'm not sure this the best place to put this code. One of the big goals here is to reduce the diff with the upstream version of stylo.

Well I was imagining this would go upstream. That would of course be dependent on upstream accepting it.

Other options include:

  • This crate exists in this form in the servo/servo repo. That would be mostly fine (although it would be kinda annoying to deal with having to run Servo's entire CI every time we want to make a change).
  • This crate exists in this form in the Taffy repo. This would allow for code sharing, but would make Stylo syncs annoying, with an additional repo needing to updated.
  • We include copies of this code in each repo it is used in, and copy-paste sync changes. This wouldn't be a huge deal from a code POV, but would either require Servo to include an MIT/APACHE2.0 files or Blitz to include an MPL ones (we could also triple-license but that would still require clear communication of the license which applies to the files).
  • We include copies of this code in each repo with separate licenses, and accept that such code will diverge and that changes can't be copied between them.

@mrobinson
Copy link
Member

This crate exists in this form in the servo/servo repo. That would be mostly fine (although it would be kinda annoying to deal with having to run Servo's entire CI every time we want to make a change).

Let's just do this. This is where the conversion is done, so it makes sense that the code lives there so that dead code analysis works properly. Note that we have this kind of conversion code other places in Servo as well.

@nicoburns
Copy link
Contributor Author

Ok, this code is now included as part of servo/servo#32619

@nicoburns nicoburns closed this Nov 2, 2024
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 this pull request may close these issues.

2 participants