Remove old rendering backend. by glennw · Pull Request #13711 · servo/servo

glennw · 2016-10-12T05:59:58Z

This removes paint threads, rust-layers dependency, and changes
optional webrender types to be required.

The use_webrender option has been removed, however I've left
the "-w" command line option in place for now so that wpt
runner can continue to pass that. Once it's removed from there
we can also remove the -w option.

Once this stage is complete, it should be fine to change the
display list building code to generate webrender display
lists directly and avoid the conversion step.

This change is

highfive · 2016-10-12T06:00:02Z

Heads up! This PR modifies the following files:

@asajeffrey: components/constellation/lib.rs, components/constellation/pipeline.rs, components/constellation/Cargo.toml, components/constellation/constellation.rs
@KiChjang: components/script/dom/canvasrenderingcontext2d.rs, components/script/dom/window.rs, components/script_traits/lib.rs, components/script_traits/lib.rs, components/script_layout_interface/Cargo.toml, components/script_traits/script_msg.rs, components/script_traits/script_msg.rs, components/net/image_cache_thread.rs, components/script_layout_interface/lib.rs, components/script/script_thread.rs, components/script_layout_interface/message.rs, components/net_traits/image/base.rs, components/net_traits/image/base.rs, components/script_traits/Cargo.toml, components/script_traits/Cargo.toml

highfive · 2016-10-12T06:00:03Z

Warning

These commits modify gfx, layout, net, and script code, but no tests are modified. Please consider adding a test!

glennw · 2016-10-12T06:00:07Z

@bors-servo try

bors-servo · 2016-10-12T06:00:09Z

⌛ Trying commit ba9d0bb with merge e429ebd...

Remove old rendering backend. This removes paint threads, rust-layers dependency, and changes optional webrender types to be required. The use_webrender option has been removed, however I've left the "-w" command line option in place for now so that wpt runner can continue to pass that. Once it's removed from there we can also remove the -w option. Once this stage is complete, it should be fine to change the display list building code to generate webrender display lists directly and avoid the conversion step.

bors-servo · 2016-10-12T06:00:23Z

💔 Test failed - mac-rel-wpt1

glennw · 2016-10-12T06:02:14Z

@bors-servo retry

glennw · 2016-10-12T06:21:35Z

@bors-servo try

Remove old rendering backend. This removes paint threads, rust-layers dependency, and changes optional webrender types to be required. The use_webrender option has been removed, however I've left the "-w" command line option in place for now so that wpt runner can continue to pass that. Once it's removed from there we can also remove the -w option. Once this stage is complete, it should be fine to change the display list building code to generate webrender display lists directly and avoid the conversion step.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13711)

bors-servo · 2016-10-12T06:21:38Z

⌛ Trying commit c241737 with merge 409399e...

bors-servo · 2016-10-12T06:45:19Z

💔 Test failed - windows-dev

glennw · 2016-10-12T07:20:02Z

@bors-servo try

bors-servo · 2016-10-12T07:20:05Z

⌛ Trying commit 90c3764 with merge e43bd42...

Remove old rendering backend. This removes paint threads, rust-layers dependency, and changes optional webrender types to be required. The use_webrender option has been removed, however I've left the "-w" command line option in place for now so that wpt runner can continue to pass that. Once it's removed from there we can also remove the -w option. Once this stage is complete, it should be fine to change the display list building code to generate webrender display lists directly and avoid the conversion step.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13711)

glennw · 2016-10-12T07:24:50Z

We should get the stacking context patch from @mrobinson reviewed and merged before this. This can still be reviewed, and I'll rebase on top of that when it lands.

Ms2ger · 2016-10-12T06:37:21Z

components/compositing/compositor.rs

-    webrender: Option<webrender::Renderer>,
+    webrender: webrender::Renderer,

    /// The webrender interface, if enabled.


Fix the comment, please

Ms2ger · 2016-10-12T06:44:34Z

components/compositing/compositor.rs


    gl::bind_texture(gl::TEXTURE_2D, 0);

-    let renderbuffer_ids = if opts::get().use_webrender {


This appears to be backwards

That piece of code was WR-only, but it's now not required by WR at all - so I've removed it completely.

Ms2ger · 2016-10-12T06:51:35Z

components/compositing/compositor_thread.rs

 use euclid::point::Point2D;
 use euclid::size::Size2D;
-use gfx_traits::{Epoch, FrameTreeId, LayerId, LayerProperties, PaintListener};
+use gfx_traits::{LayerId};


Nit: no braces

Ms2ger · 2016-10-12T07:04:17Z

components/servo/Cargo.lock

 name = "webrender"
 version = "0.6.0"
 source = "git+https://github.com/servo/webrender#b31b4cf76324bfbe82d27a059e1330ef5ae34c35"
+replace = "webrender 0.6.0"


Oops pushed a [replace] section override accidentally - fixed.

emilio

Only a few comments, not an in-depth review.

emilio · 2016-10-12T08:07:00Z

components/canvas/canvas_paint_thread.rs

+                                            webrender_traits::ImageFormat::RGBA8,
+                                            element.into());

            let pixel_data = CanvasPixelData {


Please fill a follow-up to remove the image_data field from CanvasPixelData.

emilio · 2016-10-12T08:08:53Z

components/compositing/compositor.rs

@@ -756,18 +614,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
            }

            (Msg::CollectMemoryReports(reports_chan), ShutdownState::NotShuttingDown) => {


At this point we could also remove this message entirely. Please do it or file a followup for it.

emilio · 2016-10-12T08:14:04Z

components/compositing/compositor.rs

-        }
+                                _pipeline_id: PipelineId,
+                                _layer_id: LayerId,
+                                _point: Point2D<f32>) {


So if I understand this code correctly, this is only called on resize to keep the scroll where the fragment of the URL points to. The code in script_thread.rs has a FIXME(pcwalton) saying it's very bogus. I think it's a regression we could take, what do you think?

It's also used for scrolling to a # anchor in a page. However, it doesn't work with webrender right now anyway, so it's not technically a regression. #13736

glennw · 2016-10-12T21:54:47Z

@bors-servo try

bors-servo · 2016-10-12T21:54:50Z

⌛ Trying commit 29567a8 with merge 1102593...

bors-servo · 2016-10-17T11:04:17Z

📌 Commit 324914d has been approved by larsbergstrom

larsbergstrom · 2016-10-17T17:46:56Z

@bors-servo clean try- r+

bors-servo · 2016-10-17T17:46:57Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: Bump html5ever to 0.5.5 #13801

bors-servo · 2016-10-17T17:46:57Z

📌 Commit 324914d has been approved by larsbergstrom

emilio · 2016-10-18T00:13:15Z

@bors-servo: p=1000

Prone to bitrot

emilio · 2016-10-18T00:17:34Z

@bors-servo: clean r- try-

emilio · 2016-10-18T00:17:54Z

@bors-servo: r=larsbergstrom,pcwalton,Ms2ger,emilio

bors-servo · 2016-10-18T00:17:55Z

📌 Commit 324914d has been approved by larsbergstrom,pcwalton,Ms2ger,emilio

This removes paint threads, rust-layers dependency, and changes optional webrender types to be required. The use_webrender option has been removed, however I've left the "-w" command line option in place so that wpt runner can continue to pass that. Once it's removed from there we can also remove the -w option. Once this stage is complete, it should be fine to change the display list building code to generate webrender display lists directly and avoid the conversion step.

glennw · 2016-10-18T00:22:18Z

@bors-servo r=larsbergstrom

bors-servo · 2016-10-18T00:22:19Z

📌 Commit acfdfd2 has been approved by larsbergstrom

bors-servo · 2016-10-18T00:22:21Z

⌛ Testing commit acfdfd2 with merge f96718d...

Remove old rendering backend. This removes paint threads, rust-layers dependency, and changes optional webrender types to be required. The use_webrender option has been removed, however I've left the "-w" command line option in place for now so that wpt runner can continue to pass that. Once it's removed from there we can also remove the -w option. Once this stage is complete, it should be fine to change the display list building code to generate webrender display lists directly and avoid the conversion step.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13711)

bors-servo · 2016-10-18T01:16:55Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

highfive assigned larsbergstrom Oct 12, 2016

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 12, 2016

highfive added the S-tests-failed The changes caused existing tests to fail. label Oct 12, 2016

glennw force-pushed the remove-old-renderer branch from ba9d0bb to c241737 Compare October 12, 2016 06:02

highfive removed the S-tests-failed The changes caused existing tests to fail. label Oct 12, 2016

highfive added the S-tests-failed The changes caused existing tests to fail. label Oct 12, 2016

glennw force-pushed the remove-old-renderer branch from c241737 to 90c3764 Compare October 12, 2016 07:19

highfive removed the S-tests-failed The changes caused existing tests to fail. label Oct 12, 2016

Ms2ger mentioned this pull request Oct 12, 2016

Integrate stacking contexts into the display list #13682

Merged

5 tasks

glennw force-pushed the remove-old-renderer branch from 90c3764 to c233541 Compare October 12, 2016 07:23

Ms2ger reviewed Oct 12, 2016

View reviewed changes

emilio reviewed Oct 12, 2016

View reviewed changes

glennw force-pushed the remove-old-renderer branch from c233541 to 29567a8 Compare October 12, 2016 21:54

glennw force-pushed the remove-old-renderer branch from 324914d to acfdfd2 Compare October 18, 2016 00:22

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 18, 2016

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 18, 2016

bors-servo merged commit acfdfd2 into servo:master Oct 18, 2016

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 18, 2016

This was referenced Oct 18, 2016

[WIP] Move asynchronous network loads to the fetch stack and remove unused code. #13714

Closed

Replace rust-websocket with ws-rs in the debugger server. #13258

Closed

[WIP] Implement History State #13781

Closed

glennw deleted the remove-old-renderer branch October 18, 2016 01:32

mrobinson mentioned this pull request Jan 17, 2018

Fix up the x11 hack for getting stuck in XInitThreads #5780

Closed


		gl::bind_texture(gl::TEXTURE_2D, 0);

		let renderbuffer_ids = if opts::get().use_webrender {

		@@ -756,18 +614,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
		}

		(Msg::CollectMemoryReports(reports_chan), ShutdownState::NotShuttingDown) => {

Uh oh!

Conversation

glennw commented Oct 12, 2016 • edited by larsbergstrom Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

highfive commented Oct 12, 2016

Uh oh!

highfive commented Oct 12, 2016

Uh oh!

glennw commented Oct 12, 2016

Uh oh!

bors-servo commented Oct 12, 2016

Uh oh!

bors-servo commented Oct 12, 2016

Uh oh!

glennw commented Oct 12, 2016

Uh oh!

glennw commented Oct 12, 2016

Uh oh!

bors-servo commented Oct 12, 2016

Uh oh!

bors-servo commented Oct 12, 2016

Uh oh!

glennw commented Oct 12, 2016

Uh oh!

bors-servo commented Oct 12, 2016

Uh oh!

glennw commented Oct 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilio left a comment

Choose a reason for hiding this comment

Uh oh!

emilio Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glennw commented Oct 12, 2016

Uh oh!

bors-servo commented Oct 12, 2016

Uh oh!

bors-servo commented Oct 17, 2016

Uh oh!

larsbergstrom commented Oct 17, 2016

Uh oh!

bors-servo commented Oct 17, 2016

Uh oh!

bors-servo commented Oct 17, 2016

Uh oh!

emilio commented Oct 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emilio commented Oct 18, 2016

Uh oh!

emilio commented Oct 18, 2016

glennw commented Oct 12, 2016 •

edited by larsbergstrom

Loading

emilio Oct 12, 2016 •

edited

Loading

emilio commented Oct 18, 2016 •

edited

Loading