Skip to content
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

Multi-layer Compositing #675

Closed
wants to merge 5 commits into from
Closed

Multi-layer Compositing #675

wants to merge 5 commits into from

Conversation

@eschweic
Copy link

eschweic commented Aug 5, 2013

This refactors the compositor and adds initial support for multiple compositor layers being displayed at once. A new module, compositor_layer.rs, does most of the heavy lifting that was once in mod.rs.

--- clipped so bors is happy ---

@pcwalton

This comment has been minimized.

difference == how many times you have to double or halve

@pcwalton

This comment has been minimized.

Oh, I see, so you're recursively dividing in this branch and otherwise recursively halving in the other branch. Might want to leave a comment to this effect.

container: @mut ContainerLayer,
}

pub fn CompositorLayer(pipeline: Pipeline, page_size: Size2D<f32>, tile_size: uint,

This comment has been minimized.

@tikue

tikue Aug 5, 2013

I would move this function into impl CompositorLayer and name it new.

@pcwalton

This comment has been minimized.

nit: I would say "potentially has a larger size, since it is rounded up to a power of two".

@pcwalton

This comment has been minimized.

Copy link

pcwalton commented on 95f089e Aug 5, 2013

It would be good to allocate only as much space as is needed in video memory for tiles with portions outside the clip. Doesn't have to be done as part of this patch, but we should get this issue on file as I suspect there will be nasty memory usage edge conditions that could arise without it.

@pcwalton

This comment has been minimized.

Please move this to a function called new inside the impl. This is the current Rust style.

@pcwalton

This comment has been minimized.

Comment whether higher numbers mean "closer to the viewer" or "farther from the viewer".

@pcwalton

This comment has been minimized.

What is the ID used for? Needs a comment.

@pcwalton

This comment has been minimized.

By "redisplayed" you mean "rerendered", right?

@pcwalton

This comment has been minimized.

Child layers, you mean?

@pcwalton

This comment has been minimized.

To the scroll position of the page? I'm a little confused as to what this means. Is it relative to the top left of the page, do you mean?

@pcwalton

This comment has been minimized.

Might be nice to factor these out to use the Tree trait (you don't have to do this as part of this patch)

@pcwalton
Copy link
Contributor

pcwalton commented Aug 5, 2013

Looks good modulo nits. Once those are fixed, r=me.

bors-servo pushed a commit that referenced this pull request Aug 7, 2013
This refactors the compositor and adds initial support for multiple compositor layers being displayed at once. A new module, `compositor_layer.rs`, does most of the heavy lifting that was once in `mod.rs`. New features include:
* Better scrolling. Scrolling is now applied on a per-layer basis. The layer under the cursor is scrolled first; if it cannot be scrolled (i.e. it is at a page boundary), the scroll is propagated to the layer's parents.
* Better zooming. Most of the time, layers are not aware of the current zoom factor; this is handled by the compositor. Everything else is computed in page coordinates.
* Better clicking. Clicks are propagated from child layers upwards, and take scroll offsets into account. This also fixes an issue where hit testing was off for retina displays.

Still to do:
* Connect with tikue's changes.
* Figure out snap-to-pixel scrolling. This is a lot trickier than it was before.
* iframes don't have backgrounds yet; they appear transparent until their tiles are rendered.

Depends on:
* servo/rust-opengles#47
* servo/rust-layers#23
* servo/euclid#11

Here's a preview:
![iframe-test](https://f.cloud.github.com/assets/4186321/912968/1c305e02-fe0c-11e2-876d-2333afd27af9.png)
What looks like a compositor bug is actually a separate layer that I hard-coded in. So far it seems to be fully functional.
bors-servo pushed a commit that referenced this pull request Aug 7, 2013
This refactors the compositor and adds initial support for multiple compositor layers being displayed at once. A new module, `compositor_layer.rs`, does most of the heavy lifting that was once in `mod.rs`. New features include:
* Better scrolling. Scrolling is now applied on a per-layer basis. The layer under the cursor is scrolled first; if it cannot be scrolled (i.e. it is at a page boundary), the scroll is propagated to the layer's parents.
* Better zooming. Most of the time, layers are not aware of the current zoom factor; this is handled by the compositor. Everything else is computed in page coordinates.
* Better clicking. Clicks are propagated from child layers upwards, and take scroll offsets into account. This also fixes an issue where hit testing was off for retina displays.

Still to do:
* Connect with tikue's changes.
* Figure out snap-to-pixel scrolling. This is a lot trickier than it was before.
* iframes don't have backgrounds yet; they appear transparent until their tiles are rendered.

Depends on:
* servo/rust-opengles#47
* servo/rust-layers#23
* servo/euclid#11

Here's a preview:
![iframe-test](https://f.cloud.github.com/assets/4186321/912968/1c305e02-fe0c-11e2-876d-2333afd27af9.png)
What looks like a compositor bug is actually a separate layer that I hard-coded in. So far it seems to be fully functional.
@metajack metajack closed this Aug 7, 2013
@metajack metajack reopened this Aug 8, 2013
bors-servo pushed a commit that referenced this pull request Aug 8, 2013
This refactors the compositor and adds initial support for multiple compositor layers being displayed at once. A new module, `compositor_layer.rs`, does most of the heavy lifting that was once in `mod.rs`. New features include:
* Better scrolling. Scrolling is now applied on a per-layer basis. The layer under the cursor is scrolled first; if it cannot be scrolled (i.e. it is at a page boundary), the scroll is propagated to the layer's parents.
* Better zooming. Most of the time, layers are not aware of the current zoom factor; this is handled by the compositor. Everything else is computed in page coordinates.
* Better clicking. Clicks are propagated from child layers upwards, and take scroll offsets into account. This also fixes an issue where hit testing was off for retina displays.

Still to do:
* Connect with tikue's changes.
* Figure out snap-to-pixel scrolling. This is a lot trickier than it was before.
* iframes don't have backgrounds yet; they appear transparent until their tiles are rendered.

Depends on:
* servo/rust-opengles#47
* servo/rust-layers#23
* servo/euclid#11

Here's a preview:
![iframe-test](https://f.cloud.github.com/assets/4186321/912968/1c305e02-fe0c-11e2-876d-2333afd27af9.png)
What looks like a compositor bug is actually a separate layer that I hard-coded in. So far it seems to be fully functional.
@eschweic eschweic closed this Aug 8, 2013
@eschweic eschweic reopened this Aug 8, 2013
@eschweic eschweic closed this Aug 8, 2013
@metajack metajack reopened this Aug 8, 2013
bors-servo pushed a commit that referenced this pull request Aug 8, 2013
This refactors the compositor and adds initial support for multiple compositor layers being displayed at once. A new module, `compositor_layer.rs`, does most of the heavy lifting that was once in `mod.rs`.

--- clipped so bors is happy ---
@eschweic

This comment has been minimized.

Copy link
Owner

eschweic commented on 4b2d7f0 Aug 8, 2013

r=pcwalton

This comment has been minimized.

Copy link

metajack replied Aug 8, 2013

r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 4b2d7f0 Aug 8, 2013

saw approval from metajack
at eschweic@4b2d7f0

This comment has been minimized.

Copy link
Contributor

bors-servo replied Aug 8, 2013

merging eschweic/servo/multi-layer = 4b2d7f0 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Aug 8, 2013

eschweic/servo/multi-layer = 4b2d7f0 merged ok, testing candidate = 0d46164

This comment has been minimized.

Copy link
Contributor

bors-servo replied Aug 8, 2013

fast-forwarding master to auto = 0d46164

bors-servo pushed a commit that referenced this pull request Aug 8, 2013
This refactors the compositor and adds initial support for multiple compositor layers being displayed at once. A new module, `compositor_layer.rs`, does most of the heavy lifting that was once in `mod.rs`.

--- clipped so bors is happy ---
@bors-servo bors-servo closed this Aug 8, 2013
ChrisParis pushed a commit to ChrisParis/servo that referenced this pull request Sep 7, 2014
Remove redundant HTMLVideoElement.audio tests
glennw pushed a commit to glennw/servo that referenced this pull request Jan 16, 2017
Implement HeapSizeOf for ScrollPolicy.

<!-- 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/675)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.