Add a ClearRectangle item type. #1929

mstange · 2017-10-24T22:04:08Z

Fixes #1926.

I haven't extensively tested this yet. This draws ClearRectangle items as Rectangle primitives with opaque black and operator destination out.
I'm not really sure how to optimize the case where we don't need blending, because the color is set on the primitive before we decide how to blend it. But if there's no blending, we need to set the color to transparent black instead of opaque black.

This change is

bors-servo · 2017-10-24T22:16:21Z

☔ The latest upstream changes (presumably #1924) made this pull request unmergeable. Please resolve the merge conflicts.

mstange · 2017-10-25T03:52:44Z

Somehow this change causes my back button's white fill to ignore the rounded clip. Will investigate.

bors-servo · 2017-10-25T13:45:12Z

☔ The latest upstream changes (presumably #1937) made this pull request unmergeable. Please resolve the merge conflicts.

kvark · 2017-10-26T15:59:10Z

webrender/src/border.rs

@@ -385,7 +385,7 @@ impl FrameBuilder {
                self.add_solid_rectangle(
                    clip_and_scroll,
                    &info,
-                    &border.top.color,
+                    &FillOrClear::Fill(border.top.color),


"XxxxOrYyy" pattern sounds a bit awkward to me. Perhaps, we could go with something like Fill::Color versus Fill::Clear ?

kvark · 2017-10-26T15:59:10Z

webrender/src/frame_builder.rs

-            return;
-        }
+        let prim = match operation {
+            &FillOrClear::Fill(ref color) => {


Could also be RectangleContent::Fill/Clear

I went with this suggestion, RectangleContent::Fill/Clear. Thanks!

kvark · 2017-10-26T15:59:10Z

webrender/src/frame_builder.rs

+            &FillOrClear::Fill(ref color) => {
+                // Don't add transparent rectangles to the draw list, but do consider them for hit
+                // testing. This allows specifying invisible hit testing areas.
+                if color.a == 0.0 {


should probably move this condition up into the matching, maybe even turn it into an if/let i.e.

if let &FillOrClear::Fill(ColorF{a: 0.0, ..}) = operation { (something something return) } let prim = RectanglePrimitive { operation: *operation };

mstange · 2017-10-28T03:39:05Z

I've addressed the comments. I added a TODO for the missing optimization but didn't address it. I suppose I could make AlphaRenderItem::add_to_batch (which knows the chosen blend mode, and whether blending is needed) write a piece of information into the data that's passed to the shader, and then I could modify the rectangle shader to respect that piece of information to switch between opaque black and transparency... but that seems awful.

r? @glennw

glennw

A couple of minor changes, otherwise this looks great!

glennw · 2017-10-29T20:40:39Z

webrender/src/frame.rs

+                }
+            }
+            SpecificDisplayItem::ClearRectangle => {
+                if !self.try_to_add_rectangle_splitting_on_clip(


Let's not bother trying to split clear rectangles - we can just directly call add_solid_rectangle here.

glennw · 2017-10-29T20:40:39Z

webrender/src/prim_store.rs

@@ -138,6 +138,7 @@ pub struct PrimitiveMetadata {
    pub cpu_prim_index: SpecificPrimitiveIndex,
    pub gpu_location: GpuCacheHandle,
    pub clip_task_id: Option<RenderTaskId>,
+    pub is_clear: bool,


This should be in the RectanglePrimitive type since it's not common to all primitives.

How do I get access to the RectanglePrimitive inside get_blend_mode?

Something like:

let rect = &self.cpu_rectangles[metadata.cpu_prim_index.0];

should work. The TextRun primitive in the same function does something similar.

bors-servo · 2017-10-30T14:26:51Z

☔ The latest upstream changes (presumably #1954) made this pull request unmergeable. Please resolve the merge conflicts.

mstange · 2017-10-30T17:55:56Z

I've addressed the comments.

mstange · 2017-10-31T00:23:39Z

r? @glennw

glennw · 2017-10-31T00:59:16Z

@bors-servo r+

I think the opacity type for a clear rectangle does work - we can easily change it if required, since this is a new interface.

It would be good to add some wrench reftests as a follow up.

bors-servo · 2017-10-31T00:59:17Z

📌 Commit fb8b6b3 has been approved by glennw

bors-servo · 2017-10-31T02:14:00Z

☔ The latest upstream changes (presumably #1962) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2017-10-31T02:14:01Z

🔒 Merge conflict


        Add a ClearRectangle item type.

mstange · 2017-10-31T02:39:30Z

Rebased. r? @glennw

glennw · 2017-10-31T03:59:59Z

@bors-servo r+

bors-servo · 2017-10-31T04:00:00Z

📌 Commit 52da7ba has been approved by glennw

bors-servo · 2017-10-31T05:05:38Z

⌛ Testing commit 52da7ba with merge c0194de...


        Auto merge of #1929 - mstange:clear-rect, r=glennw

Add a ClearRectangle item type. Fixes #1926. I haven't extensively tested this yet. This draws ClearRectangle items as Rectangle primitives with opaque black and operator destination out. I'm not really sure how to optimize the case where we don't need blending, because the color is set on the primitive before we decide how to blend it. But if there's no blending, we need to set the color to transparent black instead of opaque black.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1929)

bors-servo · 2017-10-31T05:53:09Z

☀️ Test successful - status-appveyor, status-travis
Approved by: glennw
Pushing c0194de to master...

mstange force-pushed the mstange:clear-rect branch 4 times, most recently from fdfac1e to a7af4aa Oct 25, 2017

kvark reviewed Oct 26, 2017

View changes

mstange force-pushed the mstange:clear-rect branch from a7af4aa to 6ce56c5 Oct 28, 2017

mstange changed the title ~~[WIP] Add a ClearRectangle item type.~~ Add a ClearRectangle item type. Oct 28, 2017

glennw requested changes Oct 29, 2017

View changes

mstange force-pushed the mstange:clear-rect branch from 6ce56c5 to fb8b6b3 Oct 30, 2017

glennw approved these changes Oct 31, 2017

View changes

Add a ClearRectangle item type.

Loading status checks…

52da7ba

mstange force-pushed the mstange:clear-rect branch from fb8b6b3 to 52da7ba Oct 31, 2017

servo / webrender

Add a ClearRectangle item type. #1929

Add a ClearRectangle item type. #1929

mstange commented Oct 24, 2017 •

edited by larsbergstrom

bors-servo commented Oct 24, 2017

mstange commented Oct 25, 2017

bors-servo commented Oct 25, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mstange commented Oct 28, 2017

glennw left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors-servo commented Oct 30, 2017

mstange commented Oct 30, 2017

mstange commented Oct 31, 2017

glennw commented Oct 31, 2017

bors-servo commented Oct 31, 2017

bors-servo commented Oct 31, 2017

bors-servo commented Oct 31, 2017

mstange commented Oct 31, 2017

glennw commented Oct 31, 2017

bors-servo commented Oct 31, 2017

bors-servo commented Oct 31, 2017

bors-servo commented Oct 31, 2017

servo / webrender

Join GitHub today

Add a ClearRectangle item type. #1929

Add a ClearRectangle item type. #1929

Conversation

mstange commented Oct 24, 2017 • edited by larsbergstrom

bors-servo commented Oct 24, 2017

mstange commented Oct 25, 2017

bors-servo commented Oct 25, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mstange commented Oct 28, 2017

glennw left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors-servo commented Oct 30, 2017

mstange commented Oct 30, 2017

mstange commented Oct 31, 2017

glennw commented Oct 31, 2017

bors-servo commented Oct 31, 2017

bors-servo commented Oct 31, 2017

bors-servo commented Oct 31, 2017

mstange commented Oct 31, 2017

glennw commented Oct 31, 2017

bors-servo commented Oct 31, 2017

bors-servo commented Oct 31, 2017

bors-servo commented Oct 31, 2017

mstange commented Oct 24, 2017 •

edited by larsbergstrom