Bump euclid to 0.14.x. #17184

nical · 2017-06-06T14:38:39Z

./mach build -d does not report any errors (kinda, need webrender published and Cargo.toml fixed up)
./mach test-tidy does not report any errors
These changes fix #__ (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because it is a refactoring in which the difference is mostly a compile-time/strong-typing thing with no change to the logic.

This change is

highfive · 2017-06-06T14:38:46Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @asajeffrey (or someone else) soon.

highfive · 2017-06-06T14:38:47Z

Heads up! This PR modifies the following files:

@bholley: components/style/gecko_bindings/sugar/ns_timing_function.rs, components/style/properties/gecko.mako.rs, components/style/animation.rs, components/style/properties/helpers/animated_properties.mako.rs, components/style/properties/properties.mako.rs and 7 more
@jgraham: components/webdriver_server/Cargo.toml
@wafflespeanut: python/tidy/servo_tidy_tests/rust_tidy.rs
@emilio: components/style/gecko_bindings/sugar/ns_timing_function.rs, components/style/properties/gecko.mako.rs, components/style/animation.rs, components/layout/multicol.rs, components/layout/list_item.rs and 21 more
@fitzgen: components/script/dom/htmlcanvaselement.rs, components/script/timers.rs, components/script/dom/imagedata.rs, components/script_traits/script_msg.rs, components/script_traits/script_msg.rs and 25 more
@KiChjang: components/script/dom/htmlcanvaselement.rs, components/script/timers.rs, components/script/dom/imagedata.rs, components/script_traits/script_msg.rs, components/script_traits/script_msg.rs and 25 more
@asajeffrey: components/constellation/browsingcontext.rs, components/constellation/constellation.rs, components/webdriver_server/Cargo.toml, components/constellation/pipeline.rs, components/constellation/Cargo.toml

highfive · 2017-06-06T14:38:48Z

Warning

These commits modify unsafe code. Please review it carefully!

nical · 2017-06-06T14:47:26Z

This requires webrender the equivalent WebRender and rust-azure changes to be merged/published, in order to build (I did get it to work locally), but the review is going to be a lot of work so I am submitting this early for people to start looking at it. This pull request is enormous, and I am truly sorry for this.
We made some major changes to euclid, in particular the separation of points and vector types. See servo/euclid#159.

cc @nox

bors-servo · 2017-06-06T18:33:25Z

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

asajeffrey · 2017-06-07T21:17:51Z

Gosh, you're not kidding about this being a big change!

Are most of the changes to euclid naming? Would it be possible/useful to publish an intermediate version of euclid with both naming schemes?

nical · 2017-06-08T08:45:28Z

The biggest change is the separation between vector and points. The rest of the diff is trivial changes like renaming Matrix2D/4D into Transform2D/3D and cleaning up the namespaces (use euclid::rect::Rect; becomes use euclid::Rect).
There is also two commits about SVG and animations that are not actually from this PR but that appear in the diff. After I have rebased this, they will disappear from the diff.

Once I have rebased this patch though, most of the non-trivial changes won't be in the diff, and if you want I can roll back the the Matrix/Transform renaming (this specific part doesn't occupy much room in the diff though) since I left an alias for the old name in euclid marked as deprecated, so it can be postponed if servo build does not deny warnings.

For the other changes, I am sorry but I spent way too much time getting this euclid bump into all of the servo crates outside the main repo, getting reviews an getting things published on crates in the correct order. I ended up doing mostly that for more than a week and I won't do it again. Had I known how much work this would be, I would not have made these changes to euclid.

nical · 2017-06-08T12:23:57Z

I rebased and added a commit that reverts the matrix/transform renaming part (at the cost of introducing deprecation warnings). The total diff is a lot less scary to look at now.
Let me know if you prefer things squashed as is and move the matrix/transform stuff to a separate PR or if you want it in this one.

I kept another commit separated: the one that turns BaseFlow::stacking_relative_position into a vector instead of a point. For this one I wasn't 100% sure whether it made most sense as a vector or a point (short of knowing the layout code well enough), so it is still in its own commit which can be removed easily if need be.

Things still don't build on CI because I still need the equivalent webrender PR to be merged, but it works locally.

bors-servo · 2017-06-09T22:56:26Z

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

mrobinson · 2017-06-12T09:41:15Z

Cargo.toml

+
+"https://github.com/servo/webrender#0.40.0" = { path = "../webrender/webrender/" }
+"https://github.com/servo/webrender#webrender_traits:0.40.0" = { path = "../webrender/webrender_traits/" }
+"https://github.com/servo/rust-azure#azure:0.16.0" = { path = "../rust-azure/" }


Now that azure has the WebRender bump as well, I guess this can be removed.

asajeffrey

Looks perfectly sensible, and a lot of work! Just a couple of nits.

asajeffrey · 2017-06-12T15:01:23Z

components/layout/fragment.rs

-                                                         transform_origin_y,
-                                                         transform_origin_z);
+                                                            transform_origin_y,
+                                                            transform_origin_z);


Nit: I think our indentation style is the old version of the code.

asajeffrey · 2017-06-12T15:01:23Z

components/layout/fragment.rs

-    pub fn translate(&mut self, point: &Point2D<Au>) {
-        self.scroll = self.scroll.translate(point);
-        self.paint = self.paint.translate(point);
+    pub fn translate(&mut self, by: &Vector2D<Au>) {


This makes a lot more sense with the new type name!

asajeffrey · 2017-06-12T15:01:23Z

components/layout/webrender_helpers.rs

@@ -86,12 +86,22 @@ trait ToPointF {
    fn to_pointf(&self) -> webrender_traits::LayoutPoint;
 }

+trait ToVectorF {
+    fn to_vectorf(&self) -> webrender_traits::LayoutVector2D;


Nit: the function name is not very self-explanatory.

SimonSapin · 2017-06-13T12:40:11Z

Looks good to me.

Reviewed 39 of 87 files at r1, 41 of 51 files at r4, 9 of 9 files at r5.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

components/canvas/canvas_paint_thread.rs, line 651 at r5 (raw file):

        // Step 6.
        let dest_rect = dirty_rect.translate(&offset.to_vector()).to_i32();

Should offset be a vector to start with?

components/script/dom/document.rs, line 869 at r5 (raw file):

                let rect = iframe.upcast::<Element>().GetBoundingClientRect();
                let child_origin = Point2D::new(rect.X() as f32, rect.Y() as f32);
                let child_point = client_point - child_origin.to_vector();

Could child_origin be a vector to start with? (Here and 3 others in this file.)

Comments from Reviewable

nical · 2017-06-14T11:52:03Z

The review comments have been addressed, should we land this now?

SimonSapin · 2017-06-14T13:18:30Z

Thank you!

@bors-servo r+

Reviewed 3 of 13 files at r6, 9 of 9 files at r7, 1 of 10 files at r8, 9 of 9 files at r9.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

bors-servo · 2017-06-14T13:18:31Z

📌 Commit 87e6a6c has been approved by SimonSapin

bors-servo · 2017-06-14T13:18:35Z

⌛ Testing commit 87e6a6c with merge 4e25084...


        Auto merge of #17184 - nical:euclid-bump, r=SimonSapin

Bump euclid to 0.14.x. - [x] `./mach build -d` does not report any errors (kinda, need webrender published and Cargo.toml fixed up) - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because it is a refactoring in which the difference is mostly a compile-time/strong-typing thing with no change to the logic.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17184)

bors-servo · 2017-06-14T13:32:55Z

💔 Test failed - linux-dev


        Bump euclid to 0.14.


        Make BaseFlow::stacking_relative_position a vector.

SimonSapin · 2017-06-14T14:19:14Z

@bors-servo r+

Reviewed 15 of 24 files at r10, 9 of 9 files at r11.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

bors-servo · 2017-06-14T14:19:15Z

📌 Commit 997608f has been approved by SimonSapin

bors-servo · 2017-06-14T14:25:06Z

⌛ Testing commit 997608f with merge 18653f6...


        Auto merge of #17184 - nical:euclid-bump, r=SimonSapin

Bump euclid to 0.14.x. - [x] `./mach build -d` does not report any errors (kinda, need webrender published and Cargo.toml fixed up) - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because it is a refactoring in which the difference is mostly a compile-time/strong-typing thing with no change to the logic.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17184)

bors-servo · 2017-06-14T15:32:23Z

☀️ 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: SimonSapin
Pushing 18653f6 to master...

highfive assigned asajeffrey Jun 6, 2017

highfive added the S-awaiting-review label Jun 6, 2017

nical mentioned this pull request Jun 6, 2017

Bump euclid to 0.14 servo/webrender#1337

Merged

highfive added the S-needs-rebase label Jun 6, 2017

nical force-pushed the nical:euclid-bump branch 2 times, most recently from 7251988 to 7ff6ee1 Jun 8, 2017

highfive added the S-needs-rebase label Jun 9, 2017

mrobinson reviewed Jun 12, 2017

View changes

nical force-pushed the nical:euclid-bump branch 2 times, most recently from 2de0439 to 2f5fbdf Jun 12, 2017

asajeffrey reviewed Jun 12, 2017

View changes

nical changed the title ~~[WIP] Bump eucld to 0.14.x.~~ [WIP] Bump euclid to 0.14.x. Jun 12, 2017

nical force-pushed the nical:euclid-bump branch from 2f5fbdf to cee0715 Jun 12, 2017

nical changed the title ~~[WIP] Bump euclid to 0.14.x.~~ Bump euclid to 0.14.x. Jun 12, 2017

nical force-pushed the nical:euclid-bump branch from cee0715 to b3a42b1 Jun 13, 2017

nical force-pushed the nical:euclid-bump branch from b3a42b1 to 87e6a6c Jun 14, 2017

highfive unassigned asajeffrey Jun 14, 2017

highfive assigned SimonSapin Jun 14, 2017

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase labels Jun 14, 2017

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 14, 2017

nical added 2 commits Jun 2, 2017

Bump euclid to 0.14.

8617320

Make BaseFlow::stacking_relative_position a vector.

Loading status checks…

997608f

nical force-pushed the nical:euclid-bump branch from 87e6a6c to 997608f Jun 14, 2017

highfive added S-awaiting-review and removed S-tests-failed labels Jun 14, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Jun 14, 2017

bors-servo merged commit 997608f into servo:master Jun 14, 2017
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 Jun 14, 2017

This was referenced Jun 14, 2017

[WIP] Unfork glutin. #17311

Closed

WIP - Added prompt user for url #17062

Closed

Remove compositor forwarding code and use dedicated mechanism #17269

Merged

servo / servo

Bump euclid to 0.14.x. #17184

Bump euclid to 0.14.x. #17184

nical commented Jun 6, 2017 •

edited by larsbergstrom

highfive commented Jun 6, 2017

highfive commented Jun 6, 2017

highfive commented Jun 6, 2017

nical commented Jun 6, 2017

bors-servo commented Jun 6, 2017

asajeffrey commented Jun 7, 2017

nical commented Jun 8, 2017

nical commented Jun 8, 2017 •

edited

bors-servo commented Jun 9, 2017

This comment has been minimized.

asajeffrey left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SimonSapin commented Jun 13, 2017

nical commented Jun 14, 2017

SimonSapin commented Jun 14, 2017

bors-servo commented Jun 14, 2017

bors-servo commented Jun 14, 2017

bors-servo commented Jun 14, 2017

SimonSapin commented Jun 14, 2017

bors-servo commented Jun 14, 2017

bors-servo commented Jun 14, 2017

bors-servo commented Jun 14, 2017

servo / servo

Join GitHub today

Bump euclid to 0.14.x. #17184

Bump euclid to 0.14.x. #17184

Conversation

nical commented Jun 6, 2017 • edited by larsbergstrom

highfive commented Jun 6, 2017

highfive commented Jun 6, 2017

highfive commented Jun 6, 2017

nical commented Jun 6, 2017

bors-servo commented Jun 6, 2017

asajeffrey commented Jun 7, 2017

nical commented Jun 8, 2017

nical commented Jun 8, 2017 • edited

bors-servo commented Jun 9, 2017

This comment has been minimized.

mrobinson Jun 12, 2017

asajeffrey left a comment

This comment has been minimized.

asajeffrey Jun 12, 2017

This comment has been minimized.

asajeffrey Jun 12, 2017

This comment has been minimized.

asajeffrey Jun 12, 2017

SimonSapin commented Jun 13, 2017

nical commented Jun 14, 2017

SimonSapin commented Jun 14, 2017

bors-servo commented Jun 14, 2017

bors-servo commented Jun 14, 2017

bors-servo commented Jun 14, 2017

SimonSapin commented Jun 14, 2017

bors-servo commented Jun 14, 2017

bors-servo commented Jun 14, 2017

bors-servo commented Jun 14, 2017

nical commented Jun 6, 2017 •

edited by larsbergstrom

nical commented Jun 8, 2017 •

edited