mozilla :: #servo

14 Jul 2017
00:08gw\o/
00:19jryansbholley: your dev-servo email links to a try run, but i think it's supposed to be some code...?
00:20bholleyjryans: thanks
00:26canaltinovawaffles: I'm thinking more fundamental change for TrackRepeat parsing btw, that might make these hacks unnecessary but I prefer reducing the failing tests to 0 first :)
00:26wafflescanaltinova: makes sense to me
00:29jycseems like I can't build servo on my Arch laptop -- it looks like I'm crashing on https://github.com/servo/servo/blob/master/ports/glutin/window.rs#L261
00:30jycis there maybe something else I need to install to get it to work? I was able to load the upstream glutin example just fine
00:31mbrubeckjyc: Hmm... are you running X, or Wayland?
00:31mbrubeckgw: Text looks a *lot* better on Linux after #17706
00:31crowbotPR #17706: Improve font layout in Linux / Freetype platforms. - https://github.com/servo/servo/pull/17706
00:31jycmbrubeck: I'm running X
00:32gwmbrubeck: great! same for me on all the sites I tested too :)
00:32gwmbrubeck: still at least one bug I know that needs to be fixed for linux + hidpi, but it's minor, I think
00:33mbrubeckjyc: I'm not sure what the cause is then, but maybe your graphics drivers don't support some GL option that Servo requests?
00:33mbrubeckjyc: Is there any other info in the panic message?
00:35jycmbrubeck: the panic message actually doesn't have anything, I just get the backtrack and stuck print statements to find where it's crashing
00:35mbrubeckgw: underline thickness rounding still a little weird... on https://limpet.net/mbrubeck/ with --device-pixel-ratio 2, it looks like some underlines are 2 device pixels and some are 1
00:35mbrubeckgw: (they should all be 1px == 2 device pixels)
00:35jycmbrubeck: will get the full log
00:35gwmbrubeck: interesting - could you file a bug for that?
00:36gwmbrubeck: also, going to be working on native WR text-decoration support next week, so could be a good time to fix that up
00:36jycmbrubeck: full backtrace here: https://gist.github.com/jyc/6901fd9d934f8d0ee26bd5bb23514168 , the "creating window" line is from a println I added the line before, there's a "window created" line right after
00:37mbrubeckgw: okay
00:38mbrubeckjyc: `compiler_builtins::probestack::__rust_probestack` I wonder if this means it's hitting the new stack overflow protection...
00:39Manishearthmbrubeck: yes that it is
00:39mbrubeckjyc: related, https://github.com/rust-lang/rust/issues/43102
00:39crowbotIssue #43102: Since last nightly, winit doesn't create windows any more - https://github.com/rust-lang/rust/issues/43102
00:39Manishearththough iirc that doesn't imply that this is a stack overflow issue, just that we were probing the stack here
00:43ajagw: adding "ink" perchance?
00:43mbrubeckjyc: It looks like this should be fixed by updating to the latest nightly rustc
00:43jycmbrubeck: thanks for the help--in a related issue, someone says they fixed it by going back
00:44jycoh
00:44jycok, cool! will try that
00:44mbrubeckjyc: Note that the Servo build system downloads its own nightly by default
00:44mbrubeckbased on the `rust-commit-hash` file
00:44jycmbrubeck: I tried cargo clean when I first saw this and built again, do I also need to delete that flie manually?
00:44jycfile*
00:45mbrubeckjyc: One option is to `cp servobuild-example .servobuild` (if you don't have a .servobuild already) and then edit .servobuild to set `system-rust = true`
00:46mbrubeckand then make sure the latest nightly is your default `rustc`
00:46canaltinovawaffles, Manishearth: finally ^^ :)
00:47mbrubeckjyc: If that works without breaking anything, then we can update `rust-commit-hash` to point to the newer version
00:47jycmbrubeck: ok, will try that
00:47mbrubeck(You can get its commit hash using `rustc -Vv`)
00:49gwaja: just all the different underline styles (wavy, dotted etc), correct shadows etc at this point
00:53ajagw: iow, what gecko has now?
00:54gwaja: yep. does gecko not support ink?
00:54ajanope
00:55gwaja: ok. I don't think it's super hard ... render the text run to a sub-task, sample from that while drawing the decoration, I guess ...
00:55* gw hasn't actually read the spec
00:57canaltinovambrubeck, jyc: I think nox updated the rustc today and it merged 4 hours ago https://github.com/servo/servo/pull/17714
00:57crowbotPR #17714: Update to rustc 1.20.0-nightly (f85579d4a 2017-07-12) - https://github.com/servo/servo/pull/17714
00:57canaltinovajyc: did you pull the latest changes?
00:57wafflescanaltinova: nice!
00:57ajaline positions would be helpful for CJK, too
00:57wafflescanaltinova: will look at it later
00:57jyccanaltinova: seems I pulled from yesterday! maybe that's it
00:58canaltinovajyc: yeah, I guess they reverted rustup yesterday and did another rustup again today
00:58canaltinovawaffles: thanks!
01:04gwaja: want to open a bug in the WR repo with details?
01:14ajagw: Text Decoration 4...maybe not ready for prime time
01:17KiChjangfitzgen, r- on https://github.com/servo/rust-bindgen/pull/813
01:17crowbotPR #813: 11/10 MAJESTIC AF - https://github.com/servo/rust-bindgen/pull/813
01:17ajawouldn't be part of the shorthand, in any event
01:17KiChjanghow dare you put doggos under servo's repos
01:17KiChjangeveryone in servo knows cats are better
04:43jdmcrowbot: what's new?
04:43crowbotServo destroyed Microsoft Edge&#39;s WebBluetooth :<
07:20emilioheycam: quick re-r in #17713? I have some already reviewed patches that conflict with that PR :)
07:20crowbotPR #17713: style: Kill some style sharing code. - https://github.com/servo/servo/pull/17713
07:21heycamsorry, didn&#39;t see your replies there
07:22Manishearthemilio: is there any easy way to track leaks?
07:22* heycam cries at lack of interdiff
07:22heycamemilio: can you point me to the changes?
07:22emilioManishearth: MOZ_COUNT_DTOR/MOZ_COUNT_CTOR?
07:22emilioheycam: I just answered questions (need to move the comment though)
07:22heycamemilio: ah k
07:23heycamr=me then
07:23Manishearthemilio: I&#39;m getting leakcheck errors on try but i dont know which test it is since the error is printed when the suite finishes
07:23Manishearthemilio: how does that work?
07:23Manishearthemilio: no so that seems to register with leakcheck. that&#39;s already done
07:24Manishearthbut I can&#39;t find which test is leaking, and I can&#39;t track the leak down (maybe rr will help)
07:24Manishearthbut i wonder if there&#39;s tooling
07:24SimonSapinnox: amazing https://reviewboard.mozilla.org/r/157274/diff/1#index_header
07:25emilioheycam: thanks!
07:25emilioManishearth: which class is it?
07:25noxSimonSapin: Wtf
07:25emilioSimonSapin: lol
07:33Manishearthemilio: lots of them, but basically ServoComputedValues (i.e. CVInner)
07:33Manishearthemilio: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6edf477940bcd347de71b4f225f60e5ab8d141f7
07:38cynicaldevilnox: ping
07:38Manishearthbholley: the fuse stuff is ready for perf testing btw if you want to get some numbers
07:41Manishearth(I want to get some numbers :) )
07:53hiroemilio: thanks for the review. A question about restyle damage.
07:53hiroemilio: is the restyle damage accumulated even if the element has no WAS_RESTYLED flag?
07:54hiroI am looking RestyleData.contains_restyle_data()
07:54emilioManishearth: it might be the case that you&#39;ve missed a call to the counter too...
07:55emiliohiro: so, the WAS_RESTYLED flag is set on compute_style
07:55emiliohiro: which is entered, if the element has a restyle hint to process
07:56emiliohiro: so, in theory, no, it&#39;s not accumulated if there&#39;s no WAS_RESTYLED flag, because the element is not restyled
07:57hiroemilio: OK, thanks that&#39;s exactly I am hoping. :)
07:57emiliohiro: thanks a lot again for working on that btw, it&#39;s awesome :)
07:58hiroemilio: np, you always notice me a lot of things what we should do. it&#39;s really helpful.
08:05Manishearthemilio: hmmm
08:05Manishearthemilio: how would that happen?
08:05Manishearthemilio: like, these are all child fields of SCV, but SCV itself isn&#39;t counted
08:10heycamemilio: do you know if there&#39;s a specific reason accumulate_damage etc. take the old values as &ComputedValues and the new value as &Arc<ComputedValues>
08:10emilioheycam: I _thought_ there was, let me take a quick look
08:11heycamemilio: it&#39;s just going to be a bit annoying over FFI to sometimes be passing a pointer to the arc inner and sometimes to the ComputedValues itself
08:12emilioheycam: Oh, right, so it was just to pass it over FFI, but I&#39;d prefer if it took to &ComputedValues instead... But whatever is easier, really
08:13heycamemilio: great thanks
08:14heycamemilio: btw I just realised that we don&#39;t call ResolveSameStructsAs on visited style contexts
08:15heycamemilio: and I wonder if that could possible cause us to miss some visited style changes
08:15emilioheycam: I think/thought we always repainted if the visited style changed?
08:15emilioheycam: oh, you mean dynamic changes to :visited rules... yeah, that sounds plausible
08:15heycamemilio: yeah, since the Peek calls will return null
08:16* heycam files
08:52noxcynicaldevil: Bank holiday but pong.
08:57cynicaldevilnox: I found that a few tests are failing because some TreeSink methods are being called after h5e tokenizer&#39;s feed function has returned. How is that possible?
09:20noxcynicaldevil: That sounds like a bug.
09:20noxcynicaldevil: I&#39;m not sure how that can happen.
09:20noxemilio: Do you see the point I&#39;m trying to make in https://github.com/servo/servo/pull/17704#issuecomment-315312148?
09:20crowbotPR #17704: Add animation value related with stroke-dasharray / stroke-dashoffset / stroke-width. - https://github.com/servo/servo/pull/17704
09:21noxemilio: If you do, and if you could explain what I mean by a specific type, I would be happy for you to comment there if I&#39;m not making enough sense.
09:24sewardjemilio: I have a non-optimised build (w/ stylo) failing on Linux due to inlining problems. is this a known problem?
09:25cynicaldevilnox: here&#39;s the test that&#39;s failing: https://github.com/servo/servo/blob/master/tests/wpt/mozilla/tests/mozilla/out-of-order-stylesheet-loads.html
09:26cynicaldevilnox: This wasn&#39;t problem in the sync version, but now with async what&#39;s happening is that as soon as h5e&#39;s feed function returns, the main thread stops listening for parser operations immediately, and therefore they are sent from the parser thread but never received.
09:27emilionox: that makes sense to me... I&#39;m not sure how should it be clearer?
09:27emiliosewardj: not that I know of...
09:27emiliosewardj: do you have some error message?
09:27noxemilio: If it immediately made sense to you, it&#39;s probably ok.
09:27emilioheycam: quick r? #17729
09:27crowbotPR #17729: stylo: Honor cursor: progress. - https://github.com/servo/servo/pull/17729
09:28noxemilio: So we don&#39;t support calc() for percentages in Servo?
09:28emilionox: how not?
09:28emilionox: not for standalone percentages
09:28noxemilio: https://bugzilla.mozilla.org/show_bug.cgi?id=1380918
09:28emilionox: because afaict there was no property that used them
09:28sewardjemilio: https://pastebin.mozilla.org/9027062
09:28firebotBug 1380918 NEW, nobody@mozilla.org stylo: Support calc percent in -webkit-gradient
09:28noxemilio: What do you mean?
09:28noxemilio: Any property that says <percentage> should support calc() where the percentage is.
09:29emiliosewardj: I think that&#39;s a general incremental build issue, that code is DOM code
09:29emiliosewardj: so you probably need to clobber or something like that
09:30sewardjemilio: that is a from-clean build
09:30noxWhich code?
09:30emilionox: the code in sewardj&#39;s pastebin, it&#39;s Gecko code
09:30noxAh.
09:30* nox swiftly disappears.
09:31emilionox: sure, the thing is that the only property that used a standalone Percentage (that is, not LengthOrPercentage) was a random moz property that didn&#39;t support it in Gecko
09:31sewardjemilio: here&#39;s the mozconfig. https://pastebin.mozilla.org/9027064
09:31emilionox: last time I checked at least
09:31noxemilio: Oh I see.
09:31emiliosewardj: huh, not sure what&#39;s going on, sorry :(
09:31sewardjemilio: the exact same thing with
09:31noxemilio: I feel like we should make a SimpleCalc type or something.
09:32noxemilio: For Number, Time, Angle, Percentage, and things like that.
09:32sewardjemilio: &quot;ac_add_options --enable-optimize=&quot;-g -O2&quot;&quot; works fine
09:32noxAh never mind, Number is specially special with the clamping mode.
09:32noxThough the clamping mode too could be probably abstracted out in the type itself.
09:32sewardjemilio: so you&#39;re using an optimised build? can you show me your mozconfig?
09:33nox&quot;We need more types&quot; is the new &quot;we need more vespene gas&quot;.
09:33emiliosewardj: I have a debug non-opt build, and an opt build
09:33emiliosewardj: My opt mozconfig is:
09:33emiliohttps://www.irccloud.com/pastebin/eQI2eLZO/mozconfig
09:34emilionox: you mean to avoid all the duplication in CalcNode::to_foo?
09:34sewardjemilio: ok, thanks -- but what&#39;s your non-opt mozconfig?
09:35emiliosewardj: also pretty standard:
09:35emiliohttps://www.irccloud.com/pastebin/RBUz4u2m/mozconfig
09:35noxemilio: Nah, I mean the types of simple calc() things.
09:36noxemilio: Like Number and the clamping mode just there to serialise the calc().
09:36noxI&#39;m wondering if we can abstract the whole &quot;was this a calc&quot; thing in its own generic type.
09:36sewardjemilio: ok, thanks
09:41emilionox: I guess that&#39;d be nice
09:41emilionox: anyway, I can take that percentage bug
09:41sewardjemilio: I&#39;ll file a bug. I need a non-opt build to work.
09:49noxemilio: Cool, thanks!
09:49noxemilio: I didn&#39;t want to ask but I expected you would do it, calc-senpai.
09:49emilionox: lololol
09:49emilionox: it&#39;s easy enough, the hard part is ripping off all the users of percentage.0, I guess...
09:50noxemilio: Or do Percentage(f32, bool) but meh.
09:50emilionox: we need the clamping mode, but anyway, no big deal
09:50noxemilio: Percentages get clamped?
09:50emilionox: for NumberOrPercentage
09:50noxMmh...
09:50emilionox: (apparently)
09:50noxAm confused now.
09:51noxYou can&#39;t mix numbers and percentages.
09:51cynicaldevilnox: How do I test whether the bug is present or not, independently of Servo?
09:52noxemilio: Actually I am not even sure the spec says that percentage&#39;s calcness should be preserved for serialisation.
09:52noxcynicaldevil: Not sure I understand your question.
09:52emilionox: you can for <number-percentage>, but remember that our NumberOrPercentage is actually <number> | <percentage>
09:52noxemilio: Ok, values-4 says it should be preserved.
09:52noxemilio: But!
09:52noxemilio: Tab Atkins told me <number-percentage> is going away.
09:53emilionox: that&#39;s good, but we still need <number> | <percentage>, right?
09:53noxemilio: Yes, in which calc is either made of numbers,
09:53noxor of percentages,
09:53emilionox: which is where we apparently need it...
09:53noxbut not both.
09:53emilionox: yeah, sure
09:53noxemilio: So it can just be Either<Number, Percentage>,
09:53emilionox: but I still need to clamp percentages
09:53noxWhere?
09:54emilionox: NumberOrPercentage::parse_non_negative(context, i)?
09:54noxMmh...
09:54emilionox: moz_image_rect ;_;
09:55noxOk, unfortunate I guess.
09:55noxI don&#39;t remember reading any property from actual specs talking about clamped percentages.
09:56heycamemilio: r=me
09:56heycam(was afk)
09:57emilioheycam: thanks!
11:46emilionox: There you go ^
11:47noxemilio: Sweet.
11:58emilionox: if you could review https://github.com/servo/servo/pull/17732, that&#39;d be appreciated too
11:58crowbotPR #17732: style: Simplify font_size::SpecifiedValue::as_font_ratio. - https://github.com/servo/servo/pull/17732
12:00noxemilio: It&#39;s Bastille Day.
12:00emilionox: ouch, of course, nvm, enjoy your red wine :)
12:00noxNp. :)
12:00emilionox: as long as you don&#39;t drink 43 I&#39;m fine
12:00noxemilio: Looked at that one anyway, r+ if geckotry is green.
12:01noxemilio: I bought some Get 27, but it&#39;s 2pm.
12:01emilionox: thanks!
12:02* emilio kicks nox from the channel
12:02noxAh ah that log.
12:02emilionox: go enjoy your holiday
12:02noxOh don&#39;t worry about that.
12:02noxAlready threw a few Overwatch games.
12:10emilionox: did you know about https://lists.webkit.org/pipermail/webkit-dev/2013-February/023820.html?
12:10* emilio didn&#39;t
12:20noxI did know about that.
12:26emiliohttp://webkitmemes.tumblr.com/post/20185509171/paranoid-parrot-on-code-review classic
12:27emilioOh, we have http://mozillamemes.tumblr.com/
14:16jdmcrowbot: what should I work on?
14:16crowbotjdm: extract feature queries (@supports) into an independent crate (https://drafts.csswg.org/css-conditional/#at-supports)
15:30* jdm disembarks
15:35* larsberg has to look up the word disembark at least once a month to remind himself if that means he arrived or departed
15:37gsnedderswell, really it&#39;s neither
15:37gsneddersyou can disembark without having arrived
15:38gsnedders(e.g., I had to disembark off a flight to the US because the gate staff had failed to mark my boarding pass for secondary screening, so they had to get me off the plane to then go through all my stuff)
15:42larsbergyeah I assume he mainly means &quot;I am leaving the train, which had internet, and going to the office/home, and will be offline for a bit&quot;
15:42jryansjgraham: ping
15:42bzThe question is whether the office/home has internet
15:50noxlarsberg: At this point we can redefine disembarking to &quot;whatever jdm does when he goes away from channel&quot;.
16:01gsneddersDoes Servo support the Font Loading API?
17:26Manishearthemilio: there?
17:32emilioManishearth: just for a few minutes
17:38Manishearthemilio: this styleresolver stuff broke everything :|
17:38emilioManishearth: broke everything as in...?
17:38Manishearthemilio: should parent_style be a style context or a ComputedValuesInner?
17:38Manishearthemilio: the patches
17:38Manishearthemilio: all the important changes were in matching.rs, this moves everything, but also changes the behavior in a way I don&#39;t completely understand yet
17:38Manishearthe.g the with_default_parent_styles
17:38emilioManishearth: it should be whatever you should inherit from... Ideally a StyleContext, because it&#39;s not clear to me why ComputedValuesInner exists at all
17:39emilioManishearth: that with_default_parent_styles was what was in matching.rs before to grab the layout parent and the inheritance parent spread through a bunch of places
17:40emilioManishearth: also, any opinion on bug 1380980?
17:40firebothttps://bugzil.la/1380980 NEW, nobody@mozilla.org stylo: font_size::SpecifiedValue::as_font_ratio looks super-fishy.
17:40emilioManishearth: it&#39;s not clear to me how that fits in the setup you have for font-size
17:41Manishearthemilio: it&#39;s an oversight
17:43emilioManishearth: sure, but how is it supposed to work? i.e., suppose you have 10em + 10% + 4px, what&#39;s supposed to be the ratio?
17:46Manishearthemilio: like i said in the comment
17:46Manishearthemilio: you store two bits
17:46Manishearthwe store ax right now, now you store ax+b
17:47Manishearthshit I messed up :|
17:47Manishearthugh this rebase is really annoying
17:47Manishearththis is why I wanted to land part 1-5 early
17:47emilioManishearth: oh, sorry, I misunderstood what you meant by &quot;oversight&quot; :)
17:48emilioManishearth: but anyway, can I help you with the StyleResolver thing? My idea is that it was going to be easier to write your patch without modifying behavior with that
17:48Manishearthemilio: well the patch was already written :|
17:49Manishearthemilio: nothing you can help with, i just need to keep trying things here
17:50Manishearthemilio: thanks for this fix -- i see how it can help clear things up :)
17:50emilioManishearth: sure, but your patch made it even more complex, which is what was trying to avoid with my changes :)
17:50emilioManishearth: np, I&#39;m glad to hear that! It removed ~1k lines of sloppy code :)
17:50Manishearthemilio: I don&#39;t see how this avoids complexity
17:51Manishearthor maybe ive misunderstood the patch
17:51Manishearthbecause this rebased patch is becoming even more complex
17:53emilioManishearth: well, indeed your changes to matching.rs weren&#39;t that terrible
17:53emilioManishearth: but anyway, just compare the 6 ways we had to call accumulate_damage vs. now
17:54Manishearthsure sure
17:54gsneddershttps://github.com/w3c/web-platform-tests/pull/6558 --- anyone able to say whether or not this&#39;ll break Servo?
17:54Manishearthemilio: in the meantme, any luck figuring out the transitions stuff?
17:54crowbotPR #6558: Make reftests wait for fonts to load over WebDriver - https://github.com/w3c/web-platform-tests/pull/6558
17:54Manishearthwill take me a while to get this rebase worked out
17:54emilioManishearth: Not so far (had to go on a search for apartments yesterday :/)
17:55emiliogsnedders: IIRC we load webfonts sync for reftests
17:55emiliogsnedders: let me double-check that
17:57Manishearthemilio: np
17:58emiliogsnedders: tests/wpt/harness/wptrunner/executors/executorservo.py: &quot;-Z&quot;, &quot;disable-text-aa,load-webfonts-synchronously,replace-surrogates&quot;
18:00Manishearthemilio: what&#39;s up with the computed values argument on Servo_StyleSet_GetBaseComputedValuesForElement ?
18:00emiliogsnedders: oh, what you fear to break is the removal of reftest-wait_servodriver?
18:00emilioManishearth: it&#39;s the style from nsComputedDOMStyle (i.e., the already resolved style)
18:00Manishearthemilio: i&#39;m not sure what to do with that
18:00emilioManishearth: when the element doesn&#39;t have animations or isn&#39;t styled, we return that
18:00Manishearthemilio: you call into_strong on it a bunch of times
18:01Manishearthemilio: ok so it needs to be a style context? I don&#39;t think that&#39;s possible, let me check
18:01emilioManishearth: I guess it should become a StyleContext now?
18:01emilioManishearth: you definitely have a style context on the caller
18:01gsneddersemilio: yes
18:01Manishearthemilio: kay
18:01gsneddersemilio: and relying on the same as everywhere else
18:02emiliogsnedders: hmm... I dunno how well our mutation observers are implemented... probably jdm knows?
18:02jdmthey are incomplete
18:02jdmwhether they&#39;re complete enough for what we need in another question
18:03* jdm looks at the PR
18:03jdmgsnedders: luckily we don&#39;t use webdriver at all so servo will not break due to that PR
18:18jgrahamjryans: Ah, sorry I&#39;m on PTO, but if it&#39;s trivial I&#39;ll do it now
18:19jryansjgraham: ah okay, tried to check for that via your bugzilla name... anyway, it&#39;s quite a small change!
18:21jryansjgraham: thanks!
18:23bholleyjdm: yt?
18:23jdmbholley: yes
18:23bholleyjdm: see bug 1381045
18:23firebothttps://bugzil.la/1381045 NEW, nobody@mozilla.org stylo: Significant Tp5 regression between rev 0e41d07a703f and rev b07db5d650b7
18:23jdmnooo
18:24bholleyjdm: at least I did the analysis before bothering you about it :-)
18:24jdmyeah, my anguish was a response to the analysis
18:25bholleyjdm: should be relatively easy to profile though - just load the dailymotion page and profile stylesheet parsing
18:25jdmtime for an opt build I guess
18:32bholleyjdm: just grabbed a quick profile: https://perfht.ml/2urWtCk
18:32bholleyjdm: error reporting definitely dominates
18:33jdmbholley: is that in nightly or with my change?
18:33bholleyjdm: 526ms out of 553ms total parse time
18:33bholleyjdm: oh right
18:33jdmgood lord
18:33bholleyjdm: that&#39;s not with your change
18:33jdmcertainly the utf8 conversions should dominate less now
18:34bholleyjdm: I could do it with your change if it would be helpful, but if you&#39;re well on your way then I can leave you to it
18:34jdmyeah, build in progress
18:34jdmgo do stuff that needs bholley magic
18:34bholleyjdm: :-)
18:35bholleyjdm: oh, pro tip: ./mach talos-test --suite tp5n
18:35bholleyjdm: will download the suite
18:35jdmthanks
18:35bholleyjdm: then grep your tree for dailymotion
18:35bholleyjdm: and it&#39;ll be in there
18:45edunhamjack: hey so uh i am going through issues and i just noticed that i asked you a couple things on https://github.com/servo/servo/issues/7080 and neglected to check for answers for an embarrassingly long time
18:45crowbotIssue #7080: CI improvement meta bug - https://github.com/servo/servo/issues/7080
18:47jackedunham: I meant build without cleaning the build sir
18:47jackDir
18:47jackNot rust incremental builds but cargo ones
18:47jackI thought we had done that though
19:02KiChjangdoes anyone know which method in WindowMethods controls what to draw on the embedder?
19:27KiChjangpaul or cbrewster: ^
19:27KiChjangor ajeffrey ^
19:30ajeffreyKiChjang: well the embedder provides the GL context with fn gl(&self) -> Rc<gl::Gl>,
19:30ajeffreybut you may have something else in mind?
19:32edunhamjack: yeah i think we have; we have a bunch of open issues for cleaning up those incrementals at the right times
20:08KiChjangajeffrey, ah, so that&#39;s what makes it possible
20:09KiChjangi&#39;ve always wondered how in the world it works
20:15bzI just want to make sure we actually fix this in 55, one way or another
20:16bzer, wrong window
20:20Manishearthemilio: there?
20:20emilioManishearth: yes
20:22Manishearthemilio: nvm
20:22emilioheh
20:22* emilio goes having some dinner
20:22Manishearthemilio: http://searchfox.org/mozilla-central/source/dom/animation/KeyframeEffectReadOnly.h#368-372
20:22Manishearthbasically, with my changes we&#39;d have one that takes an nsStyleContext and one that takes a ServoStyleContext
20:22Manishearthbut the nsstylecontext one is gecko only so i just made it take gecko
20:23emilioManishearth: makes sense, what&#39;s the problem with that?
20:23bzjdm: ping
20:23jdmbz: pong
20:24Manishearthemilio: no i hadn&#39;t realized it was gecko only when i pinged you
20:24Manishearth:)
20:24bzbholley: ping
20:24bzjdm: Are you looking into the error reporting perf thing?
20:24jdmbz: yes
20:25bzjdm: I was considering grabbing some builds and profiling, but figured there&#39;s no point in duplicating work
20:25bzjdm: ok, great
20:25jdmyep, I&#39;m already looking at a profile
20:25jdmand it&#39;s not what I expected at all
20:25bzoh?
20:25emiliojdm: just curious, how does it look?
20:25bzIf it&#39;s a gecko profiler profile, link?
20:25jdmbz: https://perf-html.io/public/af250efba2212b8b44f0387e7c47713cdc61b9b0/calltree/?hiddenThreads=&search=Repor&thread=0&threadOrder=0-1
20:25jdmbz: all of the time is in the nsScriptError destructor deallocating
20:26bzhuh
20:26emiliofun
20:26jdmand I&#39;ve got a non-stylo profile to compare against
20:27bzhrm
20:27jdmhttps://perfht.ml/2tc3Q0s
20:27bzSo that profile shows a bunch of time under devtools prettifyCSS
20:27mbrubeckwhat platform?
20:27bzI assume that&#39;s not the part you&#39;re looking at?
20:28jdmbz: no, I filtered by ReportUnexpected
20:28bzah
20:28bzhere we are
20:28* bz pokes
20:28bzSo the stylo one has ParseSheet at 31ms
20:29ManishearthFireBlood: canaltinova https://air.mozilla.org/Birunthan_Mohanathas/
20:29bzof which 28ms is error reporting
20:29bzIs this a synthetic thing, or the dailymotion page?
20:29Manishearthhttps://air.mozilla.org/mlayzell/
20:29jdmbz: the dailymotion page from the suite
20:29bz(Gecko has 18ms under ParseSheet, 2ms under ReportUnexpected)
20:30bzSo the good news is that stylo can parse this 5x as fast as Gecko, normally?
20:30bzanyway
20:30bzok
20:31noxManishearth: How dare you paste links about IndexedDB
20:32jdmthe utf8->utf16 source line conversion still kills us
20:32jdmaccording to this profile
20:32jdmhmm, I bet that stylo ends up logging more errors than non-stylo
20:32jdmand that exceeds the message buffering
20:32jdmcausing us to retire older logged errors in stylo
20:33jdmwhich would hit https://dxr.mozilla.org/mozilla-central/source/xpcom/base/nsConsoleService.cpp?q=logmessagewithmode&redirect_type=direct#322
20:33jdmhence the destructor showing up under that function
20:33jdmnot really sure that we can do anything about that :/
20:33jdm100ms under huge_dealloc ;_;
20:34mbrubeckAre there more errors because it uses stuff that Gecko accepts but Stylo doesn&#39;t, or just because Stylo reports things differently?
20:34jdmmbrubeck: the former
20:34mbrubeckand are those things that we expect to fix in Stylo?
20:35emiliojdm: are you sure? which things is stylo reporting that we should implement?
20:35jdmso, for example, the page uses a -background-image css property
20:35jdmstylo reports that it doesn&#39;t know what that is
20:35jdmbut I don&#39;t see any errors about that in gecko
20:36mbrubeckweird
20:36jdmsame with _height
20:36bzAh
20:36bzSo...
20:36bzStylo needs to be fixed
20:36jdm-moz-box-shadow, -moz-border-radius...
20:37bzthose need to be supported
20:37bzSo for -background-image
20:37bzhttp://searchfox.org/mozilla-central/source/layout/style/nsCSSParser.cpp#7165-7169
20:37bzhttp://searchfox.org/mozilla-central/source/layout/style/nsCSSParser.cpp#1670-1675
20:38bzWe don&#39;t report errors for &quot;vendor prefixed unknown property name&quot;
20:38bzWhich means things that start with &quot;-&quot; or&quot;_&quot; but not &quot;-moz&quot;
20:38jdmaha
20:38bzSo that&#39;s the &quot;style needs to be fixed&quot; part.
20:38bzer, stylo
20:38bzDo we really not support box-shadow and border-radius yet?
20:39bzUgh
20:39bzLooks like stylo supports box-shadow and -webkit-box-shadow but not -moz-box-shadow
20:39bzhttp://searchfox.org/mozilla-central/source/servo/components/style/properties/longhand/effects.mako.rs#17-26
20:40bzSame for border-radius
20:40bzhttp://searchfox.org/mozilla-central/source/servo/components/style/properties/shorthand/border.mako.rs#207-210
20:40* bz checks what Gecko does
20:40noxShould be just a matter of adding moz to the list of vendor prefixes.
20:40noxIf the parsing is the same that is.
20:40bzWell, if Gecko supports it
20:41* bz is checking
20:41bzNo, Gecko dropped those
20:41bzok
20:42bzSo good. We should fix that &quot;don&#39;t report vendor-prefixed non-moz things&quot;
20:42bzbit
20:42bzAnd see how much that helps
20:42bzhttps://bugzilla.mozilla.org/show_bug.cgi?id=693510 for the record
20:42firebotBug 693510 FIXED, dbaron@dbaron.org drop support for prefixes from border-radius* and box-shadow
20:42bzWe only fixed that in Firefox 13
20:42bzso it makes sense that websites haven&#39;t updated yet.
20:43bz(and we supported the unprefixed versions since Firefox 4....)
20:44* bz goes back to the profile, sees it really is all in ~nsScriptError doing huge_dalloc
20:44bzand hence poisoning
20:44bzand munmap, wow
20:52bzjdm: fwiw, I&#39;m not seeing the encoding conversion in your profile...
20:52jdmbz: yeah, I took another one and it cropped up
20:52bzok
20:52jdmbz: https://perfht.ml/2tbzLOK
20:53bzweird
20:53bzon the same testcase?
20:53jdmyep
20:53jdmjust did a force refresh and saved the profile
20:54* jdm will try ignoring the prefixes
20:54* bz is poking at something
20:55bzwait
20:55bzOh, the error line
20:56bzdidn&#39;t you then have a followup to make that happen less?
20:57bzRight, 1380488
20:57bzIs this profile with or without that fix?
20:57jdmbz: yeah, but we still need to convert the utf8 source to utf16 once for each line that causes an error to be reported
20:57jdmbz: with that fix
20:57bzok
20:57bzIt&#39;s really bizarre that it was not caught in the other profile at all
20:58jdmmhm
20:58bzAlso, this is rather dumb
20:58bzin that we&#39;re getting data off the network, converting to utf-16, converting to utf-18, feeding to servo
20:58jdmyes
20:58bzAnd then having to convert back to utf-16 here. :(
20:59* bz hefts his two pieces of coax and watches the reflections at the interface
20:59* jdm wonders what the devtools client uses errorLine for
20:59bzwell
20:59bzIn theory for showing a nice view with the actual line and text pointing to it
20:59bzBut in practice I suspect &quot;nothing at all&quot;
20:59bzWhich is what the devtools client tries to do with CSS errors in general
20:59jdmyeah, considering that we just link to the actual source in the console
21:00bz&quot;link&quot;
21:00bz(it doesn&#39;t work very well)
21:00bzAnyway, simply not providing the source line string for stylo CSS errors is a possibility....
21:01jdmI see only one use of &quot;.sourceLine&quot; in DXR outside of tests
21:01jdmand that&#39;s the code that copies that value to send it to the devtools client
21:01bzmmhm
21:02bzok, so that sends it as lineText
21:03* bz not sure that gets used for anything
21:05bzAnyway, we should probably do that
21:05bzwith some comments as to why or something
21:05jdmyeah, that seems like the simplest way out of this performance sink
21:06jryanshmm, what are we removing from the CSS errors? the `sourceLine`?
21:07jdmjryans: yes
21:11jryansjdm: okay, i agree it appears to be unused :) just wanted to double check
21:11jdmexcellent, I will stake jryans&#39; reputation on this decision
21:12jryansjdm: haha, well, most likely i am the only one who will notice for now when i re-run the devtools tests ;)
21:17jryansjdm: on the topic of errors, i noticed the messages are _almost_ the same, but there are few spots where gecko adds an extra &quot;declaration dropped&quot; string to the message as well, like: http://searchfox.org/mozilla-central/source/layout/style/nsCSSParser.cpp#1898-1899
21:18jryanswondering if it&#39;s slightly easier to garden tests if we make the messages match where it&#39;s easy to do so...
21:18jdmjryans: yeah, I&#39;ve been meaning to file a bug about adding the declaration dropped/skipped
21:18* jdm does that
21:18jryansokay, great!
21:30jdmbz: so just providing an empty string instead of the actual CSS source makes a _huge_ difference to the profile; it now looks indistinct from the gecko one
21:30jdm2ms under error reporting code total
21:50Manishearthemilio: looks like I have to pass ServoStyleContext EVERYWHERE in animations code now :)
21:50Manishearthmaybe this will fix the bug though
22:15pcwaltonstandups: Got more aggressive de Casteljau subdivision working in the subdivision branch of Pathfinder; now benchmarking Loop Blinn vs. tessellation to see which one is better
22:15standupsOk, submitted #48241 for https://www.standu.ps/user/pcwalton/
22:19Manishearthyesss it compiled
22:19Manishearthwait no
22:19Manishearthit didn&#39;t link
22:21ManishearthIT LINKED
22:22Manishearthand it crashed
22:22Manishearthoh ok this is an expected crash
22:28bholleybirtles: yt?
22:32Manishearthbholley: btw, had a chance to perf test the patches?
22:32Manishearthwanna be sure I&#39;m not doing something stupid there and killing the perf wins
22:33bholleyManishearth: I have not, sorry. It&#39;s on my list, I just have a big list today
22:33bholleyManishearth: is your stuff otherwise ready to land?
22:33Manishearthnp
22:33Manishearthbholley: no :(
22:33bholleyManishearth: what&#39;s left?
22:33Manishearthbholley: there&#39;s a leakcheck issue, transitions are inexplicably broken, the pres context issue, and this really long rebase I&#39;ve spent most of today pushing through
22:34Manishearththere are also followups that can wait
22:34Manishearthaccording to emilio the bug I just rebased over should make it easier to track down the transitions issue, so there&#39;s that
22:34Manishearthtransitions are broken by part 9, which basically ... doesn&#39;t do anything that should affect behavior?
22:34bholleyManishearth: I think the prescontext issue can be a followup
22:35Manishearthcool
22:36Manishearthanyway, the major work is done, and it can be perf tested, it just can&#39;t be landed
22:37Manishearthall the review comments are addressed though, so it can land pretty much immediately after i find the bugs, fix them, and get a quick review
22:53luke_nukem[m]-MI recently used bindgen to output bindings for libffi which has a few functions that take another function as an argument - bindgen has wrapped this arg in an Option<>... is this correct?
22:53hiro!seen emilio
22:53firebotemilio was last seen 2 hours and 18 minutes ago, saying &#39;jdm: are you sure? which things is stylo reporting that we should implement?&#39; in #servo.
22:54jdmluke_nukem[m]-M: yes; it allows for null function pointers
22:54luke_nukem[m]-Mjdm: how does that work? Does the None option get translated to a null?
22:55luke_nukem[m]-Mjdm: and does the rust compiler deal with the Option unwrap automatically?
22:56jdmluke_nukem[m]-M: in C a function pointer can be null; in Rust they cannot. an Option around means the compiler will automatically turn a null argument into None, and a non-null one into Some
22:57jdmor if it&#39;s the reverse (rust calling C) then yes, the resulting Option is represented as a pointer under the hood. there&#39;s no separate discriminant, so C can treat it like a normal function pointer.
22:57luke_nukem[m]-MAh cool, thanks for the explanation
22:58canaltinovawaffles, Manishearth: I&#39;m pretty sure this is the final form ^^ :)
23:00canaltinovawaffles: actually, it turns out you already made the repeating and line names merging part in the `ToComputedValue`. So only I had to move it to parsing process.
23:02canaltinovawaffles, Manishearth pushed to try and we&#39;ll see the results in here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de2c71ef9a7b67aea9d81ceed2778406cd11b66a
23:04bholleyManishearth: I&#39;m getting build errors on your stuff
23:08bholleyManishearth: nm, they seem to have