mozilla :: #servo

20 May 2017
00:00emilionox: r=me on this one
00:00noxThanks. :)
00:01noxI'll try to come up with more functions in syn for improving the where clauses.
00:02noxMy code potentially add duplicate where clauses, it will compile but well, that feels ugly to me.
00:02noxIt could also skip adding clauses for field types that are not parameterised.
00:04noxemilio: Just realised we don't need specialisation to make some kind of ViewportIndependent trait.
00:05noxemilio: So for example ImageRect would derive that marker trait,
00:05emilionox: what do you mean?
00:05noxemilio: and that would implement HVP with a literal false return only if it's NumberOrPercentage is ViewportIndependent too.
00:06emilionox: Oh, I see
00:06noxemilio: Make a trait similar as ToComputedValueAsSpecified but for HVP.
00:06noxThat way we can be a bit more sure that rustc does the right thing.
00:06emilionox: didn't we have something like that before? I'm pretty sure we did
00:06emilionox: and then have to switch to the macro thing
00:06noxemilio: We have a macro, but it doesn't handle parameters.
00:06emilio*had
00:06noxemilio: And it will not check that the param really had no viewport percentages.
00:07emilionox: yeah, but i'm pretty sure before the macro we had NoViewportPercentage
00:07noxThe ViewportIndependent bound would.
00:07noxemilio: Unfortunately without specialisation we can&#39;t derive ViewportIndependent for Vec<T>.
00:07emilionox: I can&#39;t recall when or why it was removed, I&#39;ll try to look up in the git log
00:07noxhttps://github.com/servo/servo/commit/abc40f61c0dc1703028a744b42c0a9b5b45339fc
00:07noxemilio: For once that GH introduced a useful feature. :P
00:08noxemilio: https://github.com/servo/servo/pull/15411#issuecomment-278374576
00:08crowbotPR #15411: Box larger specified values to avoid memmove impact - https://github.com/servo/servo/pull/15411
00:08noxemilio: Was removed to remove some code repetition.
00:08noxemilio: Which derive macros avoid. :)
00:09emilionox: yup! I see, fine for me reintroducing it
00:09jryansemilio: when computing restyle hints, do we see all element state changes, or only those we already know that some selector is sensitive to?
00:10emiliojryans: heycam added an optimization where we filtered by selectors, so state changes known non-sensitive won&#39;t appear AFAIK
00:10emiliojryans: See Servo_StyleSet_HasStateDependency
00:11emiliojryans: note that there&#39;s already an special-case for :dir, but we shouldn&#39;t need that for :link/:visited i think
00:12noxemilio: Oh you know what&#39;s going to be fun to derive too?
00:12noxAnimatable.
00:12emilionox: the interpolation stuff?
00:12noxYes.
00:14jryansemilio: ah, i see... looks like only the servo side landed so far https://bugzilla.mozilla.org/show_bug.cgi?id=1352306
00:14firebotBug 1352306 NEW, cam@mcc.id.au stylo: track the attributes and state that a DependencySet is sensitive to, and use it to cull snaps
00:16emilionox: sure, do you want me to try do that? I&#39;m happy helping you to kill code :)
00:16noxemilio: compute_distance and its friend complicate deriving it, but splitting the trait in two shouldn&#39;t be too hard.
00:16jryansemilio: i was asking because i thought we might want to keep the visited state check inside the dependency loop, so that we make sure there&#39;s actually a selector that uses that state first
00:16noxemilio: There is ToCss, ToComputedValue and Animatable.
00:16emiliojryans: there&#39;s always a selector in UA sheets for :visited
00:17noxemilio: ToCss, the challenge is to have attributes to tweak stuff, for example whether variants transparently just calls to_css on their contents, or they print some keyword first, space vs comma separated, etc.
00:17noxemilio: ToComputedValue, the challenge is that there is an associated type.
00:17emilionox: oh, also, something that would be servo-only, but would be nice to have, is some sort of compute_difference
00:17jryansemilio: mm... alright, fair enough :)
00:17noxemilio: Animatable, I just said it.
00:17noxemilio: Choose your poison. :P
00:17noxemilio: What would compute_difference do?
00:18emilionox: kill all this code: https://github.com/servo/servo/blob/master/components/style/servo/restyle_damage.rs#L190
00:18noxemilio: &quot;166,970 ++ / 164,382 --&quot;
00:18emilionox: which I&#39;m pretty sure is so buggy
00:18noxMy changes are positive only because of stylo.
00:19emilionox: yeah, know you don&#39;t want to keep it that way ;)
00:19noxemilio: That looks lovely.
00:19noxI passed in front of this code once,
00:19noxI concluded that for my happiness I should ignore I&#39;ve ever seen it.
00:20noxemilio: How does compute_difference relate to that code?
00:20emilionox: yeah, that &quot;// FIXME: Test somehow that every property is included.&quot; is lovely
00:20noxOh. :/
00:20noxemilio: That reminds me,
00:20noxemilio: if you look at the font longhand,
00:20emilionox: basically, you want to have a trait for each CSS property outputs a &quot;RestyleDamage&quot;
00:20noxspecifically the system font stuff,
00:21emilionox: and derive that in the style structs to combine it
00:21noxComputedSystemFont seems to be meant to include all longhands included in the font shorthands,
00:21noxbut I think it misses one.
00:21noxemilio: Oh. What is RestyleDamage? Just some bitflags! no?
00:22emilionox: yeah
00:22noxBut why &quot;compute_difference&quot;?
00:22emilionox: the key is adding damage only if fields change
00:23emilionox: so I was thinking of something like: trait StyleDifference { fn compute_difference(&self, other: &Self) -> RestyleDamage; }
00:23noxI see.
00:23noxI was going to bikeshed about naming again.
00:24noxI feel old that I managed to resist it.
00:24emilioheh
00:24emilionox: which one is it missing?
00:24noxemilio: Not sure derive is the best for the individual properties.
00:25emilionox: yeah, that&#39;s fair
00:25noxemilio: Or rather,
00:25emilionox: in any case, we should ensure every computed_value::T implements that
00:25emilionox: but that can probably wait a bit
00:25noxemilio: maybe that trait should be implemented for all types that are PartialEq and that implement HasRestyleDamage or a trait like that,
00:26noxwith a nullary static method that just returns the restyle damage that would be returned if there are indeed differences.
00:27noxemilio: font-size-adjust and line-height.
00:27noxemilio: But I just remembered that font-size-adjust is special,
00:27emilionox: yeah, it&#39;s somewhat tricky because the damage may depend on the differences, it&#39;s not an all-or-nothing
00:27noxand line-height probably too.
00:27noxemilio: Oh I see, ok.
00:28noxemilio: Not with add_if_not_equal though, right?
00:28noxemilio: Is all of this code that I am trying to derive all handled with fancy templates in Gecko?
00:28noxOr written by hand?
00:29emilionox: guess :)
00:29emilionox: (tl;dr), it&#39;s written by hand
00:30mystornox: neat!
00:30noxemilio: Hah.
00:30emilionox: and macros, lots of macros :-)
00:30noxmystor: Uses the FnMut, bindings, and the variant. :)
00:30mystornox: I&#39;m glad I could make synstructure more useful
00:31noxI&#39;m glad you wrote it hah!
00:31noxI&#39;m not very good at convenience user-facing APIs.
00:35mystornox: I&#39;m not sure I&#39;m good at it either, but I was writing custom derives and found that pretty much all of them needed some variant of the same theme so I wrote synstructure
00:35mystor:P
00:36noxmystor++
00:43heycamemilio: jryans: that bug still hasn&#39;t landed yet (filtering based on whether state pseudo classes appear in style sheets)
00:43noxemilio: Oh and I forgot... I also want to derive some Parse impls.
00:43jryansheycam: yep, just noticed that... no worries :)
00:43mystornox: Btw, you could have done this with each_field
00:44noxmystor: With a mut blabla yeah, except for the Fn part.
00:44mystor(not that this isn&#39;t a good way to do it)
00:44noxmystor: Err, in my mind that implied &quot;with 0.5.0&quot;, disregard the Fn part.
00:44noxmystor: Yeah for my personal tastes I wanted to avoid the mut variable. :D
00:45mystornox: Nah, I would&#39;ve said with early returns
00:45mystorhttps://is.gd/PIiqCO <- something like this
00:45mystor(note: untested)
00:45noxOh yeah that works too.
00:46* mystor just saw what you wrote and realized that perhaps a fold_field might be good
00:46mystoror reduce field
00:46mystoriunno
00:47noxmystor: I could have just folded over the bindings vec.
00:47emilioheycam: how likely are you to land your restyle patches soon-ish?
00:48mystorsynstructure::fold_field(&input, &style, quote!(false), |cur, val| {
00:48mystor quote!(#cur || ::style_traits::HasViewportPercentage::has_viewport_percentage(#val))
00:48emilioheycam: I wanted to move all the style sharing code out of matching.rs
00:48mystor})
00:48mystorbut yeah, you could&#39;ve *shrug*
00:48emilioheycam: but that&#39;d bitrot your patches :/
00:49mystornox: I think the next step for synstructure or perhaps a separate crate (though you&#39;ll usually want to use them together) is to replace the where_clause stuff
00:50noxmystor: I&#39;ll add some convenience methods in syn to create the various things more easily, for a first step.
00:51mystornox: IMO you should be able to just write this: https://is.gd/fmav7l
00:51mystornox: And have it DTRT
00:51noxmystor: Btw to derive ToComputedValue, which returns an associated type, I&#39;ll have to use https://docs.rs/synstructure/0.5.2/synstructure/fn.match_pattern.html from the closure passed to each_variant, to produce the returned value.
00:51noxMaybe this function could have a more general name.
00:51mystornox: 1 sec, looking at the trait
00:52noxmystor: DTRT?
00:52mystornox: Do The Right Thing
00:52noxmystor: https://github.com/servo/servo/blob/derive-all-the-things/components/style/values/generics/position.rs#L30
00:52mystornox: So that function at the end would generate the impl block with the right generics and the right trait type and such
00:52noxmystor: It can&#39;t do the right thing always, e.g. with ToComputedValue the where clauses are more complex.
00:53mystornox: Of course, but for simple traits like this one you shouldn&#39;t have to write complex code
00:53noxmystor: But yeah something should help for the whole trait impl of simpler traits.
00:53mystornox: The point of synstructure is to make the easy case easy
00:53mystorand focus on the hard bits
00:53noxmystor: JSTraceable is an example of a simple trait but it is unsafe, so it would need a knob for that too, or maybe we just don&#39;t care.
00:54mystornox: I&#39;d probably add a bool for unsafeness
00:54noxmystor: Yeah but it&#39;s often painful to handle &quot;easy case + one single detail&quot;.
00:54mystornox: I&#39;d want to use that function for gc_derive too (for rust-gc) which means that I need it anyway
00:54mystornox: So with ToComputedValue - what is the tricksy bit you need?
00:55noxmystor: where SomeFieldType<T>: TCV<CV=SomeFieldType<<T as TCV>::CV>>
00:56heycamemilio: I keep trying to but I keep having to rebase over conflicts :)
00:56emilioheycam: d&#39;oh :(
00:56heycamemilio: I will try to land them this morning
00:56emilioheycam: well, my code move can wait :)
00:56ajeffreypaul: cbrewster: https://github.com/asajeffrey/servo/commit/9864cbf9cca105a349657e9b0734fdc32fb3dce7#diff-55c92a6a5ba7654ce45fe6fc6c63740fL254
00:56ajeffreyGot rid of the root browsing context.
00:57ajeffreyTurns out most of the pain was in webdriver, sigh.
00:57mystornox: That&#39;s for type parameters though, right? I don&#39;t think synstructure really helps with that
00:57noxmystor: Yeah. That&#39;s why the derive for HVP is a bit verbose too.
00:58noxWell, just syn helpers would help;
00:58heycamemilio: (well really I&#39;m only just trying to land them today, but I did have a few rebase pains during the week!)
00:58noxs/;/./
00:58ajeffreythe webdriver spec is all written in terms of browsing contexts, our implementation was using pipelines instead.
00:58emilioheycam: yeah, I&#39;m partially responsible for that, I bet splitting cascade_primary_or_pseudo didn&#39;t help :(
00:59cbrewsterajeffrey: nice!
00:59emilioheycam: I wanted to get that done though, because that affects boris&#39;s OMTA work, which was going to start passing stuff to that function that only applied to the primary style
00:59heycamemilio: otoh I think matching.rs is nice and neat now
00:59ajeffreyjgraham: https://github.com/asajeffrey/servo/commit/9864cbf9cca105a349657e9b0734fdc32fb3dce7#diff-a1d91ab451658c0134bb24f8f406dccbR35
00:59ajeffreycbrewster: thanks!
00:59emilioheycam: It&#39;ll be nicer when I get rid of all the sharing code ;)
00:59mystortrue
00:59emilioheycam: but yeah, I also felt somewhat uneasy about the complexity growth in that file :)
01:00noxReminds me,
01:00noxwhat the hell does OMTA stand for?
01:01froydnj|awaynox: off main thread animation
01:01noxOh, thanks.
01:10heycamhomu&#39;s queue is looking the longest I&#39;ve seen in a while!
01:17emilioheycam: ^ shouldn&#39;t affect you I think, if you could review it, that&#39;d be neat :)
01:21heycamemilio: looks good, r=me
01:22emilioheycam: thanks!
02:07emilioheycam: sorry for the mess I (and mozreview, d&#39;oh) made of bug 1364871 and bug 1366144 btw :(
02:07firebothttps://bugzil.la/1364871 NEW, emilio+bugs stylo: Restyle ::-moz-list-bullet and ::-moz-list-number pseudo-elements (for nsBulletFrame)
02:07firebothttps://bugzil.la/1366144 NEW, emilio+bugs stylo: ::after pseudo element under a parent with transition didn&#39;t show up
02:15heycamemilio: heh np, will try to get a chance to look at them later this weekend
02:15emilioheycam: cool, thanks!
02:31wafflesemilio, we can get the failing tests from the log of any stylo build, right?
02:31waffles(I mean, the mochitests and reftests)
02:32emiliowaffles: yes, why not?
02:33wafflesemilio, no, just asking, planning to file easy bugs for some parse/css impls
02:34waffles(especially for grid)
04:15cpetersonStylo question: I am trying to build Firefox with Stylo on Mac. My mozconfig has --enable-stylo and a correct LLVM_CONFIG, but bindgen fails because the clang std headers installed by mach bootstrap can&#39;t find the real std headers: /Users/chris/.mozbuild/clang/include/c++/v1/stdlib.h:94:15: fatal error: &#39;stdlib.h&#39; file not found
04:16cpetersonis my mozconfig missing some other include paths?
04:55* stshine headache
06:14jryanscpeterson: try xcode-select --install
06:19Manishearthcould someone land https://github.com/servo/app_units/pull/26#issuecomment-301931183 on autoland ?
06:19crowbotPR #26: Clamp Au values between 1<<30 and -(1<<30) and saturate all operations - https://github.com/servo/app_units/pull/26
06:19Manishearthau update just landed
06:19cpetersonjryans: thanks. I&#39;ll try that. what Stylo dependency does xcode-select install that is not needed for building Firefox without Stylo?
06:19Manishearthcc jryans since you seem to be around
06:24hiroManishearth: I can land it.
06:24Manishearththanks hiro
06:37semarieis it possible to release a new version of rust-bindgen (with clang-sys:0.18.0 update) [if it is the right channel to ask that] ? the purpose is to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1365488 which requires servo to have clang-sys:0.18.0
06:37firebotBug 1365488 NEW, nobody@mozilla.org stylo doesn&#39;t build on OpenBSD
06:38semarieafter, it will also mean updating servo to use the just released rust-bindgen :)
06:38cpetersonsemarie: trying pinging fitzgen re rust-bindgen
07:17xidorninteresting that #16914 doesn&#39;t trigger an auto revendor
07:17crowbotPR #16914: Update app_units to 0.4.1 - https://github.com/servo/servo/pull/16914
07:19xidornglob: do you know why there&#39;s no auto revendor for that?
08:56emilior? anyone
09:57emiliocrowbot: is border-collapse intermittent?
09:57crowbotNo intermittent issues filed with &#39;border-collapse&#39; in the title
09:57emiliocrowbot: is border intermittent?
09:57crowbot#16685 - Intermittent timeout in /css21_dev/html4/border-conflict-element-001e.htm (https://github.com/servo/servo/issues/16685)
09:57crowbot#14377 - Intermittent timeout in /_mozilla/css/border_radius_shorthand_a.html (https://github.com/servo/servo/issues/14377)
09:57crowbot#14229 - Intermittent timeout in /_mozilla/css/border_radius_elliptical_a.html (https://github.com/servo/servo/issues/14229)
09:57crowbot#11663 - Intermittent crash in /css21_dev/html4/border-conflict-element-001d.htm (https://github.com/servo/servo/issues/11663)
10:12emilioboris: oh, you&#39;re faster than me :). Sorry for commenting in the last moment, I don&#39;t remember to have reviewed that From<MatchingResults> impl
10:13borisemilio, I am trying to update that patch. Thanks for catching it!
10:15emilioboris: I think the pseudo-element-important-rules thing may be as easy as calling important_rules_changed in that path too. For the rest, I think passing the boolean down the function may be ok for now instead (given we can&#39;t compute RulesChanged accurately without way more effort)
10:17borisemilio, ok. I will pass the boolean down to other functions.
10:17emilioboris: sorry for the back-and-forth there, I thought we could come up with an accurate RulesChanged from everywhere, but apparently that&#39;s not the case :)
10:19borisemilio, it&#39;s ok.
10:20emilioboris: (I&#39;m super-excited to see the OMTA stuff land btw, it&#39;s awesome \o/)
10:22borisemilio, actually, I&#39;m quite sure the pseudo part. so what is &quot;pseudo-element-important-rules thing&quot; means?
10:22boris*not quite sure
10:23borisemilio. Oh, looks like I also need to get the important rules for it
10:23borisI see.
10:23boristhanks
10:24emilioboris: right, that&#39;s it. np :)
10:24emilioboris: the implemented_pseudo_element stuff is relatively recent, but there&#39;s where we handle animations for ::before and ::after, so presumably they also need that bit checked
10:26borisemilio: I see. Thanks a lot. Will send review request later in that pr.
10:26emilioboris: no problem, thanks! :)
10:32emilioboris: oh, btw, while I have you here, what are the semantics of the may_have_animations flag?
10:32emilioboris: in particular, can I use them to early-return from get_animation_rules without two FFI calls?
10:33borisemilio: I think the answer is yes. we use may_have_animations() as the pre-check in has_animations()
10:33emilioboris: i.e., is this patch correct?
10:33emiliohttps://www.irccloud.com/pastebin/w34GcYdQ/animations.patch
10:34emilioboris: oh, I see, that&#39;d be awesome, I&#39;ll open a PR with it then.
10:35borisyes
10:35borisin Gecko, http://searchfox.org/mozilla-central/rev/02cae69058d41e2c6b635edccbec91a6ca2cb57f/dom/animation/EffectSet.cpp#88
10:35borisemilio, we set this flag if it has an effectSet (i.e. may have animations)
10:36borisso i think we can use it to avoid calling two FFI
10:38emilioboris: nice :). r? ^ then?
10:49travis-ciServo failed to build with Rust nightly: https://travis-ci.org/servo/servo-with-rust-nightly/builds/234282305 CC nox, SimonSapin
11:42noxemilio: 525 unused warnings, what could go wrong...
11:43emilionox: you managed to kill another trait? Or is it about the nightly build?
11:43noxemilio: #![allow(unused_warnings)] in helpers.mako.rs,
11:43emilionox: d&#39;oh
11:43noxemilio: instead of allowing each unused import made in helpers.mako.rs,
11:44noxemilio: so unused warnings from the other .mako.rs files don&#39;t warn,
11:44noxand never get removed when actually unused and we can do something about it.
11:46emilioyeah, I looked yesterday into uncommenting https://github.com/servo/servo/blob/master/components/style/lib.rs#L35, wasn&#39;t fun
11:46emilionox: I might try to do it today
11:47noxemilio: I filed a bunch of I-safety issues for stylo btw.
12:21noxemilio: https://github.com/servo/servo/blob/master/components/style/properties/longhand/font.mako.rs#L472-L478 :/
12:22emilionox: funtimes.png
12:24noxemilio: &quot;LLVM ERROR: Invalid data was encountered while parsing the file&quot;
12:25noxFUN. TIMES.
12:25est31lol
12:26noxest31: HL on &quot;LLVM ERROR&quot;? :P
12:34noxemilio: +44/-77 fixing the imports.
12:34noxemilio: May just add a commit to my HVP branch.
12:41noxemilio: There was an impl of HVP in the middle of nowhere in geckolib for f32,
12:41noxemilio: so compilation would have failed anyway, fixing it.
12:41emilionox: lol
12:42noxemilio: So I&#39;ll add that import removing commit.
12:42emilionox: sure, I can take a look and re-stamp it if you want
12:42noxYeah, soon.
12:55emilioCJKu: ping?
13:01noxThe year is 2045, people are still not able to write short commit messages...
13:01noxtitles*
13:03noxemilio: Why the two p=10 btw? Is there a bustage?
13:03noxWould be nice to land non-stylo things during the weekend, for our volunteers.
13:03emilionox: ouch, feel free to p=1, or p=0 as needed. I just have patches on top of patches on top of patches :(
13:03noxemilio: No no probs.
13:04noxemilio: Just checking that you didn&#39;t do that in a Pavlovian way just because they are Stylo PRs. :P
13:04emilionox: heh
13:07noxemilio: Lol, +53/-119
13:08noxwith just import changes.
13:12emiliolol
13:13emilioheycam|away: I&#39;ll look into merging your changes with the ones of bug 1366144 and debug that failure.
13:13firebothttps://bugzil.la/1366144 NEW, emilio+bugs@crisal.io stylo: ::after pseudo element under a parent with transition didn&#39;t show up
13:24noxemilio: r? https://github.com/servo/servo/pull/16960
13:24crowbotPR #16960: Derive HasViewportPercentage - https://github.com/servo/servo/pull/16960
13:25nox 40 files changed, 258 insertions(+), 574 deletions(-)
13:25noxNEED MOAR DELETIONS
13:49noxemilio: Gone?
13:49emilionox: I went for lunch, but I r+&#39;d it
13:50noxOh never mind missed it.
13:50noxThanks!
13:50noxI hope it won&#39;t get bitrot.
13:50emilionox: yeah, feel free to prioritize it as needed IMO
14:26emiliomach cargo check is so lovely for rebases <3
14:33emilionox: btw, could you stamp https://github.com/servo/rust-bindgen/pull/709?
14:33crowbotPR #709: Minor version bump to peek up clang-sys updates in stylo. - https://github.com/servo/rust-bindgen/pull/709
14:33emilionox: (changes: https://github.com/servo/rust-bindgen/compare/v0.25.0...master)
14:59Guest25625Hi all, I&#39;m writing some tests and was wondering if there&#39;s anyway to be more granular about which tests I run. I know this can be accomplished `cargo test <filter>` but it seems as though the tests only run with `./mach test`. The best I&#39;ve got is `./mach test unit`. `./mach test unit <filter>` doesn&#39;t seem to work. I only ask because the tests take quite a while to run, even when filtered down to just
14:59Guest25625the unit tests.
15:00emilioGuest25625: I think it should support --test-name
15:00emilioGuest25625: which would pass the relevant argument to cargo
15:02noxxidorn: I&#39;m confused,
15:02noxwhy would enabling testharness.js tests be blocked by implementing grid?
15:02noxThe two sound quite orthogonal to each other.
15:03noxAh omg, there is no way to say that such test doesn&#39;t pass with stylo?
15:03noxThat sounds like quite the shortcoming.
15:04xidornnox: no way to record failures in stylo-failures.md
15:04noxBut these are WPT stuff right?
15:04xidornand not even possible to state fail-if
15:04xidornnox: those are not
15:04xidornthey are mochitest
15:05noxMmh.
15:05noxSo mochitest is also called testharness.js?
15:05noxIt&#39;s not this testharness.js? https://github.com/w3c/web-platform-tests/tree/master/resources
15:05xidornsome of them use testharness.js and some use SimpleTest, they are two different frameworks
15:06xidornthe same testharness.js, but its integration with our mochitest system is worse than SimpleTest framework
15:06Guest25625emilio: Ah, silly me. Passing it the path just works. `./mach test <path>` - thank you
15:48* stshine guesses gw won&#39;t like webrender depends on azure and skia
15:55noxstshine: Why does it do?
15:56stshinenox: I want to do that for SVG
15:56noxThat sounds pretty sad.
15:56noxstshine: Can&#39;t it be do like for images, sort of?
15:57noxs/be//
15:57stshinenox: you mean, raster the SVG to image in the layout side?
15:57stshineI&#39;ve been trying to do that for weeks
15:57noxFor example.
15:58noxWouldn&#39;t making WR stop depending on azure and skia something that would take way more than weeks, in the future?
15:58stshineand get the conclusion that it is nearly impossible
15:59stshinenox: there are other rasterizers we can choose
15:59stshinenox: like FreeType
15:59* stshine loves FreeType
16:00noxWhy is it nearly impossible?
16:00stshinenox: But I will try to prototype using azure since we have a pretty good high level binding for it and it is what gekco uses
16:00stshineAnd I don&#39;t even know I could complete it
16:01stshinenox: Because the size of a SVG is determined during layout
16:01stshinewhile webrender_api.update_image() cannot update width and height
16:03stshineso when the dimension of a svg changes, we have to delete the previous image_key, generate a new one, and call add_image for it
16:03noxstshine: I think I see. Anyway I assume you already mentioned these difficulties to gw right?
16:04stshinenox: I just asked him if update_image() could change dimensions, and he said it would be ineffient
16:05stshineinefficient*
16:05noxstshine: How is this different from some width attribute changing on an <img> element though?
16:06stshinenox: the image does not change in this scenario, just we should tell webrender should render it using a different size;
16:06noxOh right.
16:07stshinenox: while for svg we must create a new image and pass the data
16:08stshinethe resource management becomes a burden, especially when the svg fragment get out of scope and I can&#39;t find a way to tell webrender to drop the image
16:08noxstshine: Could rasterising vectorial images be parameterised by some type in WR instead of hardcoding the dependency?
16:08stshinebecause webrender_api is not Sync
16:09stshinenox: you mean, as a optional dependency?
16:09noxNo.
16:09noxNo dependency, let WR callers define some trait impl for rasterisation.
16:10mbrubeckAnyone know how pcwalton&#39;s SVG experiments have gone?
16:10mbrubeckIf pathfinder could be extended to render SVG, and if WR used pathfinder for fonts, then using it for SVG wouldn&#39;t be an additional dependency...
16:10stshinenox: oh, hmm, you know pcwalton is working on his fancy vector rasterizer
16:11mbrubeckbut I don&#39;t know if that&#39;s feasible, or how far away it might be
16:11noxI guess that would be nice yeah.
16:11noxstshine: I&#39;m just wary of adding C dependencies to WR, sounds pretty unfortunate.
16:11stshinenox: and if he succeds it will be in webrender, so what I work is a kind of prototyping
16:12noxOh, I see, nice.
16:12stshinemore exactly, experimenting, because I haven&#39;t even gotten myself into webrender.
16:12nox:)
16:12noxstshine: Disregard me then. :)
16:13stshinembrubeck: He said he finished yet another try a few days ago :)
16:32luisbgI want to start reading the code related to SVG rendering in Servo, for example this: http://tutorials.jenkov.com/svg/circle-element.html
16:32luisbganybody can point me to which areas of the code I should start in?
16:33noxstshine: ^
16:34luisbgnox: thanks :)
16:34stshineluisbg: you can take a look at https://github.com/servo/servo/issues/12973#issuecomment-241551718
16:34crowbotIssue #12973: Support a subset of SVG - https://github.com/servo/servo/issues/12973
16:35stshineluisbg: fwiw, I&#39;ve been working on it for a while, but feel free to try if you like
16:38luisbgstshine: most of this thread is from August 2016
16:39noxluisbg: Is that bad?
16:39luisbgstshine: but if I understand correctly, except a few off master-branch patchsets, the rest is depending on Skia and/or Cairo
16:39luisbgnox: not at all :) just making sure I understand before assuming things
16:39mbrubeckluisbg: To clarify, there is currently no SVG implementation in Servo
16:39mbrubeckluisbg: So this is all just talking about what would need to happen in order to create one.
16:40luisbgmbrubeck: that&#39;s what I see in this thread, double-checking :)
16:40noxemilio: HYPE, PR in test.
16:41luisbgmbrubeck: stshine: I am a bit too newb to attack something that needs writing from the foundations up
16:42luisbgI should look into an other area to explore to get comfortable with layout (style) rendering
16:43stshineluisbg: oh, If you mean to read it, then it does not exist yet :) hope I can make some advance sometime later
16:44luisbgstshine: rendering is a domain of software I haven&#39;t explored yet and was curious about
16:45emiliostshine: I wonder why do you need to update the image synchronously to webrender from layout. AFAIK all major browsers avoid re-encoding the image synchronously that way
16:45luisbgstshine: I wanted to blend the enjoyment of Rust with exploring renderers
16:46emiliostshine: for example, when you zoom an SVG image in Firefox, AFAIK, it first reuses the same resource first, scaling it as needed, then it re-rasterizes the image
16:46stshineluisbg: I understand :) if you want to know how layout works, you can take a look at https://github.com/servo/servo/wiki/Layout-Overview
16:47stshineemilio: what do you mean by &#39;scaling it as needed&#39;?
16:47emiliostshine: also, are you aware of https://github.com/nical/lyon? AFAIK nical wanted eventually to make that work for servo
16:48stshineemilio: you mean, upload the image asynchronously?
16:48emiliostshine: yes
16:48luisbgstshine: nice! reading it now
16:49emiliostshine: is that not doable for some reason?
16:49stshineemilio: because no matter how we upload the image, the image_key have to be stored somewhere
16:50emiliostshine: sure? What&#39;s the problem with a single image key per svg?
16:50stshineemilio: and if you want to rasterize it in layout, you have to store it in svg fragment info
16:50emiliostshine: the thing I&#39;m suggesting is not rasterizing it in layout
16:50stshineemilio: update_image() cannot change the dimensions of texture
16:50emiliostshine: also, other way it could be made, is making it work the same way canvas2d does, though that&#39;s slightly terrible r/n I think
16:50luisbgstshine: amazing! this document is exactly what I need :)
16:51stshineemilio: and changing it would be inefficient acorriding to gw
16:51stshineaccording*
16:51emiliostshine: sure, changing it synchronously surely is
16:51emiliostshine: but it doesn&#39;t need to be if you do it async, which is my point. You don&#39;t need to rasterize the image all the time you do layout
16:52emiliostshine: that would make a demo that animates an SVG setting the width attribute _so_ terrible
16:52stshineemilio: I have been reading the canvas code, but I don&#39;t feel like the idea of create a thread for each svg to do the rasterization
16:52noxstshine: Canvas is broken.
16:52emiliostshine: right, that&#39;s the part that&#39;s terrible right now
16:53noxstshine: And multiple canvases should be multiplexed over a single thread.
16:53emiliostshine: but multiplexing all the canvas rendering in a single thread is doable
16:53noxstshine: Because of this one-thread-one-canvas thing slither.io is unusable.
16:54stshineemilio: didn&#39;t surprise me :)
16:54emiliostshine: and if you can make the same solution work for WebGL, Canvas2D and SVG, that&#39;d be great
16:54emiliostshine: but I think people have been intentionally moving away of doing image decoding synchronously in layout in general
16:54emiliostshine: (image decoding + rasterization, I mean)
16:55emiliostshine: probably worth to talk with nical or other people at #gfx about how they make that work for WR-in-gecko
16:55stshineemilio: well since add geometry api is what webrender need, I would like to try this way
16:56stshinesomewhat terrible to be through. webrender is scary :)
16:57noxstshine: Gankro made it less scary on the soundness front. :P
16:57emiliostshine: I&#39;d really really love you to discuss with the #gfx people about an architecture that would work for this and canvas
16:58stshinenox: what is that?
16:58noxstshine: There was much code that was unsound in the IPC stuff used by WR.
16:58noxstshine: Casting [u8] blobs into enum types and whatnot.
16:59stshineemilio: sure. let me know a little more about webrender first :)
16:59stshinenox: Ah ah, sounds good
17:01emiliostshine: sure, that&#39;s fine :)
18:00noxemilio: Still 25 minutes.
18:02* emilio can see nox with his bottle of 43 celebrating his code-killing spree
18:02noxemilio: Nope, being French tonight.
18:02emilionox: just wine?
18:03noxemilio: Am currently drinking some red wine yes,
18:04noxemilio: An organic sulphide-free one.
18:30nox10 more minutes...
18:41cynicaldevilnox: What&#39;s gonna happen now? :P
18:43noxcynicaldevil: derive(HasViewportPercentage) landing.
18:44noxUgh it&#39;s 1h32min now.
18:44noxSo still ~6min.
18:44noxOOOOH.
18:45noxWOOOOO
18:45noxemilio: ^
18:48jdmheh
18:53noxjdm: Pick one: T, P, C
18:53noxor A.
18:55noxI need to remove ~2,700 more lines for my impact to be negative again.
18:58emilionox: rm -rf components/style
18:58noxemilio: T/P/C/A?
19:03jdmnox: C!
19:04noxjdm: Ok, will derive ToComputedValue then.
19:04jdmnice
19:04noxT was already taken by ToCss.
19:06emilionox: T
19:06noxToo late. :P
19:13nicalstshine: I just read the backlog, in WebRender we added the concept of blob images, which used like images but instead of passing pixel data you pass a serialize list of drawing commands
19:13nicalthat&#39;s what gecko will use for svg in the short/edium term at least
19:14nicaland it can be used with tiled images to represent very large images
19:14nicalthe idea is to split the image into smaller tiles and only render tiles when they are visible.
19:16bholleyManishearth: yt?
19:24cynicaldevilnox: Are there some methods in the TreeSink trait which are only used by the XML tokenizer?
19:24cynicaldevilfor example this one: https://github.com/servo/html5ever/blob/cee43bd304b7bfaadd7c25ed0c53caeebfadfe54/markup5ever/interface/tree_builder.rs#L201 ?
19:29noxcynicaldevil: Nope, https://github.com/servo/html5ever/blob/cee43bd304b7bfaadd7c25ed0c53caeebfadfe54/html5ever/src/tree_builder/actions.rs#L956
19:29noxActually it&#39;s called only by h5e. :)
19:30jdmcynicaldevil: judging by https://github.com/servo/html5ever/commit/19c89ff9b1cb5336b5967285d2d40b6bd4cfa739 I think no
19:30cynicaldevilDamn I was hoping I could skip that one
19:56noxjdm: Lol I thought I could move ToComputedValue to style_traits,
19:56noxI had forgotten about its huuuge Context argument.
20:01jdmoops
20:01jdmtrait?
20:02jdm* philor wonders whether we actually fixed the problems that caused everything to explode from the last wineglass commit message
20:02jdmnox continues to perform penetration testing on mozilla&#39;s build infrastructure
20:03noxAH AH AH
20:03noxjdm: Where was that? :)
20:03jdmnox: #developers
20:24bholleyemilio: your patch busted autoland: https://pastebin.mozilla.org/9022258
20:25emiliobholley: it&#39;s heycam&#39;s bug 1289868 which I landed along my changes, need to land the Gecko side, which I was doing now :)
20:25firebothttps://bugzil.la/1289868 NEW, cam@mcc.id.au stylo: Use the result of CalcStyleDifference to cull parallel DOM traversal
20:27emiliothat&#39;s assuming that my internet connection is stable enough, ofc
20:27bholleyemilio: ah k
20:35emilio_If someone around could land the expectations in bug 1364845, that&#39;d be nice, since my internet doesn&#39;t feel like connecting to mozreview today :(
20:35firebothttps://bugzil.la/1364845 NEW, nobody@mozilla.org stylo: add support for a &quot;re-run selector matching on children only&quot; restyle hint
20:35emilio_bholley_mobile: Manishearth: ^
20:37Manishearthemilio_ no soy aqui
20:37Manishearth(on phone)
20:37canaltinovaemilio_: I did (yay, I can now)
20:37Manishearthwait does canaltinova have l3?
20:38canaltinovaManishearth: yep :)
20:38canaltinovaManishearth: recently got it
20:38emilioManishearth: cool!
20:38emilioManishearth: thanks a bunch
20:39Manishearthnice!
21:57emilionox: wanna review a big mv?
21:57emilionox: ^^
22:33xidornwe probably need to implement -moz-prefixed gradient for stylo then...
22:34xidornnox: ^
22:34noxWhy?
22:34emilioxidorn: why&#39;s that?
22:35xidornwe got compat report the first day we disable it
22:35xidornhttps://bugzilla.mozilla.org/show_bug.cgi?id=1366526
22:35firebotBug 1366526 NEW, nobody@mozilla.org Gmail buttons have no background
22:35noxSad!
22:36noxxidorn: But it&#39;s only Gmail, who cares!
22:36* nox runs away.
22:37emilionox: sorry that&#39;ll cause your code ratio to be even bigger :)
22:37noxDamn it.
22:38noxemilio: This also means -webkit-gradient has to be modified again to be handled as -moz versions.
22:39emilionox: ouch
22:39noxUgh I forgot about from_computed_value...
22:41emilionox: It&#39;s quite sad that the first report is due to a google engineer messing up the standard gradient syntax :(
22:42noxemilio: Wait that means WebKit&#39;s syntax is more lenient than the spec?
22:42emilionox: probably
22:42noxemilio, xidorn: Doesn&#39;t that mean we can just align the spec, support missing &#39;to&#39;, and still unship -moz?
22:43emilionox: shrug? that&#39;s only assuming all the problems w