Blitz tracking PR #47
Conversation
Any ancestors of this commit are from upstream mozilla-central, with some filtering and renaming. Our patches and sync tooling start here. The sync tooling has all been squashed into this commit, based on: https://github.com/servo/stylo/commits/64731e10dc8ef87ef52aa2fb9f988c3b2530f3a7
Cargo accepts this manifest, but there are compile errors with: $ cargo build --features 'servo servo-layout-2013' $ cargo build --features 'servo servo-layout-2020' Co-authored-by: Martin Robinson <[email protected]>
There was a problem hiding this comment.
This is a nice cleanup. Is there a corresponding Servo change. It would be nice to have that. In addition, I think it might make more sense to contribute this directly to upstream stylo and then wait until the next sync, to avoid too much of a hassle when we update. We need to start doing that, I think.
| ${helpers.predefined_type( | ||
| "justify-content", | ||
| "JustifyContent", | ||
| "specified::JustifyContent(specified::ContentDistribution::normal())", | ||
| engines="gecko servo", | ||
| spec="https://drafts.csswg.org/css-align/#propdef-justify-content", | ||
| extra_prefixes="webkit", | ||
| animation_value_type="discrete", | ||
| servo_restyle_damage="reflow", | ||
| affects="layout", | ||
| )} |
There was a problem hiding this comment.
It's great to unite these, but we shouldn't lose the servo_pref setting here in the process.
There was a problem hiding this comment.
The problem with the pref is that this property applies both to Flexbox and CSS Grid, and there is currently no way to specify 2 prefs (I could add one - see description for a discussion of this). I suppose I could just keep the original pref too, but it would be ideal if you could enable Grid without Flexbox (although I don't have a practical use case for this)
There was a problem hiding this comment.
Servo will implement flex well before grid, so I would stick with the flexbox preference here.
There was a problem hiding this comment.
Sure. Blitz will just enable both, so that works for us. I can go through and update the flags to use the flexbox flag for the case where it is needed for both. Although it wouldn't be that hard to add support for multiple flags if that was desirable (just swap out the Option<&str> for &[&str] (or Option<&[&str]>)
| servo_restyle_damage="reflow", | ||
| affects="layout", | ||
| )} | ||
| impl_align_conversions!(crate::values::specified::align::AlignItems); |
There was a problem hiding this comment.
It was already present in the Gecko version, I just made it unconditional (this doesn't show up very well in the diff due to unnesting). It adds conversions to and from u8. I'm not entirely sure why that's useful, but it didn't seem particularly gecko-specific. I could make it conditional again if that's preferred.
| ${helpers.predefined_type( | ||
| "align-items", | ||
| "AlignItems", | ||
| "specified::AlignItems::normal()", | ||
| engines="gecko", | ||
| spec="https://drafts.csswg.org/css-align/#propdef-align-items", | ||
| extra_prefixes="webkit", | ||
| animation_value_type="discrete", | ||
| servo_restyle_damage="reflow", | ||
| affects="layout", | ||
| )} | ||
|
|
||
| #[cfg(feature = "gecko")] | ||
| impl_align_conversions!(crate::values::specified::align::AlignItems); | ||
|
|
||
| ${helpers.predefined_type( | ||
| "justify-items", | ||
| "JustifyItems", | ||
| "computed::JustifyItems::legacy()", | ||
| engines="gecko", | ||
| spec="https://drafts.csswg.org/css-align/#propdef-justify-items", | ||
| animation_value_type="discrete", | ||
| affects="layout", | ||
| )} | ||
|
|
||
| #[cfg(feature = "gecko")] | ||
| impl_align_conversions!(crate::values::specified::align::JustifyItems); |
There was a problem hiding this comment.
I think it's better not to adjust the position of these, as this makes the diff with upstream harder to process.
There was a problem hiding this comment.
I think I've put them back in the order they were in.
Co-authored-by: Bastien Orivel <[email protected]>
Co-authored-by: Josh Matthews <[email protected]>
* MallocSizeOf for Index{Set, Map}
* like as iterable in WebIDL
* Codegen magic for like interfaces
* TestBinding for like
* Test for Setlike and Maplike test bindings
* Some fixes
* Switch to any.js
* nit
* Keep order
* script: Do not run layout in a thread Instead of spawning a thread for layout that almost always runs synchronously with script, simply run layout in the script thread. This is a resurrection of #28708, taking just the bits that remove the layout thread. It's a complex change and thus is just a first step toward cleaning up the interface between script and layout. Messages are still passed from script to layout via a `process()` method and script proxies some messages to layout from other threads as well. Big changes: 1. Layout is created in the script thread on Document load, thus every live document is guaranteed to have a layout. This isn't completely hidden in the interface, but we can safely `unwrap()` on a Document's layout. 2. Layout configuration is abstracted away into a LayoutConfig struct and the LayoutFactory is a struct passed around by the Constellation. This is to avoid having to monomorphize the entire script thread for each layout. 3. Instead of having the Constellation block on the layout thread to figure out the current epoch and whether there are pending web fonts loading, updates are sent synchronously to the Constellation when rendering to a screenshot. This practically only used by the WPT. A couple tests start to fail, which is probably inevitable since removing the layout thread has introduced timing changes in "exit after load" and screenshot behavior. Co-authored-by: Josh Matthews <[email protected]> * Update test expectations * Fix some issues found during review * Clarify some comments * Address review comments --------- Co-authored-by: Josh Matthews <[email protected]>
This builds successfully with: $ cargo build --features servo This patch is up to date as of: * servo#31351 * servo#31363 * servo#31358 * servo#31365 * servo#31387 * servo#31408
* Fix target path for css-properties.json * Move css-properties.json to target/doc/stylo Co-authored-by: Martin Robinson <[email protected]> --------- Co-authored-by: Martin Robinson <[email protected]>
nicoburns
force-pushed
the
blitz
branch
2 times, most recently
from
85044b3 to
0af5ac8
Compare
| @@ -808,11 +815,37 @@ | |||
| Ok(()) | |||
| } | |||
| } | |||
|
|
|||
| #[cfg(feature = "servo")] | |||
There was a problem hiding this comment.
We need a different impl here because we're feature flagging these properties and gecko isn't.
nicoburns
force-pushed
the
blitz
branch
6 times, most recently
from
fe00617 to
279baa2
Compare
In order to do this, an interface is exposed so that the embedder can set a default style for a device. Signed-off-by: Martin Robinson <[email protected]> Co-authored-by: Mukilan Thiyagarajan <[email protected]>
…s' properties. r=emilio These properties were added as part of the experimental Masonry support added in bug 1607954. Since then the CSS WG resolved to remove these properties in: w3c/csswg-drafts#9529 This patch is a fairly brain dead removal of the properties, simply changing the consumer code to take the code paths that would have been taken previously if the properties were not set. That leaves some obvious dead code, which has been removed, but no attempt has been made to redesign the Masonry code to "make sense" without these properties. That would require a more prolonged effort to understand Masonry, how the spec has changed in the last four years, and how we should best change our code. For now, this removal is simply focused on reducing the amount of memory used by nsStyleDisplay to unblock the landing of bug 1899949. Differential Revision: https://phabricator.services.mozilla.com/D212358
|
I've cherry-picked ddb7af7127e6c6c64e6202fab00d42aa4b33a5c3 from upstream, which along with #51 makes the diff (both against |
|
Thanks for pushing this work upstream! We have isolated the changes affecting the align properties into #53. |
|
All changes from this PR have now been upstreamed and synced back to this repo :) Closing this. If we need further changes in future I can open a new PR. |
Putting this up to track the Stylo changes we have made for Blitz and that we would like to see upstreamed into Servo's version in this repo. Servo will also want these in it's own right as it comes to implement a full implementation of the CSS Grid,
gap, and alignment styles.Not necessarily expecting this to merged as-is(UPDATE: given improvements to the feature flagging, I feel like this probably could be merged / upstreamed now).Corresponding Servo PR: servo/servo#32686
Upstream submissions
Changes made
row-gapand the shorthandgap(behind flexbox flag)column-gapto being behind the flexbox flag rather than the multi-column layout flagNotes
I think the "use gecko alignment implementation" changes probably could be upstreamed as-is (but this will require a corresponding servo PR)
The grid-only styles are also quite straightforward because they're behind a flag.
The case where it's more tricky is the styles that are shared by Flexbox and Grid because Stylo currently only allows styles to be gated behind a single pref. There are a few options here:
I'd probably favour (ii).
Currently based on: 2024-05-31