Closed Bug 1335525 Opened 9 years ago Closed 9 years ago

Land webrender rust code and build glue in m-c

Categories

(Core :: Graphics: WebRender, defect, P3)

Product:
Core
Component:
Graphics: WebRender
Version:
Other Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla54
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(4 files)

Bug 1335525 - Add top-level webrender crates to gfx/ (already reviewed by gfx).
59 bytes, text/x-review-board-request
Details
Bug 1335525 - Update libgkrust to include webrender as an optional feature.
59 bytes, text/x-review-board-request
froydnj
: review+
Details
Bug 1335525 - Add webrender dependencies to third_party/rust.
59 bytes, text/x-review-board-request
Details
Bug 1335525 - Add --enable-webrender configure option and hook it up build webrender.
59 bytes, text/x-review-board-request
gps
: review+
Details
As per the plan I outlined at [1] the first step in the merge is to land the webrender bindings rust code and all its dependencies into m-c. This will allow us to get the build glue properly reviewed. [1] https://groups.google.com/forum/#!msg/mozilla.dev.tech.gfx/Foxlm-c-Md8/y6khtK1CCwAJ
The exact contents of parts 1 and 3 will change tomorrow as we are planning to update the version of webrender in the graphics tree. However parts 2 and 4 won't change, and that's really what needs review. Please also do take a look at part 3 (which adds a large pile of stuff into third_party/rust) to see if there's anything you object to there. Eventually this will be subject to a code audit and licensing stuff which are tracked separately in bug 1322798 and bug 1316990 respectively.
Comment on attachment 8832199 [details] Bug 1335525 - Update libgkrust to include webrender as an optional feature. https://reviewboard.mozilla.org/r/108532/#review109998
Attachment #8832199 - Flags: review?(nfroyd) → review+
FYI bug 1335799 is tracking the webrender update that's in-progress (which will change the contents of parts 1 and 3).
Depends on: 1335799
^ rebased to latest m-c. The webrender update hasn't happened yet, so I won't land these until Monday at the earliest, even assuming gps granted an r+ today.
Comment on attachment 8832198 [details] Bug 1335525 - Add top-level webrender crates to gfx/ (already reviewed by gfx). https://reviewboard.mozilla.org/r/108530/#review110684 Was any thought given to alternate locations for webrender? Given that it is effectively a standalone project, I think /webrender is more appropriate than /gfx/webrender.
Comment on attachment 8832201 [details] Bug 1335525 - Add --enable-webrender configure option and hook it up build webrender. https://reviewboard.mozilla.org/r/108536/#review110698 This looks good. I know you asked in IRC, but I want it stated in the bug: please hold off landing this until Servo lands. That should be by EOD.
Attachment #8832201 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] (disappearing for a month after 2017-02-10) from comment #13) > Was any thought given to alternate locations for webrender? Given that it is > effectively a standalone project, I think /webrender is more appropriate > than /gfx/webrender. We did discuss how the webrender code overall should be laid out (bug 1331515) although I don't think we ever considered putting webrender *outside* of the gfx directory. I think it makes sense inside gfx/ (and I suspect most of the graphics team would agree) because it's used for gfx purposes, just like other libraries (cairo, skia, graphite2, etc.) which all also live under gfx/. (In reply to Gregory Szorc [:gps] (disappearing for a month after 2017-02-10) from comment #14) > I know you asked in IRC, but I want it stated in the bug: please hold off > landing this until Servo lands. That should be by EOD. Will do, thanks!
I took the code I wrote for importing Servo into mozilla-central and applied it to the webrender repo. The result is at https://hg.mozilla.org/users/gszorc_mozilla.com/webrender-linear/. If you want to import that into mozilla-central so you have full VCS history, let me know.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #15) > (In reply to Gregory Szorc [:gps] (disappearing for a month after > 2017-02-10) from comment #13) > > Was any thought given to alternate locations for webrender? Given that it is > > effectively a standalone project, I think /webrender is more appropriate > > than /gfx/webrender. > > We did discuss how the webrender code overall should be laid out (bug > 1331515) although I don't think we ever considered putting webrender > *outside* of the gfx directory. I think it makes sense inside gfx/ (and I > suspect most of the graphics team would agree) because it's used for gfx > purposes, just like other libraries (cairo, skia, graphite2, etc.) which all > also live under gfx/. I also think that other third party libraries like cairo, skia, graphite2, etc should not live inside gfx/. My main concern is wanting a clear division between code with different workflows and semantics because it improves comprehension and makes tooling slightly easier. Projects like Servo and WebRender fall into that bucket IMO because they aren't specific to Firefox/Gecko whereas lots of the code in gfx/ is.
(In reply to Gregory Szorc [:gps] (disappearing for a month after 2017-02-10) from comment #16) > If you want to import that into mozilla-central so you have full VCS > history, let me know. Thanks, but I don't think we want this. If anything I'd prefer to eventually go the other way, and remove gfx/webrender from our tree, and just have it as a regular vendored crate in third_party/rust. However we want to wait for it to stabilize a bit more before we do that. (In reply to Gregory Szorc [:gps] (disappearing for a month after 2017-02-10) from comment #17) > I also think that other third party libraries like cairo, skia, graphite2, > etc should not live inside gfx/. > > My main concern is wanting a clear division between code with different > workflows and semantics because it improves comprehension and makes tooling > slightly easier. Projects like Servo and WebRender fall into that bucket IMO > because they aren't specific to Firefox/Gecko whereas lots of the code in > gfx/ is. There's definitely an argument to be made for that, but I'd rather do it as a separate bug/discussion than conflate it with getting this webrender stuff landed.
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/bafbb19be9a4 Add top-level webrender crates to gfx/. r=gfx https://hg.mozilla.org/mozilla-central/rev/7fa68c2685b2 Update libgkrust to include webrender as an optional feature. r=froydnj https://hg.mozilla.org/mozilla-central/rev/d300c689f52e Add webrender dependencies to third_party/rust. r=gfx https://hg.mozilla.org/mozilla-central/rev/bfb91220ceb5 Add --enable-webrender configure option and hook it up build webrender. r=gps
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
No longer depends on: 1335799
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: