Add WebRender integration to Servo. #9589

glennw · 2016-02-10T00:20:25Z

WebRender is an experimental GPU accelerated rendering backend for Servo.

The WebRender backend can be specified by running Servo with the -w option (otherwise the default rendering backend will be used).

WebRender has many bugs, and missing features - but it is usable to browse most websites - please report any WebRender specific rendering bugs you encounter!

highfive · 2016-02-10T00:20:29Z

Heads up! This PR modifies the following files:

@ecoal95: components/script/dom/webglrenderingcontext.rs

highfive · 2016-02-10T00:20:30Z

Warning

These commits modify unsafe code. Please review it carefully!

glennw · 2016-02-10T00:20:59Z

Since this is such a large patch, if anyone can take a look at their areas and do partial reviews, that would be great!

emilio · 2016-02-10T00:59:27Z

Awesome work!

Reviewed everything (except the glsl files) and I couldn't spot anything that looked wrong or deserved a change... Not an expert in every part of the codebase this patch touches though.

Left a few notes for myself and for the curious regarding the moved WebGL code.

Review status: 0 of 76 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

components/canvas/webgl_paint_thread.rs, line 182 [r1] (raw file):
If someone's curious, all this logic is moved to webrender_traits in order to be shared with the webrender WebGL backend, which is different from Servo's.

components/canvas_traits/lib.rs, line 43 [r1] (raw file):
Just as a note to self when this merges: An E-Easy should be filled as a follow-up to remove this re-export(s), which were done to avoid extra rebase pain.

Comments from the review on Reviewable.io

pcwalton · 2016-02-10T07:07:18Z

Should I review parts of this or am I too close to being the author for that?

jgraham · 2016-02-10T11:19:15Z

I don't know if it's needed, but note that wptrunner supports switching the expected results of tests based on the render backend, so if we plan to run webrender in CI we should check in metadata updates and not disable tests.

RoxasShadow · 2016-02-10T18:41:22Z

resources/filter.vs.glsl

+{
+	vColorTexCoord = aColorTexCoordRectTop.xy;
+	vMaskTexCoord = aMaskTexCoordRectTop.xy;
+    gl_Position = uTransform * vec4(aPosition, 1.0);


The alignment is broken in this line

jdm · 2016-02-10T18:51:28Z

   Compiling gfx_tests v0.0.1 (file:///home/travis/build/servo/servo/components/servo)

/home/travis/build/servo/servo/tests/unit/gfx/font_cache_thread.rs:14:27: 14:57 error: this function takes 2 parameters but 1 parameter was supplied [E0061]

/home/travis/build/servo/servo/tests/unit/gfx/font_cache_thread.rs:14   let font_cache_thread = FontCacheThread::new(inp_chan);

                                                                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/home/travis/build/servo/servo/tests/unit/gfx/font_cache_thread.rs:14:27: 14:57 help: run `rustc --explain E0061` to see a detailed explanation

error: aborting due to previous error

Build failed, waiting for other jobs to finish...

Could not compile `gfx_tests`.

steveklabnik · 2016-02-10T19:25:39Z

Builds on my machine, seems to work just fine 👍

There's a lot of strange indentation in the GSL files, not sure if that's on purpose or not.

glennw · 2016-02-10T21:33:54Z

Fixed up the unit tests and the notes about the shaders.

glennw · 2016-02-10T21:34:55Z

@jgraham Great! Are there any docs / examples on how I should set up the files to have different expectations?

jgraham · 2016-02-10T21:50:47Z

You can use backend == "cpu" or backend == "webrender" in the expectation files in the same way you can use e.g. os == "mac". If you create a log file from a non-webrender run and a log file from a webrender run then use mach update-wpt default.log webrender.log then it will add the necessary conditions for any tests with different results.

mrobinson · 2016-02-11T00:23:59Z

components/gfx/display_list/mod.rs

@@ -976,6 +985,7 @@ pub enum DisplayItem {
    StackingContextClass(Arc<StackingContext>),
    LayeredItemClass(Box<LayeredItem>),
    NoopClass(Box<BaseDisplayItem>),
+    IframeClass(Box<IframeDisplayItem>),


It feels like NoopClass could simply be removed in favor of the new IframeClass. NoopClass exits just as a placeholder so that iframes can hold a place in the display list without drawing anything.

@mrobinson Sounds good - is it fine to this as a follow up once this PR lands do you think?

@glennw Yeah, I think it would be fine to do in a separate patch.

emilio · 2016-02-11T03:10:53Z

Review status: 0 of 77 files reviewed at latest revision, 7 unresolved discussions.

components/gfx/font_cache_thread.rs, line 300 [r4] (raw file):
This could probably use or_insert_with. Something like:

let font_key = self.webrender_api.as_ref().map(|webrender_api| {
    self.webrender_fonts.entry(template.identifier.clone()).or_insert_with(|| {
        match (template.bytes_if_in_memory(), template.native_font()) {
            (Some(bytes), _) => webrender_api.add_raw_font(bytes),
            (None, Some(native_font)) => webrender_api.add_native_font(native_font),
            (None, None) => webrender_api.add_raw_font(template.bytes().clone()),
        }
    })
});

Comments from the review on Reviewable.io

mbrubeck · 2016-02-11T17:31:38Z

Reviewed 3 of 76 files at r1.
Review status: 3 of 77 files reviewed at latest revision, 8 unresolved discussions.

components/compositing/compositor.rs, line 1422 [r4] (raw file):
Should have a TODO comment and/or follow-up issue to implement zooming.

Comments from the review on Reviewable.io

mbrubeck · 2016-02-11T22:13:20Z

Reviewed 7 of 76 files at r1, 1 of 1 files at r2.
Review status: 11 of 77 files reviewed at latest revision, 9 unresolved discussions.

tests/wpt/harness/wptrunner/browsers/servo.py, line 61 [r4] (raw file):
This will also need to be upstreamed.

Comments from the review on Reviewable.io

mbrubeck · 2016-02-11T22:16:17Z

Reviewed 4 of 76 files at r1.
Review status: 15 of 77 files reviewed at latest revision, 11 unresolved discussions.

components/util/Cargo.toml, line 29 [r4] (raw file):
This appears to be unused.

components/util/lib.rs, line 49 [r4] (raw file):
This looks unused.

Comments from the review on Reviewable.io

bors-servo · 2016-02-13T14:15:37Z

☔ The latest upstream changes (presumably #9622) made this pull request unmergeable. Please resolve the merge conflicts.

pcwalton · 2016-02-16T22:35:00Z

components/compositing/compositor.rs

-fn initialize_png(width: usize, height: usize) -> (Vec<gl::GLuint>, Vec<gl::GLuint>) {
+fn initialize_png(width: usize, height: usize) -> (Vec<gl::GLuint>,
+                                                   Vec<gl::GLuint>,
+                                                   Vec<gl::GLuint>) {


Could we change the return value of this to be a struct?

WebRender is an experimental GPU accelerated rendering backend for Servo. The WebRender backend can be specified by running Servo with the -w option (otherwise the default rendering backend will be used). WebRender has many bugs, and missing features - but it is usable to browse most websites - please report any WebRender specific rendering bugs you encounter!

glennw · 2016-02-18T00:38:23Z

Fixed the android build failure in WR, and updated the cargo.lock files.

glennw · 2016-02-18T00:38:29Z

r=pcwalton

glennw · 2016-02-18T00:38:40Z

@bors-servo r=pcwalton

bors-servo · 2016-02-18T00:38:41Z

📌 Commit c0531c3 has been approved by pcwalton

bors-servo · 2016-02-18T01:46:15Z

⌛ Testing commit c0531c3 with merge 3640b82...

Add WebRender integration to Servo. WebRender is an experimental GPU accelerated rendering backend for Servo. The WebRender backend can be specified by running Servo with the -w option (otherwise the default rendering backend will be used). WebRender has many bugs, and missing features - but it is usable to browse most websites - please report any WebRender specific rendering bugs you encounter!  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9589)

samlh · 2016-02-18T02:19:49Z

▶ PASS [expected FAIL] /css21_dev/html4/floats-wrap-bfc-002-right-table.htm

▶ PASS [expected FAIL] /css21_dev/html4/position-relative-035.htm

bors-servo · 2016-02-18T02:32:06Z

💔 Test failed - mac-rel-css

KiChjang · 2016-02-18T04:25:48Z

Ran 9335 tests finished in 1478.0 seconds.
  • 9333 ran as expected. 58 tests skipped.
  • 2 tests passed unexpectedly

Tests with unexpected results:
  ▶ PASS [expected FAIL] /css-transforms-1_dev/html/transform-input-017.htm

  ▶ PASS [expected FAIL] /css-transforms-1_dev/html/transform-input-018.htm

Looking really good!

jdm · 2016-02-18T04:58:30Z

Those are the same tests that are being reported as unexpected passes in #9668. What's going on here?

glennw · 2016-02-18T05:10:27Z

@jdm This patch includes the commit in #9668 - I was trying to upstream that individually before this patch got reviewed.

glennw · 2016-02-18T19:07:13Z

@bors-servo retry

Ensure when calculating font metrics that the total line height matches requested line height. #9668 has landed now.

bors-servo · 2016-02-18T19:24:06Z

⌛ Testing commit c0531c3 with merge ab07b06...

Add WebRender integration to Servo. WebRender is an experimental GPU accelerated rendering backend for Servo. The WebRender backend can be specified by running Servo with the -w option (otherwise the default rendering backend will be used). WebRender has many bugs, and missing features - but it is usable to browse most websites - please report any WebRender specific rendering bugs you encounter!  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9589)

bors-servo · 2016-02-18T20:14:04Z

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt

highfive added the S-awaiting-review label Feb 10, 2016

RoxasShadow reviewed Feb 10, 2016
View changes

jdm added the S-needs-code-changes label Feb 10, 2016

highfive removed the S-needs-code-changes label Feb 10, 2016

mrobinson reviewed Feb 11, 2016
View changes

highfive added the S-needs-rebase label Feb 13, 2016

pcwalton reviewed Feb 16, 2016
View changes

glennw force-pushed the webrender branch from 3991d4e to c0531c3 Compare February 18, 2016 00:35

highfive added S-awaiting-review and removed S-tests-failed labels Feb 18, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Feb 18, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Feb 18, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Feb 18, 2016

bors-servo merged commit c0531c3 into servo:master Feb 18, 2016

This was referenced Feb 18, 2016

No more headless compositor. Just the normal one. #8641

Merged

Update websocket #9660

Merged

compositing: Stop compositing unnecessarily after each animation frame. #9663

Merged

glennw deleted the webrender branch December 12, 2016 20:46

Add WebRender integration to Servo. #9589

Add WebRender integration to Servo. #9589

glennw commented Feb 10, 2016

highfive commented Feb 10, 2016

highfive commented Feb 10, 2016

glennw commented Feb 10, 2016

emilio commented Feb 10, 2016

pcwalton commented Feb 10, 2016

jgraham commented Feb 10, 2016

RoxasShadow Feb 10, 2016

jdm commented Feb 10, 2016

steveklabnik commented Feb 10, 2016

glennw commented Feb 10, 2016

glennw commented Feb 10, 2016

jgraham commented Feb 10, 2016

mrobinson Feb 11, 2016

glennw Feb 11, 2016

mrobinson Feb 12, 2016

emilio commented Feb 11, 2016

mbrubeck commented Feb 11, 2016

mbrubeck commented Feb 11, 2016

mbrubeck commented Feb 11, 2016

bors-servo commented Feb 13, 2016

pcwalton Feb 16, 2016

glennw commented Feb 18, 2016

glennw commented Feb 18, 2016

glennw commented Feb 18, 2016

bors-servo commented Feb 18, 2016

bors-servo commented Feb 18, 2016

samlh commented Feb 18, 2016

bors-servo commented Feb 18, 2016

KiChjang commented Feb 18, 2016

jdm commented Feb 18, 2016

glennw commented Feb 18, 2016

glennw commented Feb 18, 2016

bors-servo commented Feb 18, 2016

bors-servo commented Feb 18, 2016

Add WebRender integration to Servo. #9589

Add WebRender integration to Servo. #9589

Conversation

glennw commented Feb 10, 2016

highfive commented Feb 10, 2016

highfive commented Feb 10, 2016

glennw commented Feb 10, 2016

emilio commented Feb 10, 2016

pcwalton commented Feb 10, 2016

jgraham commented Feb 10, 2016

RoxasShadow Feb 10, 2016

Choose a reason for hiding this comment

jdm commented Feb 10, 2016

steveklabnik commented Feb 10, 2016

glennw commented Feb 10, 2016

glennw commented Feb 10, 2016

jgraham commented Feb 10, 2016

mrobinson Feb 11, 2016

Choose a reason for hiding this comment

glennw Feb 11, 2016

Choose a reason for hiding this comment

mrobinson Feb 12, 2016

Choose a reason for hiding this comment

emilio commented Feb 11, 2016

mbrubeck commented Feb 11, 2016

mbrubeck commented Feb 11, 2016

mbrubeck commented Feb 11, 2016

bors-servo commented Feb 13, 2016

pcwalton Feb 16, 2016

Choose a reason for hiding this comment

glennw commented Feb 18, 2016

glennw commented Feb 18, 2016

glennw commented Feb 18, 2016

bors-servo commented Feb 18, 2016

bors-servo commented Feb 18, 2016

samlh commented Feb 18, 2016

bors-servo commented Feb 18, 2016

KiChjang commented Feb 18, 2016

jdm commented Feb 18, 2016

glennw commented Feb 18, 2016

glennw commented Feb 18, 2016

bors-servo commented Feb 18, 2016

bors-servo commented Feb 18, 2016