Forward component transfer filter to WebRender
Categories
(Core :: Graphics: WebRender, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: jrmuizel, Assigned: tnikkel)
References
(Depends on 1 open bug, Regressed 1 open bug)
Details
Attachments
(12 files, 4 obsolete files)
32.83 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
32.96 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
11.50 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
8.73 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
7.89 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
2.10 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
26.85 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
7.33 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
5.21 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
1.17 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
1.13 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
2.17 KB,
patch
|
gw
:
review+
|
Details | Diff | Splinter Review |
This depends on https://github.com/servo/webrender/issues/3287
Updated•5 years ago
|
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
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.
Reporter | ||
Comment 2•5 years ago
|
||
Can you write a summary of your approach?
Assignee | ||
Comment 3•5 years ago
|
||
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.
Reporter | ||
Comment 4•5 years ago
|
||
Comment on attachment 9036875 [details] [diff] [review] forward component transfer filter data to webrender Review of attachment 9036875 [details] [diff] [review]: ----------------------------------------------------------------- This seems reasonable.
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
Assignee | ||
Comment 6•5 years ago
|
||
Assignee | ||
Comment 7•5 years ago
|
||
Can you direct this review to someone more appropriate if that person is not you?
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
Assignee | ||
Comment 10•5 years ago
|
||
Comment 11•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
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).
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 15•5 years ago
|
||
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?
Assignee | ||
Comment 16•5 years ago
|
||
Assignee | ||
Comment 17•5 years ago
|
||
Assignee | ||
Comment 18•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 19•5 years ago
|
||
Assignee | ||
Comment 20•5 years ago
|
||
Comment 21•5 years ago
|
||
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•5 years ago
|
||
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.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 23•5 years ago
|
||
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.
Updated•5 years ago
|
Comment 24•5 years ago
|
||
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•5 years ago
|
||
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!
Assignee | ||
Comment 26•5 years ago
|
||
(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•5 years ago
|
||
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)?
Assignee | ||
Comment 28•5 years ago
|
||
This diff works. Seems far too easy. Maybe I'm missing something.
Comment 29•5 years ago
|
||
Comment on attachment 9045572 [details] [diff] [review] don't add PartialEq, Hash, Eq to interning structures Review of attachment 9045572 [details] [diff] [review]: ----------------------------------------------------------------- Nice!
Assignee | ||
Comment 30•5 years ago
|
||
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•5 years ago
|
||
Yup, ship it!
Updated•5 years ago
|
Assignee | ||
Comment 32•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 33•5 years ago
|
||
PictureKey contains a Picture so we need to increase it's size expectation too.
Updated•5 years ago
|
Assignee | ||
Comment 34•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
Pushed by tnikkel@gmail.com: 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•5 years ago
|
||
Backed out 10 changesets (Bug 1505871) for wrench bustages CLOSED TREE
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•5 years ago
|
||
Pushed by tnikkel@gmail.com: 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•5 years ago
|
||
Pushed by tnikkel@gmail.com: 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.
Assignee | ||
Comment 40•5 years ago
|
||
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•5 years ago
|
||
bugherder |
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
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•4 years ago
|
Description
•