Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Progressive Rendering #557

Merged
merged 9 commits into from Jul 11, 2013
Merged

Progressive Rendering #557

merged 9 commits into from Jul 11, 2013

Conversation

@eschweic
Copy link

eschweic commented Jul 3, 2013

The compositor now keeps track of what has been rendered and what needs rendering based on the window rect, and asks the renderer for tiles only when needed.
src/components/gfx/render_task.rs Outdated
@@ -32,7 +32,7 @@ pub struct RenderLayer {
pub enum Msg<C> {
AttachCompositorMsg(C),
RenderMsg(RenderLayer),
ReRenderMsg(f32),
ReRenderMsg(~[(Rect<uint>, Rect<f32>)], f32),

This comment has been minimized.

Sign in to view
Copy link
@metajack

metajack Jul 3, 2013

Contributor

Please document this struct.
src/components/gfx/render_task.rs Outdated
@@ -128,82 +128,72 @@ impl<C: RenderListener + Owned> Renderer<C> {
}
}

fn render(&mut self, scale: f32) {
fn render(&mut self, tiles: ~[(Rect<uint>, Rect<f32>)], scale: f32) {

This comment has been minimized.

Sign in to view
Copy link
@metajack

metajack Jul 3, 2013

Contributor

I would prefer to use a named structure here instead of the anonymous tuple.
src/components/main/compositing/mod.rs Outdated
@@ -368,6 +468,8 @@ impl CompositorTask {

root_layer.common.set_transform(scroll_transform);

// ask_for_tiles();

This comment has been minimized.

Sign in to view
Copy link
@metajack

metajack Jul 3, 2013

Contributor

Why is this commented out?
@eschweic
Copy link
Author

eschweic commented Jul 3, 2013

Current issues with this branch:

  • To prevent request flood, the compositor currently only asks for tiles on a click event. This should be switched to scroll and zoom events instead.
  • After merging with the current code, rendering became very slow. I'm not sure what the cause is (merge fallout?), but hopefully #553 will fix this.
  • @metajack you mentioned that you would like an option to render the entire layer. I don't have any code for this yet, but you could theoretically change the window size to the page size, which should trigger the entire layer to render. Would this work for your purposes?
src/components/main/compositing/mod.rs Outdated
@@ -406,7 +508,8 @@ impl CompositorTask {
window_size.height as f32 / -2f32,
0.0);
root_layer.common.set_transform(zoom_transform);


// ask_for_tiles();

This comment has been minimized.

Sign in to view
Copy link
@metajack

metajack Jul 3, 2013

Contributor

Why is this commented out?
@eschweic eschweic mentioned this pull request Jul 3, 2013
Closed
@eschweic
Copy link
Author

eschweic commented Jul 3, 2013

@metajack Here is an example of the output of get_html:

get_html_example
@metajack
Copy link
Contributor

metajack commented Jul 8, 2013

This needs a rebase.
@eschweic
Copy link
Author

eschweic commented Jul 8, 2013

Should be fixed now. r? @metajack
src/components/main/compositing/quadtree.rs Outdated
struct QuadtreeNode<T> {
/// The tile belonging to this node. Note that parent nodes can have tiles.
tile: Option<T>,
/// The positiong of the node in page coordinates.

This comment has been minimized.

Sign in to view
Copy link
@pcwalton

pcwalton Jul 10, 2013

Contributor

nit: position
src/components/main/compositing/quadtree.rs Outdated
tile: Option<T>,
/// The positiong of the node in page coordinates.
origin: Point2D<f32>,
/// The width and hight of the node in page coordinates.

This comment has been minimized.

Sign in to view
Copy link
@pcwalton

pcwalton Jul 10, 2013

Contributor

nit: height
src/components/main/compositing/quadtree.rs Outdated

/// Generate html to visualize the tree

This comment has been minimized.

Sign in to view
Copy link
@pcwalton

pcwalton Jul 10, 2013

Contributor

Maybe mention that this is a debugging function? Eventually we would like to mark this with #[cfg(debug)].
src/components/main/compositing/quadtree.rs Outdated

/// Generate html to visualize the tree
pub fn get_html(&self) -> ~str {
let header = "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\"> <html xmlns=\"http://www.w3.org/1999/xhtml\">";

This comment has been minimized.

Sign in to view
Copy link
@pcwalton

pcwalton Jul 10, 2013

Contributor

nit: make this a constant, and <!DOCTYPE html><html> is fine.
src/components/main/compositing/quadtree.rs Outdated
let mut ret = ~[];

match self.tile {
Some (ref tile) => ret = ~[tile],

This comment has been minimized.

Sign in to view
Copy link
@pcwalton

pcwalton Jul 10, 2013

Contributor

nit: no space after Some (eventually rustfmt should automate this)
eschweic added 9 commits Jun 22, 2013
eschweic
Make quadtrees generic
d590a7b
eschweic
Address pcwalton's comments
ad7dc32
eschweic
Implement progressive rendering
6bebda4
eschweic
Clean up and annotations
1948280
eschweic
Cleanup after merge
1b225fb
eschweic
Fix paint permissions issue, clean up
d400860
eschweic
Address metajack's comments
435941e
eschweic
Move buffer requests to quadtree; add remove_tile() to quadtree; auto…
cc816c0 
…matic rerendering after pinch-zoom
eschweic
Address pcwalton's nits; automatic rendering after scrolling
Loading status checks…
8a0878a
@pcwalton

This comment has been minimized.

Sign in to view
Copy link

pcwalton commented on 8a0878a Jul 11, 2013

r+
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented on 8a0878a Jul 11, 2013

saw approval from pcwalton
at eschweic@8a0878a

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Jul 11, 2013

merging eschweic/servo/master = 8a0878a into auto

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Jul 11, 2013

eschweic/servo/master = 8a0878a merged ok, testing candidate = 4cf1ac9

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Jul 11, 2013

all tests pass:
http://servo-buildbot.pub.build.mozilla.org/builders/linux/builds/115
http://servo-buildbot.pub.build.mozilla.org/builders/mac/builds/69

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo replied Jul 11, 2013

fast-forwarding master to auto = 4cf1ac9
bors-servo pushed a commit that referenced this pull request Jul 11, 2013
bors-servo
auto merge of #557 : eschweic/servo/master, r=pcwalton
4cf1ac9 
The compositor now keeps track of what has been rendered and what needs rendering based on the window rect, and asks the renderer for tiles only when needed.
@bors-servo bors-servo merged commit 8a0878a into servo:master Jul 11, 2013
1 check passed
1 check passed
default all tests passed
ChrisParis pushed a commit to ChrisParis/servo that referenced this pull request Sep 7, 2014
@jgraham
Merge pull request servo#557 from deniak/ms-async
0dca503 
fixes on MS async tests + rm trailing ws
glennw pushed a commit to glennw/servo that referenced this pull request Jan 16, 2017
@bors-servo
Auto merge of servo#604 - kvark:gpu_marker, r=glennw
3be3018 
GPU markers support

Fixes servo#557

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/604)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@eschweic @metajack @pcwalton @bors-servo
You can’t perform that action at this time.