Use more WebRender types in gfx/display_list. #19782

pyfisch · 2018-01-16T10:01:17Z

Use more WebRender types in gfx/display_list.
This uses floating-point (Layout) coordinates in where possible.
Replace NormalBorder struct with WebRender equivalent.
Remove ToPointF and ToRectF traits.
Convert border RepeatKeyword with ToLayout.
Add some definitions to malloc_size_of for WebRender types.

This change is

highfive · 2018-01-16T10:01:22Z

Heads up! This PR modifies the following files:

@emilio: components/layout/display_list/webrender_helpers.rs, components/layout/display_list/builder.rs, components/layout/display_list/conversions.rs, components/layout/display_list/background.rs

highfive · 2018-01-16T10:01:23Z

Warning

These commits modify gfx and layout code, but no tests are modified. Please consider adding a test!

mrobinson

This is great work! I'm really glad to see us moving iteratively toward building a WebRender display list directly. In general, this looks very good to me, but I have a few comments on some minor details that would be good to change.

mrobinson · 2018-01-16T10:17:11Z

components/gfx/display_list/mod.rs

@@ -423,7 +423,11 @@ impl BaseDisplayItem {
                node: OpaqueNode(0),
                pointing: None,
            },
-            local_clip: LocalClip::from(max_rect().to_rectf()),
+            // Create a rectangle of maximal size.
+            local_clip: LocalClip::from(LayoutRect::new(


It probably makes sense to implement max_rect for LayoutRect if we can. Perhaps this can be a trait.

There is a trait MaxRect in webrender but I can't use it from servo.

Maybe this trait should instead live in euclid?

I think it makes sense to at least copy the trait from WebRender and replace max_rect with it.

mrobinson · 2018-01-16T10:17:11Z

components/gfx/display_list/mod.rs


    /// The algorithm we should use to stretch the image. See `image_rendering` in CSS-IMAGES-3 §
    /// 5.3.
-    pub image_rendering: image_rendering::T,
+    pub image_rendering: webrender_api::ImageRendering,


We are importing the other webrender_api types directly in this file, so we should probably be consistent here and import ImageRendering directly as well.

mrobinson · 2018-01-16T10:17:11Z

components/gfx/display_list/mod.rs

-    /// True if gradient repeats infinitly.
-    pub repeating: bool,
+    /// Whether the gradient is repeated or clamped.
+    pub extend: webrender_api::ExtendMode,


Same comment here and other places in the file as well. I would also call this extend_mode in order to make the code more readable.

mrobinson · 2018-01-16T10:17:11Z

components/layout/display_list/background.rs

@@ -370,6 +370,14 @@ fn convert_gradient_stops(gradient_items: &[GradientItem], total_length: Au) ->
    stops
 }

+fn as_gradient_extend(repeating: bool) -> ExtendMode {


Probably better to call this as as_gradient_extend_mode since, in my opinion, it makes it more obvious what this function does.

mrobinson · 2018-01-16T10:17:11Z

components/layout/display_list/builder.rs

+fn simple_normal_border(
+    color: ColorF,
+    style: webrender_api::BorderStyle,
+) -> webrender_api::NormalBorder {


Again, I'd say just import NormalBorder directly.

mrobinson · 2018-01-16T10:17:11Z

components/layout/display_list/builder.rs

@@ -1374,20 +1378,31 @@ impl FragmentDisplayListBuilding for Fragment {
            display_list_section,
        );

+        let radius_tmp = build_border_radius(&bounds, border_style_struct);


Why not just call this radius or border_radius?

mrobinson · 2018-01-16T10:17:11Z

components/gfx/display_list/mod.rs

-    /// True if gradient repeats infinitly.
-    pub repeating: bool,
+    /// Whether the gradient is repeated or clamped.
+    pub extend: webrender_api::ExtendMode,


I would cal this extend_mode in order to make the code more readable.

mrobinson · 2018-01-16T10:17:59Z

I would also go ahead and title this commit 'Use more WebRender types in gfx/display_list' since I think that's a really excellent summary of what it does.

pyfisch · 2018-01-16T11:23:33Z

All comments addressed except for max_rect.

bors-servo · 2018-01-17T21:49:12Z

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


        Use more WebRender types in gfx/display_list

This uses floating-point (Layout) coordinates in where possible. Replace NormalBorder struct with WebRender equivalent. Remove ToPointF and ToRectF traits. Convert border RepeatKeyword with ToLayout. Add some definitions to malloc_size_of for WebRender types.


        Introduce MaxRect trait

It is implemented for LayoutRect and Rect<Au>. Replaces the max_rect() function from servo_geometry.

pyfisch · 2018-01-17T22:49:28Z

Rebased and updated. Copied the MaxRect trait.

mrobinson

This looks great! Thank you for the cleanups.

mrobinson · 2018-01-18T11:40:20Z

@bors-servo r+

bors-servo · 2018-01-18T11:40:21Z

📌 Commit af52233 has been approved by mrobinson

bors-servo · 2018-01-18T11:42:53Z

⌛ Testing commit af52233 with merge aee0d69...


        Auto merge of #19782 - pyfisch:dl-simple, r=mrobinson

Use more WebRender types in gfx/display_list. Use more WebRender types in gfx/display_list. This uses floating-point (Layout) coordinates in where possible. Replace NormalBorder struct with WebRender equivalent. Remove ToPointF and ToRectF traits. Convert border RepeatKeyword with ToLayout. Add some definitions to malloc_size_of for WebRender types.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19782)

bors-servo · 2018-01-18T12:44:14Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: mrobinson
Pushing aee0d69 to master...


        Auto merge of #19824 - pyfisch:dl-move, r=<try>

Use more WebRender types in gfx Removes ImageBorder details from gfx. Use WebRender BorderRadius in BoxShadow. Stores cursors as integer. Use FilterOp, BorderWidths from WebRender. Store content_rect as LayoutRect. This amends #19782.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19824)


        Auto merge of #19824 - pyfisch:dl-move, r=mrobinson

Use more WebRender types in gfx Removes ImageBorder details from gfx. Use WebRender BorderRadius in BoxShadow. Stores cursors as integer. Use FilterOp, BorderWidths from WebRender. Store content_rect as LayoutRect. This amends #19782.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19824)

highfive assigned Manishearth Jan 16, 2018

highfive added the S-awaiting-review label Jan 16, 2018

emilio requested a review from mrobinson Jan 16, 2018

mrobinson requested changes Jan 16, 2018

View changes

pyfisch changed the title ~~Cleanup Display List~~ Use more WebRender types in gfx/display_list. Jan 16, 2018

pyfisch force-pushed the pyfisch:dl-simple branch from a4c3ff6 to a1adc9e Jan 16, 2018

pyfisch force-pushed the pyfisch:dl-simple branch from a1adc9e to 3f168cc Jan 16, 2018

highfive added the S-needs-rebase label Jan 17, 2018

pyfisch added 2 commits Jan 12, 2018

Introduce MaxRect trait

Loading status checks…

af52233

It is implemented for LayoutRect and Rect<Au>. Replaces the max_rect() function from servo_geometry.

pyfisch force-pushed the pyfisch:dl-simple branch from 0ffe5d8 to af52233 Jan 17, 2018

jdm removed the S-needs-rebase label Jan 18, 2018

mrobinson approved these changes Jan 18, 2018

View changes

highfive assigned mrobinson and unassigned Manishearth Jan 18, 2018

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 18, 2018

bors-servo merged commit af52233 into servo:master Jan 18, 2018
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jan 18, 2018

bors-servo mentioned this pull request Jan 18, 2018

style: Move cursor property out of mako #19798

Merged

5 of 5 tasks complete

pyfisch deleted the pyfisch:dl-simple branch Jan 18, 2018

pyfisch mentioned this pull request Jan 20, 2018

Use more WebRender types in gfx #19824

Merged

servo / servo

Use more WebRender types in gfx/display_list. #19782

Use more WebRender types in gfx/display_list. #19782

pyfisch commented Jan 16, 2018 •

edited by SimonSapin

highfive commented Jan 16, 2018

highfive commented Jan 16, 2018

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

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mrobinson commented Jan 16, 2018

pyfisch commented Jan 16, 2018

bors-servo commented Jan 17, 2018

pyfisch commented Jan 17, 2018

mrobinson left a comment

mrobinson commented Jan 18, 2018

bors-servo commented Jan 18, 2018

bors-servo commented Jan 18, 2018

bors-servo commented Jan 18, 2018

servo / servo

Join GitHub today

Use more WebRender types in gfx/display_list. #19782

Use more WebRender types in gfx/display_list. #19782

Conversation

pyfisch commented Jan 16, 2018 • edited by SimonSapin

highfive commented Jan 16, 2018

highfive commented Jan 16, 2018

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

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mrobinson commented Jan 16, 2018

pyfisch commented Jan 16, 2018

bors-servo commented Jan 17, 2018

pyfisch commented Jan 17, 2018

mrobinson left a comment

mrobinson commented Jan 18, 2018

bors-servo commented Jan 18, 2018

bors-servo commented Jan 18, 2018

bors-servo commented Jan 18, 2018

pyfisch commented Jan 16, 2018 •

edited by SimonSapin