style/gfx/metrics/script crates serialize build

[jdm]

script depends on metrics, and metrics depends on gfx, and gfx depends on style (script also depends on style). If we move the bits of style that gfx depends upon into style_traits, and possibly the bits of gfx that metrics depends upon into gfx_traits, this should allow more crates to build in parallel.

[tigercosmos]

The one need to move it one by one or finish it in one time?

[jdm]

Moving one by one shouldn't hurt anything.

[tigercosmos]

OK! I will work on this.

[tigercosmos]

@jdm if some codes in style that gfx depends is also used by other crate(i.g. layout), shall I move it? Or just move codes only depended by gfx?

[tigercosmos]

ummm... I think they should all be moved, otherwise gfx still need to wait for style

[jdm]

Exactly. If it's moved from gfx to gfx_traits, the other crates that depend on the move items should be updated to use gfx_traits.

[tigercosmos]

This issue is much harder than I though before. Actually almost everything depend on each other.

For example, If I want to move gfx::display_list, which is used by metrics and also the only one gfx dependency in metrics, since gfx::display_list use gfx::text, I will also need to move gfx::text, and gfx::text use lot of gfx things, which mean if I want to move display_list, I will move almost everything in gfx.

So, in this case, we can make sure moving more something from gfx to gfx_traits is impossible.
In fact, I found style has the same problem. :(

[jdm]

Try creating a trait that can replace https://dxr.mozilla.org/servo/source/components/metrics/lib.rs#306-315. That trait can live in gfx_traits, and the implementation in gfx.

[tigercosmos]

I don't understand. If we move that code to gfx_traits.
The relationship would be metrics -> gfx_traits -> gfx.
I thought crate_traits will be independent with crate. Do I misunderstand?

[jdm]

No, because metrics would depend on gfx_traits, and gfx would depend on gfx_traits.

[tigercosmos]

That code use display_list. But display_list is under gfx? If we move that code to traits, it still need to call gfx?

[jdm]

The code in metrics is calculating a boolean value using the display list types from gfx. The types do not need to move - we can create a trait in gfx_traits that encapsulates the calculation in a method, and implement the trait on the types in gfx. The code in metrics only needs to refer to the trait.

[KiChjang]

@tigercosmos Is this still being worked on?

[tigercosmos]

@KiChjang NO. I can't handle this. I didn't understand how to do.
If someone is interested in this, feel free to take it. :)

[streichgeorg]

I'd like to take a shot at this then.

[jdm]

Please do!

[streichgeorg]

I opened a pull request for the gfx-metrics part. But style-gfx seems a lot harder because there is non zero amount of metaprogramming in the style crate and I would be really happy about some pointers what's going on there. gfx depends mainly on font_weight and some other font related structs, but I don't really get where they get declared.

[jdm]

The various mako.rs files under style/properties/ are Python Mako templates that generate Rust code. font_weight is defined in style/properties/longhand/font.mako.rs, and it invokes Python code from style/properties/helpers.mako.rs. However, it may be possible to modify the imports from gfx to directly import style::values::computed::font::FontWeight, for example. If that's the case, then it should be possible to extract those types into style_traits and use pub use style_traits::whatever::FontWeight; where they were originally defined.

[streichgeorg]

I'm working on removing the style dependency from gfx, but am currently stuck fighting the rust compiler. The problem is that style::computed_values::font_weight::T has some static methods that gfx needs. But when I create a trait that has static methods it's no longer object safe and cannot be passed around anymore, not even in a Box, as I understand it. Is there a nice solution to this.

[jdm]

@streichgeorg Could you push your code to a branch so it's easier to understand the problems that you're facing?

[streichgeorg]

Sure

[streichgeorg]

The name of the branch is decoupling_style_gfx. I started by moving just font_weight::T.

[jdm]

Here is what I propose:

• move the definition of FontWeight and its methods into style_traits
• add a pub use style_traits::FontWeight; line in components/style/values/computed/font.rs

There should be no need to mess with traits for that change; the code in gfx can use the type defined in style_traits directly, and style will continue to use the same type implicitly by reexporting it with the same name in its old location.

[streichgeorg]

Oh I see, style_traits isn't really all about traits.
Thank you for the help.

[SimonSapin]

(Renaming those *_traits crates might be another easy issue, once we decide what to name them instead :))

[KiChjang]

Reopening since this seems to not have been solved yet.

[atouchet]

#19934 was an unfinished PR to address this.

[SimonSapin]

the bits of gfx that metrics depends upon into gfx_traits

This was done at some point, meaning that the only intermediate dependency between style and script is now script_layout_interface, which only takes ~6 seconds to compile in release mode on our macOS CI (the slowest):

https://community-tc.services.mozilla.com/tasks/BCyqIQNdTNmcrj7kFW58Ww/runs/0/logs/https%3A%2F%2Fcommunity-tc.services.mozilla.com%2Fapi%2Fqueue%2Fv1%2Ftask%2FBCyqIQNdTNmcrj7kFW58Ww%2Fruns%2F0%2Fartifacts%2Fpublic%2Flogs%2Flive.log#L2006

The above is a screenshot of xdot with the mouse hovering style in order to color the edges connected to it. The graph is of script and its recursive path dependencies, generated with the script at #26241.

This feels good enough for me, so closing.