Skip to content

Sync changes from mozilla-central #3573

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

Merged
merged 33 commits into from
Mar 2, 2019
Merged

Conversation

moz-gfx
Copy link

@moz-gfx moz-gfx commented Feb 28, 2019

This change is Reviewable

kvark and others added 24 commits February 28, 2019 11:19
…display list bitstream. r=jrmuizel

The format for stacking contexts in the built display list goes from

PushStackingContext item
push_iter of Vec<FilterOp>

to

SetFilterOps item
push_iter of Vec<FilterOp>
1st SetFilterData item
push_iter of array of func types
push_iter funcR values
push_iter funcG values
push_iter funcB values
push_iter funcA values
.
.
.
nth SetFilterData item
push_iter of array of func types
push_iter funcR values
push_iter funcG values
push_iter funcB values
push_iter funcA values
PushStackingContext item

We need separate a SetFilterData item for each filter because we can't push_iter a variable sized thing.

When we iterate over the built display list to flatten it we work similarly to how gradients work with a SetGradientStops item before the actual gradient item. So when we see SetFilterOps or SetFilterData we use them to fill out values on the built display list iterator but don't those items return them to the iterator user and instead continue iterating until we hit the PushStackingContext item, at which point to the iterator consumer it appears as those the FilterOps and FilterDatas were on the PushStackingContext item. (This part is trickier too since we need a TempFilterData type that just holds ItemRange's until we get the actual bytes later.)

Do we need to clear cur_filters and cur_filter_data at some point to prevent them from getting ready by items for which they do not apply?

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/90660632d6410ebb7e3530c048f04137d36b6587
…pare_for_render. r=gw

We need to call update on the filter data handle which needs to use frame_state (in a later patch), so we can't borrow across that call.

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/1206e1a32e5fe2e4a329be1aef9330f7e632c91a
…display list bitstream. r=jrmuizel

The format for stacking contexts in the built display list goes from

PushStackingContext item
push_iter of Vec<FilterOp>

to

SetFilterOps item
push_iter of Vec<FilterOp>
1st SetFilterData item
push_iter of array of func types
push_iter funcR values
push_iter funcG values
push_iter funcB values
push_iter funcA values
.
.
.
nth SetFilterData item
push_iter of array of func types
push_iter funcR values
push_iter funcG values
push_iter funcB values
push_iter funcA values
PushStackingContext item

We need separate a SetFilterData item for each filter because we can't push_iter a variable sized thing.

When we iterate over the built display list to flatten it we work similarly to how gradients work with a SetGradientStops item before the actual gradient item. So when we see SetFilterOps or SetFilterData we use them to fill out values on the built display list iterator but don't those items return them to the iterator user and instead continue iterating until we hit the PushStackingContext item, at which point to the iterator consumer it appears as those the FilterOps and FilterDatas were on the PushStackingContext item. (This part is trickier too since we need a TempFilterData type that just holds ItemRange's until we get the actual bytes later.)

Do we need to clear cur_filters and cur_filter_data at some point to prevent them from getting ready by items for which they do not apply?

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/f499cb201bd6afae94acabdf5ca5fb1afa998a91
…pare_for_render. r=gw

We need to call update on the filter data handle which needs to use frame_state (in a later patch), so we can't borrow across that call.

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/e1f278d901a765d0951f156a73a8dd3fad01a101
… component transfer. r=gw

For table/discrete we just create a lookup table for all 256 possible input values. We should probably switch to just computing the value in the shader, unless the list of value is really long.

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/457a27c7b181799671e43b99becb2420d03cdf62
…. r=gw

On Windows the vFuncs array is always 0 in the fragment shader. If we move the computation of the vFuncs array outside of the switch (so that it is computed for every type of shader, even when it is not needed) then it works.

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/55ba29bf61f031b82f94a76e18f2f769ac0b264e
angle_shader_validation.rs just checks that the number of "switch" and "default:" are the same number in the source file, even if they occur in comments.

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/30f9e207c3d4dd83247792c3806668e09273cf36
…items r=gw

Document splitting is crashing with early initialization of the debug
renderer. Not sure why, and this is just a temporary workaround, but
one that I think we want anyway, as we don't want to be unnecessarily
lazy-initting the debug renderer.

Depends on D20698

Differential Revision: https://phabricator.services.mozilla.com/D20700

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/deb6826900252a703b5211a6dbd29cad21b009e3
Change the external scroll offset to be a vector, rather than a
point. This can also be updated gecko-side in future, but for
now is converted to a vector at the API boundary.

Also plumb through the external scroll offset so that it is stored
inside the ScrollFrameInfo in a spatial node. This will allow
modifying the transforms that the clip-scroll tree creates to
take into account the external scroll offset in future.

Differential Revision: https://phabricator.services.mozilla.com/D21319

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/9b9bbabd8f1004e623c09a73c392f0063e24ff9d
Backed out changeset 82cfcc2f5fac (bug 1529117)
Backed out changeset d9fd8225a95f (bug 1529117)

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/d819234834aad12e3388ceb0b09dfb6c8cdd3da9
@moz-gfx
Copy link
Author

moz-gfx commented Feb 28, 2019

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 84c218f has been approved by moz-gfx

@bors-servo
Copy link
Contributor

⌛ Testing commit 84c218f with merge ad8ae02...

bors-servo pushed a commit that referenced this pull request Feb 28, 2019
Sync changes from mozilla-central

<!-- 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/3573)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-taskcluster

@staktrace
Copy link
Contributor

Looks like rustc 1.33 decided window was unused even though it's used a couple of lines down. rustc 1.32 is perfectly happy with it. Woohoo.

@nox
Copy link
Contributor

nox commented Feb 28, 2019

@nox
Copy link
Contributor

nox commented Feb 28, 2019

Note that I'm just stating the reason why it does that here, I don't think it's a good idea, as I've also used empty enums that way in other circumstances and that code probably warns now, I think an issue should be filed on rust-lang/rust.

@staktrace
Copy link
Contributor

For now I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1531512 to fix this on the WR side. I don't plan on filing an issue against rust-lang because I don't have a strong opinion about this, but feel free to do so if you think rustc should change behaviour.

squarewave and others added 2 commits March 1, 2019 06:43
This is a stab at what the correct approach to this should be. It
seems that we should be using the window size here and not the
screen_rect, as the screen_rect is not used to offset what we
normally draw, but instead generally for scissoring(?). The end
result is if we use an offset screen_rect, we end up applying
the offset of the chrome area twice, once because the document's
screen rect is offset, and once because of the tile.world_rect
offset.

Depends on D20696

Differential Revision: https://phabricator.services.mozilla.com/D20698

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/48bf7d6e2ea7be2eff20cafdbb4c79dfd3706e8e
…le devices. r=kvark

This patch fixes some wasted GPU time on mobile devices due to
redundant resolve / copy steps.

In the first case, we would previously do:
 - Global clear of color / depth on main framebuffer.
 - Bind and draw off-screen targets.
 - Bind main framebuffer and draw scene.

Between step 1 and 2, a resolve step is triggered on tiled GPU
drivers, wasting a lot of GPU time. To fix this, the clear is
now deferred until the framebuffer of the first document is
drawn. This does slightly change the semantics of how WR does
clear operations, but I think it works fine and makes more sense.

In the second case, we would previously do:
 - ...
 - Draw main framebuffer
 - End frame and invalidate the contents of input textures.
 - Bind main framebuffer and draw debug overlay.

This also introduces an extra resolve / copy step, even if the
debug overlay is not enabled. To fix this, the invalidation step
of the input textures to the main framebuffer pass is deferred
until all drawing is complete on the main framebuffer, by doing
the invalidation in the end_frame() call of the texture resolver.

Together, these save a very significant amount of ms per frame
in GPU time on the mobile devices I tested.

Differential Revision: https://phabricator.services.mozilla.com/D21490

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/a901d60873b90ab874bfb0a741ca128c4e44a667
@moz-gfx
Copy link
Author

moz-gfx commented Mar 1, 2019

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 34db10a has been approved by moz-gfx

@bors-servo
Copy link
Contributor

⌛ Testing commit 34db10a with merge b444aa0...

bors-servo pushed a commit that referenced this pull request Mar 1, 2019
Sync changes from mozilla-central

<!-- 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/3573)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-taskcluster

@nox
Copy link
Contributor

nox commented Mar 1, 2019

rust-lang/rust#58844

staktrace and others added 7 commits March 2, 2019 04:54
…w,nical

The goal of this change was to simplify the semantics of our document placement and split the logical elements inside (display list) from the actual screen rectangle occupied by a document.
To achieve that, we introduce the framebuffer space for things Y-flipped on screen.
We fix the frame outputs, so that they get produced on the first frame without loopback from the frame building to scene building.

Differential Revision: https://phabricator.services.mozilla.com/D21641

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/93f7dc3084a1350e5c2c21d599ec6634ebe0ec8f
example error
ERROR 2019-03-01T15:23:27Z: webrender::device::gl: (error) GL_INVALID_ENUM error generated. Invalid primitive mode.
thread 'main' panicked at 'Caught GL error 500 at 'draw_elements_instanced'', webrender/src/device/gl.rs:1098:17
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Differential Revision: https://phabricator.services.mozilla.com/D21701

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/43079c556af961ab8f7f34380360d2cbbef330a3
…w,nical

The goal of this change was to simplify the semantics of our document placement and split the logical elements inside (display list) from the actual screen rectangle occupied by a document.
To achieve that, we introduce the framebuffer space for things Y-flipped on screen.
We fix the frame outputs, so that they get produced on the first frame without loopback from the frame building to scene building.

Differential Revision: https://phabricator.services.mozilla.com/D21641

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/ee88f4e35d4fd18ee954609185445c3fd7e1cecd
@moz-gfx
Copy link
Author

moz-gfx commented Mar 2, 2019

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit d33e637 has been approved by moz-gfx

@bors-servo
Copy link
Contributor

⌛ Testing commit d33e637 with merge be6624b...

bors-servo pushed a commit that referenced this pull request Mar 2, 2019
Sync changes from mozilla-central

<!-- 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/3573)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: moz-gfx
Pushing be6624b to master...

@bors-servo bors-servo merged commit d33e637 into servo:master Mar 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.