mozilla :: #gfx

8 Aug 2017
13:32Gankrorhunt: good progress on print-type-info; wanted to catch up on some details of how/where you want to handle certain manglings
14:25rhuntGankro: good to hear, what's up?
14:26Gankrorhunt: do you want rustc to distinguish &, *const, Option<&>, Box, Option<Box>, etc?
14:27GankroOstenisbly we want to squash those first 3 together in our final bindings
14:27GankroBut I&#39;m not sure if rustc doing it or us doing it is &quot;better&quot;
14:27GankroAnd the latter 2 feel like they should be left alone, but idk
14:29rhuntGankro: what is the repr of a Box? just a ptr?
14:30Gankrorhunt: yes
14:30Gankrorhunt: well, globally allocated ones, at least
14:31rhuntGankro: I think leaving Box, Option<Box> as is makes sense
14:34rhuntGankro: I&#39;ve been concerned a bit about rust&#39;s reference rules vs C++ pointers
14:35Gankrorhunt: oh?
14:37rhuntGankro: just that with rust you can either have const refs (ptrs) to something or one mut ref (ptr), and C++ doesn&#39;t enforce that. I&#39;d like to do something about it, but I don&#39;t know a good solution
14:38rhuntGankro: when something is a *const/*mut, at least it has to be converted in an unsafe block to ref so there has to be a bit more thought
14:42Gankrorhunt: well within C++ those rules don&#39;t quite matter, insofaras they &quot;only&quot; inform alias analysis
14:43GankroAlthough const casting is very clearly Not Alright for C++ to do to an &T
14:43nicalkats: did you guys manage to figure out the pipeline ids stuff with the webrender update?
14:44katsnical: do you mean document id stuff?
14:44nicalyeah
14:45katsnical: yeah. but now there are more API changes to deal with
14:45katsworking through those now
14:45nicalthe ResourceUpdates change are quite mechanical, I can help
14:46katsnical: if you can pastebin something that would be great
14:46rhuntGankro: half thought out example: https://pastebin.mozilla.org/9029121
14:46katsnical: i&#39;m just fixing up the PROFILER_DBG thing now
14:47Gankrorhunt: ah yes, that&#39;s a fair example
14:47rhuntGankro: I don&#39;t think there&#39;s much binding generation can do to prevent it, but I&#39;d just like people to be aware of it
14:47nicalkats: https://github.com/nical/servo/commit/4bd829ec96b61e9c995d69015d0de22e095683bb
14:47nicalthat&#39;s the equivalent change in servo
14:47Gankrorhunt: if we wrote incredibly const-correct local vars it would be prevented
14:47GankroBut otherwise yeah
14:48katsnical: thanks
14:48GankroErr, actually not even in this case
14:48Gankro:/
14:48rhuntGankro: const correct local vars?
14:48Gankrorhunt: you can&#39;t take a non-const reference to a const
14:48Gankroso that would catch *some* stuff
14:49Gankrorhunt: I think in practice enough stars need to align for this to go wrong that it&#39;s not a major concern
14:49rhuntGankro: yeah I agree.
14:50rhuntGankro: so the only advantage I could think of differentiating between *const, &, Option<&> in cbindgen is if we could pass on the semantic difference into the bindings, but I don&#39;t think there&#39;s a good way
14:52rhuntGankro: we could possibly annotate the function with a comment describing what the rust parameters originally were
14:52rhuntGankro: but that doesn&#39;t seem like a huge benefit when you have the rust src
14:52rhuntso either way I&#39;m fine
14:53Gankrorhunt: we could potentially make & a C++ reference?
14:54rhunthmm
14:54Gankrorhunt: minor(?) point on Box: Rust tells llvm it&#39;s a pointer for function ABI purposes
14:55rhuntGankro: I don&#39;t know enough about C++ references in FFI to know how safe that is. it does have the advantage of enforcing non null
14:56GankroSo if we made a struct Box { ptr: *T } I think we would get busted codegen
14:56Gankroerr, whatever C++ syntax, tired :p
14:56rhuntGankro: is that a special case thing for Box or just a repr(Rust) thing?
14:57Gankrorhunt: box is a magical infestation in the compiler
14:59rhunthah
15:00Gankrobotond: does C++ have a (pseudo-standard) way to say &quot;hey make this pointer a different type but still a pointer for ABI purposes&quot;?
15:01rhuntGankro: so I do feel like it is nice to have more information than less in cbindgen, in case we discover a better solution to preserving semantics. I&#39;m guessing changing cbindgen will be easier than changing this rustc component
15:01rhuntGankro: I think we could do a generic typedef for that
15:02Gankrorhunt: doing the transform in bindgen also means less hacking into the type name stringifier for me :p
15:02rhuntGankro: http://en.cppreference.com/w/cpp/language/type_alias
15:21jrmuizelrhunt: do you know what weiznich is using cbindgen for?
15:22rhuntjrmuizel: no idea, he just started opening pr&#39;s
15:23jrmuizelrhunt: great
15:24rhuntjrmuizel: so you want a C declaration of wr_moz2d_render_cb generated by cbindgen?
15:24jrmuizelrhunt: I do
15:24jrmuizelrhunt: I&#39;m adding a parameter that includes a euclid type and I&#39;d like it get binding generated
15:24rhuntjrmuizel: okay, I just have to remove an if statement that skipped writing those out
15:25rhuntotherwise they already participate in dependency gathering
15:39katsnical: any thoughts? https://treeherder.mozilla.org/logviewer.html#?job_id=121691155&repo=try&lineNumber=1808
15:42nicalkats: doesn&#39;t ring any bell
15:42nicalthis is a big slicce
15:43nicalI assume it is something like the deserialized list of display items
15:46katsi&#39;ll kick off some try pushes to bisect and see if i can repro locally to debug
15:47nicalwhy the heck aren&#39;t we getting line numbers in the backtrace (even in the debug builds)?
15:47katsi think i often don&#39;t get line numbers for rust backtraces
15:47katsi guess i need RUST_BACKTRACE=full
15:47katsat least according to the note in the output
15:49nicalwe should have that on try by default
15:49nicalshould as in &quot;it would be good&quot;
15:49katsi didn&#39;t know it changed from RUST_BACKTRACE=1
15:49katswe have that everywhere
15:49Gankrorhunt: do you think it would make sense to fold the &quot;finding public functions&quot; thing into print-type-info? As a way to get around the macro expand problem?
15:49nicalInterestingly, the slice is always the same size and the butsed index is always the same as well, so it doesn&#39;t seem to depend on the web content
15:50katsit&#39;s happening on startup it looks like
15:50katsprobably the reftest harness window
15:56rhuntGankro: how would that help get around the macro expand problem? we&#39;ve only had some types defined in macros not extern &quot;C&quot; functions
15:57Gankrorhunt: oh I thought you also had macro&#39;d funcs
15:59rhuntGankro: fortunately not
15:59rhuntGankro: what types will print-type-info output? all of them? or only ones used in the lib or ..?
16:00Gankrorhunt: ones that rustc computes the layout for
16:01Gankrohttps://gist.githubusercontent.com/Gankro/e0921f5d7639a9a506a99f807e35f29c/raw/053478e173a1a2aba48c8f2498dd443404b2d8fe/foo.json
16:01Gankro(lots of dummies in there atm)
16:02Gankrorhunt: so ya you&#39;ll want to filter some of those out
16:04rhuntso many good boys
16:04rhuntGankro: yeah we&#39;d do some filtering
16:06katsnical: does this line look correct? https://hg.mozilla.org/try/file/5d2a2b2a747fa0b673e373b3787fd55803454643/gfx/webrender_api/src/display_list.rs#l171
16:06katsnical: this is the backtrace: https://pastebin.mozilla.org/9029130
16:06nicalah!
16:07nicallooks like Gankro&#39;s recent glyph list thing
16:07Gankrorhunt: Also probs gonna make you parse/mangle (x, y, z) and &[T] yourself, but there will be entries for each
16:13rhuntGankro: so a field could have type: &quot;(x, y, z)&quot; but there will also be a type entry with type: &quot;(x, y, z)&quot;?
16:14Gankrorhunt: example: https://gist.github.com/Gankro/85748dbd5f712aca6ebe7282a39a18e2
16:15Gankrorhunt: yes
16:20Gankrorhunt: but I won&#39;t emit thin pointers or fixed-sized arrays, so basically I imagine your type lookup will do blind string lookup with a fallback that parses pointers and [T;n]
16:23katsnical: ah this is the problem: https://hg.mozilla.org/try/file/5d2a2b2a747fa0b673e373b3787fd55803454643/gfx/layers/wr/WebRenderMessageUtils.h#l230
16:23katsit needs updating with more fields
16:23katsi should change it to PodSerializer
16:30rhuntGankro: okay that works.
16:31rhuntGankro: I&#39;m going to have to step up the mangling game though, right now we only handle some generics
16:33Gankrorhunt: yeah. Interestingly pointers and arrays can be done trivially without parsing generics -- ptrs you just parse from front and take rhs opaquely, arrays from back with lhs opaque
16:34rhuntGankro: yeah it shouldn&#39;t be too hard. we could parse the whole type with syn easily, but that&#39;s too much information for a lookup
17:12nicalkats: nice catch
17:13katsgetting another error now, https://treeherder.mozilla.org/logviewer.html#?job_id=121715649&repo=try&lineNumber=2184
17:13katscan repro locally thankfully
17:19katsGankro: halp
17:20Gankrokats: did you update the definition of the DisplayListDescriptor?
17:20katsGankro: update where?
17:20Gankrosec
17:21Gankrokats: ah looks like it&#39;s generated now
17:23Gankrokats: in ffi_generated you have `size_t glyph_offset` as a field at the end?
17:23katsGankro: yeah that&#39;s autogenerated
17:23Gankrocool
17:24katsthis looks like the bincode is getting corrupted or something
17:24katsGankro: is there a good way to debug this?
17:24katslike dumping out the bincode to see if it&#39;s garbage or not?
17:25Gankrokats: probably more accurately bincode is trying to parse garbage
17:25GankroSo the question why is the input slice wrong
17:25Gankro*is why
17:27Gankrokats: before finalizing you could call print_display_list(), and see if that succeeds
17:28GankroIt won&#39;t print aux arrays, only the bare items, but you might see something interesting there (or a total failure)
17:28GankroOH WAIT
17:29Gankrokats: I didn&#39;t udpate push_nested_display_list
17:29Gankroit is super wrong
17:29GankroBecause nothing in the WR test suite uses it
17:29katsGankro: ah
17:29katsGankro: can you write a patch for it?
17:29Gankrokats: yep on it
17:29katsi can apply locally and see if it fixes the problem
17:29katsthx
17:30* kats will eat lunch in the meantime
17:47Gankrokats-lunch: also good thing no one actually uses the cache, deserializing it would have also probably crashed :/
17:54Gankrokats-lunch: try this https://github.com/servo/webrender/pull/1559
18:01katsGankro: fixes the failing test locally for me
18:01katsi&#39;ll do a try push to cover everything
18:01katsthanks for the quick turnaround!
18:02mismithjrmuizel: any guidance you could offer on https://bugzilla.mozilla.org/show_bug.cgi?id=1387764#c2 ?
18:03firebotBug 1387764 NEW, nobody@mozilla.org Perma widget/headless/tests/test_headless.js | application crashed [@ mozilla::gl::GLContextProvider
18:03mismithsummary: headless mode disables the HW_COMPOSITING pref to stop the normal gfx path from trying to set up a GL context (as we don&#39;t have a window to do that with), but WebRender doesn&#39;t check that pref. I could either special-case the WR code for headless or try to make WR obey HW_COMPOSITING
18:06jrmuizelmismith: I responded in bug
18:07mismithjrmuizel: thanks!
18:24milanmstange: what did you say the sync-dispatch class was?
18:24mstangemilan: SyncRunnable
18:24milantx
19:08katskvark: it would be great if #1559 got merged before any other API-changing PRs. on current webrender tip + #1559 I have a clean m-c try run so I can update to it
19:15kvarkkats: looking
19:26kvarkis Gankro there?
19:27Gankrokvark: yo
19:27kvarkwould you be able to address my notes in the PR
19:27kvark^?
19:30Gankrokvark: I&#39;m on the fence on the AuxIter thing, about to go into a meeting brb
19:41Gankrokvark: I&#39;d prefer to leave the AuxIter thing alone just because I have no idea how this is going to be used
19:41Gankroe.g. it might be useful to gw to be able to store the range
19:51Gankrokvark: pushed type alias change
20:02jrmuizelrhunt: what&#39;s the if statement change to emit that wr_moz2d... function?
20:03rhuntjrmuizel: https://github.com/eqrion/cbindgen/blob/master/src/bindgen/library.rs#L874
20:03jrmuizelrhunt: great
20:17jrmuizelBenWa: what does the &#39;dp&#39; in wr_dp_push_clip stand for?
20:19BenWajrmuizel: diff link to jog my memory?
20:20jrmuizelBenWa: I&#39;ll see what I can find
20:21jrmuizelBenWa: I guess https://github.com/jrmuizel/gecko-cinnabar/commit/16612ca52b2812abc65d676e20c986dbceb71121
20:21jrmuizelhttps://github.com/jrmuizel/gecko-cinnabar/commit/16612ca52b2812abc65d676e20c986dbceb71121#diff-91beeff760c148c9f972f4f651af7d49
20:22jrmuizelBenWa: maybe https://github.com/jrmuizel/gecko-cinnabar/blob/16612ca52b2812abc65d676e20c986dbceb71121/layout/base/WebRenderer.h
20:22jrmuizelBenWa: your original webrender prototype came with a header like that
20:23BenWajrmuizel: Yea I dont know
20:23jrmuizelBenWa: a mystery lost to the sands of time
20:24BenWajrmuizel: maybe display or something
20:24BenWajrmuizel: Yea, that might be hit
20:24jrmuizelBenWa: yeah, that&#39;s my best guess
20:24BenWaTheres a display begin/end. I needed some kind of boundary for some reason
22:03jrmuizelgw: what&#39;s the url for the wr perf tracking server again?
22:04mstangejrmuizel: https://wrperf.org/
9 Aug 2017
No messages
   
Last message: 13 days and 15 hours ago