1505871 - Forward component transfer filter to WebRender

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

This depends on https://github.com/servo/webrender/issues/3287

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: forward component transfer filter data to webrender

This just gets the data into webrender, it was a huge headache. Still need to actually write shaders to do the work.

Check point to see if I've done something horribly wrong.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Can you write a summary of your approach?

Comment 3

[Timothy Nikkel (:tnikkel)]

We use a WrFilterData on the C++ side to hold the filter values (and function types), it's just a collection of pointer/size pairs for the values. We change the thing we pass around in C++ from nsTArray<FilterOp> to WrFilterHolder which contains a nsTArray<FilterOp>, nsTArray<WrFilterData>, and a nsTArray whose sole purpose is to own the memory for the values in the nsTArray<WrFilterData>.

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.)

This gets the data all the way to where we construct the PictureCompositeKey. I'm not sure if we can just stick all these vecs on the PictureCompositeKey or not because I haven't looked at how it is used yet.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9036875 [details] [diff] [review]
forward component transfer filter data to webrender

Review of attachment 9036875 [details] [diff] [review]:
-----------------------------------------------------------------

This seems reasonable.

Comment 5

[Timothy Nikkel (:tnikkel)]

Attachment: C++ code to get component transfer filter data into webrender

Comment 6

[Timothy Nikkel (:tnikkel)]

Attachment: webrender displaylist stuff

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: Implement yaml reader/writer for component transfer

Can you direct this review to someone more appropriate if that person is not you?

Comment 8

[Timothy Nikkel (:tnikkel)]

Attachment: Make PictureCompositeMode not derive Copy

Comment 9

[Timothy Nikkel (:tnikkel)]

Attachment: Add component transfer to PictureCompositeMode/PictureCompositeKey

Comment 10

[Timothy Nikkel (:tnikkel)]

Attachment: Implement the shader and gpu cache part

Comment 11

[Glenn Watson [:gw]]

Comment on attachment 9041722 [details] [diff] [review]
Make PictureCompositeMode not derive Copy

Review of attachment 9041722 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/wr/webrender/src/display_list_flattener.rs
@@ +1379,5 @@
>          // Add picture for this actual stacking context contents to render into.
>          let leaf_pic_index = PictureIndex(self.prim_store.pictures
>              .alloc()
>              .init(PicturePrimitive::new_image(
> +                leaf_composite_mode.clone(),

I guess the reason for making this non-Copy is because there is now a Vec stored in the composite mode enum, is that right?

I'll leave some comments in the other patches with a possible alternative way to store the data. If that works / makes sense, we could keep this enum as Copy, and remove these clone calls.

Also, I think in at least some of these cases in this patch, the clone call should not be required - but that may not be relevant if we change how the filter data is stored.

Comment 12

[Glenn Watson [:gw]]

Comment on attachment 9041724 [details] [diff] [review]
Implement the shader and gpu cache part

Review of attachment 9041724 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/wr/webrender/res/brush_blend.glsl
@@ +195,5 @@
> +
> +            vec4 colora = alpha != 0.0 ? Cs / alpha : Cs;
> +            for (int i = 0; i < 4; i++) {
> +                switch (vFuncs[i]) {
> +                    case 0:

Instead of literals here, let's use a #define to specify the identity, table etc values.

@@ +200,5 @@
> +                        // Identity
> +                        break;
> +                    case 1: // Table
> +                    case 2: // Discrete
> +                        k = int(floor(colora[i]*255.0));

This logic isn't quite clear to me - could we add some comments on the logic here? (or it might become clear as I read further).

::: gfx/wr/webrender/src/batch.rs
@@ +1426,5 @@
>                                          );
>                                      }
>                                  }
>                              }
> +                            PictureCompositeMode::ComponentTransferFilter(ref filter_data) => {

I'm not sure about having this as a separate enum variant - can it just be part of PictureCompositeMode::Filter? We already differentiate in the batching match statement on some specific filter types (e.g. blur and drop shadow).

@@ +1435,5 @@
> +                                    .surface
> +                                    .as_ref()
> +                                    .expect("bug: surface must be allocated by now");
> +
> +                                let filter_mode : i32 = 13 |

Let's avoid using a literal here - there's probably an existing symbol for this?

::: gfx/wr/webrender/src/picture.rs
@@ +2982,5 @@
> +                    push_component_transfer_data(&filter_data.func_r_type, &filter_data.r_values, &mut request);
> +                    push_component_transfer_data(&filter_data.func_g_type, &filter_data.g_values, &mut request);
> +                    push_component_transfer_data(&filter_data.func_b_type, &filter_data.b_values, &mut request);
> +                    push_component_transfer_data(&filter_data.func_a_type, &filter_data.a_values, &mut request);
> +                    if filter_data.r_values.len() + filter_data.g_values.len() +

The intent might be clearer by using .is_empty() && ..

@@ +2985,5 @@
> +                    push_component_transfer_data(&filter_data.func_a_type, &filter_data.a_values, &mut request);
> +                    if filter_data.r_values.len() + filter_data.g_values.len() +
> +                       filter_data.b_values.len() + filter_data.a_values.len() == 0 {
> +                        // we have to push something, so push a dummy value we will never look at
> +                        request.push([0.0, 0.0, 0.0, 0.0]);

We might be able to avoid this by skipping creation of this composite mode filter much earlier in the flattening step, if the filter data is empty?

@@ +3051,5 @@
>  }
>  
> +fn push_component_transfer_data(
> +    func_type: &ComponentTransferFuncType,
> +    values: &Vec<f32>,

I think this can be &[f32] instead of a Vec

@@ +3054,5 @@
> +    func_type: &ComponentTransferFuncType,
> +    values: &Vec<f32>,
> +    request: &mut GpuDataRequest,
> +) {
> +    if values.len() == 0 {

is_empty()

@@ +3069,5 @@
> +                    if (values.len() == 1) || (i == 63 && j == 3) {
> +                        arr[j] = values[values.len()-1];
> +                    } else {
> +                        let c = ((4*i + j) as f32)/255.0;
> +                        match func_type {

Instead of this nested match, perhaps we can move this common code into a function, and pass a closure, or something similar to that.

::: gfx/wr/webrender_api/src/display_item.rs
@@ +681,5 @@
>      pub a_values: Vec<f32>,
>  }
>  
> +fn sanitize_func_type(
> +    func_type: &ComponentTransferFuncType,

Probably can just pass this by value

@@ +682,5 @@
>  }
>  
> +fn sanitize_func_type(
> +    func_type: &ComponentTransferFuncType,
> +    values: &Vec<f32>,

&[f32]

@@ +684,5 @@
> +fn sanitize_func_type(
> +    func_type: &ComponentTransferFuncType,
> +    values: &Vec<f32>,
> +) -> ComponentTransferFuncType {
> +    if values.len() == 0 {

is_empty()

@@ +693,5 @@
> +    }
> +    if values.len() < 3 && *func_type == ComponentTransferFuncType::Gamma {
> +        return ComponentTransferFuncType::Identity;
> +    }
> +    return *func_type;

No need for a return here - can just be *func_type

@@ +696,5 @@
> +    }
> +    return *func_type;
> +}
> +
> +fn sanitize_values(

Maybe this could just return a bool or something like that if it's valid, to avoid cloning the heap allocated vec.

Comment 13

[Glenn Watson [:gw]]

Comment on attachment 9041723 [details] [diff] [review]
Add component transfer to PictureCompositeMode/PictureCompositeKey

Review of attachment 9041723 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/wr/webrender/src/display_list_flattener.rs
@@ +1474,3 @@
>              let filter = filter.sanitize();
> +            let composite_mode = match filter {
> +                FilterOp::ComponentTransfer => {

This loop can probably be simplified quite a bit if we're able to use the interning logic mentioned below.

::: gfx/wr/webrender/src/picture.rs
@@ +1804,3 @@
>      Filter(FilterOp),
> +    /// Apply a component transfer filter.
> +    ComponentTransferFilter(FilterData),

I haven't totally thought this through, but I think there might be a better way to structure the way we pass FilterData through WR, which would resolve the need to have a Vec in the PictureCompositeMode enums etc.

We have this interning infrastructure, which we use for primitives and clips. It allows us to store some arbitrary amount of data, and then get a unique handle for it that identifies the content. If we were to introduce an extra interner for storing the filter data, then the PictureCompositeMode enum would effectively just store a handle to that interned filter data. An added advantage of this, if it works, is that the GPU cache handle for the table data would remain across new new display lists, which would save us invalidating and uploading GPU cache data for the filter each time a new display list is received.

Comment 14

[Glenn Watson [:gw]]

It'd be great to add a small number of wrench reftests for this functionality too (unless I missed that in one of the patches I didn't look at).

Comment 15

[Timothy Nikkel (:tnikkel)]

Comment on attachment 9041724 [details] [diff] [review]
Implement the shader and gpu cache part

Review of attachment 9041724 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/wr/webrender/src/batch.rs
@@ +1426,5 @@
>                                          );
>                                      }
>                                  }
>                              }
> +                            PictureCompositeMode::ComponentTransferFilter(ref filter_data) => {

If we make it part of PictureCompositeMode::Filter then that means we need to store whatever data we need in a FilterOp, but FilterOp is shared with C++ (it didn't use to be so we could go back to that), and I don't think we can put the required rust data into a struct that is shared with C++.

@@ +1435,5 @@
> +                                    .surface
> +                                    .as_ref()
> +                                    .expect("bug: surface must be allocated by now");
> +
> +                                let filter_mode : i32 = 13 |

This is what all the filters do in the general filter arm of the match just before this, so I just went along with that.

::: gfx/wr/webrender/src/picture.rs
@@ +2985,5 @@
> +                    push_component_transfer_data(&filter_data.func_a_type, &filter_data.a_values, &mut request);
> +                    if filter_data.r_values.len() + filter_data.g_values.len() +
> +                       filter_data.b_values.len() + filter_data.a_values.len() == 0 {
> +                        // we have to push something, so push a dummy value we will never look at
> +                        request.push([0.0, 0.0, 0.0, 0.0]);

I changed it so we avoid it much earlier now.

@@ +3069,5 @@
> +                    if (values.len() == 1) || (i == 63 && j == 3) {
> +                        arr[j] = values[values.len()-1];
> +                    } else {
> +                        let c = ((4*i + j) as f32)/255.0;
> +                        match func_type {

I thought about this, we'd still need an if at least to determine which closure to use. And separating it out into a separate function seemed like it would make the code harder to understand. I can try again if you have an idea that might work.

::: gfx/wr/webrender_api/src/display_item.rs
@@ +696,5 @@
> +    }
> +    return *func_type;
> +}
> +
> +fn sanitize_values(

I tried getting that to work, the FilterData that we need to sanitize comes out of a vector, so moving out of that doesn't work. I'm probably missing something about how to do this properly in rust?

Comment 16

[Timothy Nikkel (:tnikkel)]

Attachment: Add interning structure

Comment 17

[Timothy Nikkel (:tnikkel)]

Attachment: Don't borrow surfaces

Comment 18

[Timothy Nikkel (:tnikkel)]

Attachment: Pass DataStore

Comment 19

[Timothy Nikkel (:tnikkel)]

Attachment: Putting data into the gpu cache

Comment 20

[Timothy Nikkel (:tnikkel)]

Attachment: the shader

Comment 21

[Glenn Watson [:gw]]

Comment on attachment 9045161 [details] [diff] [review]
Add interning structure

Review of attachment 9045161 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/wr/webrender/src/filterdata.rs
@@ +16,5 @@
> +#[cfg_attr(feature = "capture", derive(Serialize))]
> +#[cfg_attr(feature = "replay", derive(Deserialize))]
> +pub enum SFilterDataComponent {
> +    Identity,
> +    Table(Vec<f32>),

In future, we might want to store these as quantized fixed point values. This would give better cache hits in the interner if values are slightly different due to FP accuracy, and would also mean there's no need for a manual Hash impl below. But this can be done later on, no need to do it now.

@@ +63,5 @@
> +#[derive(Debug, Clone, MallocSizeOf, PartialEq, Eq, Hash)]
> +#[cfg_attr(feature = "capture", derive(Serialize))]
> +#[cfg_attr(feature = "replay", derive(Deserialize))]
> +pub struct SFilterData {
> +    pub r_func : SFilterDataComponent,

nit: extra space after r_func

::: gfx/wr/webrender/src/intern.rs
@@ +78,5 @@
>  }
>  
>  #[cfg_attr(feature = "capture", derive(Serialize))]
>  #[cfg_attr(feature = "replay", derive(Deserialize))]
> +#[derive(Debug, Copy, Clone, MallocSizeOf, PartialEq, Hash, Eq)]

Why do we need to add Hash/Eq to this?

Comment 22

[Glenn Watson [:gw]]

Comment on attachment 9045161 [details] [diff] [review]
Add interning structure

Review of attachment 9045161 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good overall - just a concern over the Eq/Hash on the Handle type.

Comment 23

[Glenn Watson [:gw]]

Comment on attachment 9045165 [details] [diff] [review]
Putting data into the gpu cache

Review of attachment 9045165 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/wr/webrender/src/batch.rs
@@ +1463,5 @@
> +                                    .expect("bug: surface must be allocated by now");
> +
> +
> +                                let filter_data = &ctx.data_stores.filterdata[handle];
> +                                let filter_mode : i32 = 13 |

Maybe add a comment describing where the 13 comes from. In future, we could probably extract that out into a separate fn so it's all in one place.

::: gfx/wr/webrender/src/display_list_flattener.rs
@@ +1466,5 @@
>          }
>  
>          // For each filter, create a new image with that composite mode.
> +        let mut j = 0;
> +        for i in 0 .. stacking_context.composite_ops.filters.len() {

We probably don't need to loop by integer here - can probably be:

for filter in &stacking_context.composite_ops.filters {

@@ +1474,5 @@
> +            let composite_mode = match filter {
> +                FilterOp::ComponentTransfer => {
> +                    let filter_data = &stacking_context.composite_ops.filter_datas[j];
> +                    let filter_data = filter_data.sanitize();
> +                    j = j + 1;

nit: Maybe rename j to be current_filter_data_index or similar.

@@ +1501,5 @@
> +                }
> +                _ => Some(PictureCompositeMode::Filter(filter)),
> +            };
> +
> +            if composite_mode == None {

Instead of checking for None here, we could restructure this slightly so that instead of None if identity, the continue is placed there. Then the _ branch won't need the Some(..).

::: gfx/wr/webrender/src/filterdata.rs
@@ +147,5 @@
> +fn push_component_transfer_data(
> +    func_comp: &SFilterDataComponent,
> +    request: &mut GpuDataRequest,
> +) {
> +    if *func_comp == SFilterDataComponent::Identity {

nit: remove this if and place the return in the unreachable!() block below.

::: gfx/wr/webrender/src/picture.rs
@@ +1892,3 @@
>      Filter(FilterOp),
> +    /// Apply a component transfer filter.
> +    ComponentTransferFilter(FilterDataHandle),

It's still not totally clear to me why this is separate from being inside the FilterOp enum. But it's not a big deal - we could land as-is and consider changing this as a follow up.

Comment 24

[Glenn Watson [:gw]]

Comment on attachment 9045166 [details] [diff] [review]
the shader

Review of attachment 9045166 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/wr/webrender/res/brush_blend.glsl
@@ +3,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #define VECS_PER_SPECIFIC_BRUSH 3
>  
> +#define IDENTITY 0

nit: maybe prefix these with COMPONENT_TRANSFER_ or similar?

Comment 25

[Glenn Watson [:gw]]

Looks good! There are a few nits / questions, but most of them can be left and handled as follow ups in the future. My only real question is the Hash/Eq on the Handle type - I don't this is needed? It'd also be great to add a few wrench reftests of this functionality, although again, that can be done as a follow up. Thanks!

Comment 26

[Timothy Nikkel (:tnikkel)]

(In reply to Glenn Watson [:gw] from comment #21)

::: gfx/wr/webrender/src/intern.rs
@@ +78,5 @@

}

#[cfg_attr(feature = "capture", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
+#[derive(Debug, Copy, Clone, MallocSizeOf, PartialEq, Hash, Eq)]

Why do we need to add Hash/Eq to this?

We put FilterDataHandle (== intern_types::filterdata::Handle) into the PictureCompositeKey enum and the PictureCompositeMode enum. PictureCompositeMode has PartialEq, and PictureCompositeKey has PartialEq, Hash, Eq. So we need PartialEq, Hash, Eq on Handle and Epoch since it's in Handle. Is there a better way to do it?

Comment 27

[Glenn Watson [:gw]]

Yup, at least in theory. If what I've written below doesn't make sense / or doesn't seem easy to make work, let me know and I'll take a closer look or we can discuss the details on IRC.

The current problem is roughly that:

The PictureCompositeKey is hashable so that we can identify pictures that have the same content / filters, for purposes of invalidating cached tiles etc. If we store intern::Handle in that, the cache key will often change, because intern::Handle has items like an epoch in it, that get updated per-frame.

What we ideally want to do (and I'm not sure if there's gotchas here) is:

• Store the unique identifier of the filter in PictureCompositeKey. We can do this by calling handle.uid(). This returns the constant part of an interned handle that uniquely identifies the interned content. The ItemUid returned by that method implements Hash and Eq, so that it can be used directly in PictureCompositeKey.

• It should be fine to store the full handle in PictureCompositeMode enum, I think. This enum doesn't need to be hashed, so it doesn't have those strict requirements. The current PictureCompositeMode enum does require PartialEq, but I just tested locally and that's not actually needed (it compiles if I remove it). So hopefully, if you remove PartialEq from PictureCompositeMode, you should be able to store the intern::Handle in there.

The part that I'm a bit uncertain on is whether the first change above might mean that the complete handle is not currently available where/when the PictureCompositeMode enum is constructed. If that's the case, we might be able to fix that by using the extra scene data struct that's available for interner use (which is currently a () in your patch)?

Comment 28

[Timothy Nikkel (:tnikkel)]

Attachment: don't add PartialEq, Hash, Eq to interning structures

This diff works. Seems far too easy. Maybe I'm missing something.

Comment 29

[Glenn Watson [:gw]]

Comment on attachment 9045572 [details] [diff] [review]
don't add PartialEq, Hash, Eq to interning structures

Review of attachment 9045572 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

Comment 30

[Timothy Nikkel (:tnikkel)]

Comment on attachment 9045161 [details] [diff] [review]
Add interning structure

So if I combine this with the last patch undoing the PartialEq, Eq, Hash from the interning structures, are you okay with this?

Comment 31

[Glenn Watson [:gw]]

Yup, ship it!

Comment 32

[Timothy Nikkel (:tnikkel)]

Attachment: increase Picture size expectation

Comment 33

[Timothy Nikkel (:tnikkel)]

Attachment: increase PictureKey size

PictureKey contains a Picture so we need to increase it's size expectation too.

Comment 34

[Timothy Nikkel (:tnikkel)]

Attachment: compute component transfer function type always

For some reason this fixes the wrench reftest on Windows (works fine without this change on linux/mac). I don't understand it. Is this sane?

Comment 35

[Glenn Watson [:gw]]

Comment on attachment 9046228 [details] [diff] [review]
compute component transfer function type always

Review of attachment 9046228 [details] [diff] [review]:
-----------------------------------------------------------------

It seems like there's two possibilities here:

1) We're hitting a driver / ANGLE shader compiler bug. If we can determine that's the case, landing it like this is fine - but we'd want to add a comment to the shader describing why the code is like this, so someone doesn't change it. The comment should reference an entry that gets added in https://github.com/servo/webrender/wiki/Driver-issues.

2) Maybe there's a bug in the shifting / values of the funcs that get ORed in to the Op that is only showing up on Windows. It seems unlikely, but perhaps worth logging out of the CPU code the exact int values that are being passed in batch.rs, just to confirm they are as you expect?

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9578d20e54e
C++ code to get component transfer filter data into webrender. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/1899600a7985
Write component transfer filter data into the webrender display list bitstream. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fd8742fa662
Implement yaml reader/writer for component transfer. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ad20d485eca
Implement the necessary things for interning of filter data. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fff213d97e3
Don't borrow frame_state.surface for the duration of prepare_for_render. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cb8fb01e77e
Pass DataStores to prepare_for_render. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/0007feaf988d
Implement putting the required data in the gpu cache for component transfer. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/9be871042749
Implementation of shaders. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/6486435a048d
Work around a suspected shader miscompilation on Windows. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/045ab0ec3613
Add a wrench reftest.

Comment 37

[Noemi Erli[:noemi_erli]]

Backed out 10 changesets (Bug 1505871) for wrench bustages CLOSED TREE

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=045ab0ec361396705590c5bbcc93e828481ae4e3&selectedJob=230454693

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=230454693&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/3c030119c0dcf447dd1afe7165b2d6846a0bc7f0

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bf33f573505
C++ code to get component transfer filter data into webrender. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/90660632d641
Write component transfer filter data into the webrender display list bitstream. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/def812790f5c
Implement yaml reader/writer for component transfer. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/11c9d5a64a30
Implement the necessary things for interning of filter data. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/1206e1a32e5f
Don't borrow frame_state.surface for the duration of prepare_for_render. r=gw

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdcc424f1da3
C++ code to get component transfer filter data into webrender. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/f499cb201bd6
Write component transfer filter data into the webrender display list bitstream. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc60982f2828
Implement yaml reader/writer for component transfer. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/30cb5423d03f
Implement the necessary things for interning of filter data. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1f278d901a7
Don't borrow frame_state.surface for the duration of prepare_for_render. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/461c14f8a14c
Pass DataStores to prepare_for_render. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/457a27c7b181
Implement putting the required data in the gpu cache for component transfer. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/1562b43107f2
Implementation of shaders. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/55ba29bf61f0
Work around a suspected shader miscompilation on Windows. r=gw
https://hg.mozilla.org/integration/mozilla-inbound/rev/30f9e207c3d4
Fix comment to not trigger angle_shader_validation.rs.
https://hg.mozilla.org/integration/mozilla-inbound/rev/44192297e3ae
Add a wrench reftest.

Comment 40

[Timothy Nikkel (:tnikkel)]

The original failure was because there is a test to make sure shaders contain a default case for all switch statements. The test isn't smart and it just string matches "switch" and "default:" even in comments, since I used the word switch in a comment this caused it to fail.

Comment 41

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/cdcc424f1da3
https://hg.mozilla.org/mozilla-central/rev/f499cb201bd6
https://hg.mozilla.org/mozilla-central/rev/bc60982f2828
https://hg.mozilla.org/mozilla-central/rev/30cb5423d03f
https://hg.mozilla.org/mozilla-central/rev/e1f278d901a7
https://hg.mozilla.org/mozilla-central/rev/461c14f8a14c
https://hg.mozilla.org/mozilla-central/rev/457a27c7b181
https://hg.mozilla.org/mozilla-central/rev/1562b43107f2
https://hg.mozilla.org/mozilla-central/rev/55ba29bf61f0
https://hg.mozilla.org/mozilla-central/rev/30f9e207c3d4
https://hg.mozilla.org/mozilla-central/rev/44192297e3ae