stylo: Support linear-gradients as background-image #11456

emilio · 2016-05-26T23:42:21Z

This PR supports setting the background-image property to a linear-gradient in Geckolib.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes do not require tests because these are geckolib-only changes.

r? @mbrubeck
cc @heycam @bholley

This change is

highfive · 2016-05-26T23:42:24Z

Heads up! This PR modifies the following files:

@bholley: components/style/values.rs

highfive · 2016-05-26T23:42:25Z

Warning

These commits modify unsafe code. Please review it carefully!
These commits modify style code, but no tests are modified. Please consider adding a test!

emilio · 2016-05-26T23:52:45Z

Gecko bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1276085

mbrubeck · 2016-05-27T17:24:27Z

ports/geckolib/properties.mako.rs

+                        // NB: stops are guaranteed to be none in the gecko side by
+                        // default.
+                        stop.position.to_gecko_style_coord(&mut coord.mUnit,
+                                                           &mut coord.mValue);


You can write this as coord.set(stop.position);.

mbrubeck · 2016-05-27T17:26:18Z

ports/geckolib/properties.mako.rs

+                    if let AngleOrCorner::Angle(angle) = gradient.angle_or_corner {
+                        unsafe {
+                            angle.to_gecko_style_coord(&mut (*gecko_gradient).mAngle.mUnit,
+                                                       &mut (*gecko_gradient).mAngle.mValue);


gecko_gradient.mAngle.set(angle);

mbrubeck · 2016-05-27T17:27:15Z

r=mbrubeck with some minor cleanup suggestions (above).

highfive · 2016-05-27T22:21:27Z

New code was committed to pull request.

emilio · 2016-05-27T22:37:53Z

@mbrubeck: Thanks for the fast review! Forgot about those methods :)

Addressed the comments, maybe worth waiting for @heycam's review in Gecko to see if any other change is required.

emilio · 2016-06-01T08:39:54Z

Gecko changes got r+'d, so I'm going to land this (per the r=mbrubeck above) and make the nsTArray bindings work in a separate PR today.

@bors-servo: r=mbrubeck

bors-servo · 2016-06-01T08:39:55Z

📌 Commit bd72743 has been approved by mbrubeck

bors-servo · 2016-06-01T09:03:49Z

⌛ Testing commit bd72743 with merge e3e1cc8...


        Auto merge of #11456 - emilio:stylo-linear-gradient, r=mbrubeck

@mbrubeck

stylo: Support linear-gradients as background-image This PR supports setting the background-image property to a linear-gradient in Geckolib. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes do not require tests because these are geckolib-only changes. r? @mbrubeck cc @heycam @bholley  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11456)

bors-servo · 2016-06-01T10:09:20Z

💔 Test failed - mac-rel-css

highfive · 2016-06-01T10:09:22Z

  ▶ TIMEOUT [expected PASS] /css-transforms-1_dev/html/svg-translate-multiple-002.htm

emilio · 2016-06-01T10:51:53Z

@bors-servo: retry

Possibly new intermittent?

highfive · 2016-06-01T11:35:36Z

New code was committed to pull request.

highfive · 2016-06-01T11:45:08Z

New code was committed to pull request.

emilio · 2016-06-01T11:46:37Z

Added the nsTArray bindings, had to add a patch from @bholley that fixes overflow-wrap with the new bindings. cc @heycam about that.

Note that this won't build until #11445 lands, since it uses copy_from().

re-r? @mbrubeck


        stylo: Support linear-gradients as background-image


        stylo: Allow setting nsTArray members directly


        Stop mapping overflow-wrap to mWordWrap.

This was fixed in gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=955857


        stylo: Refactor nsTArray sugar and remove unused import

highfive · 2016-06-01T12:08:58Z

New code was committed to pull request.


        stylo: Remove unneeded null-check, and isolate unsafe code

Gecko's allocs are infallible by default.

highfive · 2016-06-01T12:13:14Z

New code was committed to pull request.

mbrubeck · 2016-06-02T16:00:20Z

-S-blocked-on-external -S-awaiting-review +S-awaiting-answer

Looks good to me, with one suggested change. r=mbrubeck with or without the change.

Previously, highfive wrote…

New code was committed to pull request.

Reviewed 5 of 5 files at r1, 1 of 1 files at r2, 6 of 6 files at r3, 2 of 9 files at r6, 4 of 6 files at r7, 1 of 1 files at r8, 1 of 2 files at r9, 1 of 1 files at r10.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

ports/geckolib/gecko_bindings/sugar/ns_t_array.rs, line 9 [r3] (raw file):

use structs::{nsTArray, nsTArrayHeader};

impl<T> Index<u32> for nsTArray<T> {

Would it make sense to instead impl Deref/DerefMut with Target=[T] for nsTArray<T>? Then we would get len, Index, IndexMut and other slice methods for free.

Comments from Reviewable


        stylo: Use Deref<Target=[T]> for nsTArray instead of Index<u32>

Makes things a lot nicer :)

highfive · 2016-06-02T16:26:22Z

New code was committed to pull request.

emilio · 2016-06-02T16:28:34Z

Yep, that's a great idea, I don't know why I didn't thought about it :)

@bors-servo: r=mbrubeck

bors-servo · 2016-06-02T16:28:34Z

📌 Commit c6340c4 has been approved by mbrubeck

bors-servo · 2016-06-02T16:50:54Z

⌛ Testing commit c6340c4 with merge e23962c...


        Auto merge of #11456 - emilio:stylo-linear-gradient, r=mbrubeck

@mbrubeck

stylo: Support linear-gradients as background-image This PR supports setting the background-image property to a linear-gradient in Geckolib. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes do not require tests because these are geckolib-only changes. r? @mbrubeck cc @heycam @bholley  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11456)

bors-servo · 2016-06-02T17:40:40Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows


        Bug 1276085: followup: Remove Gecko_SetGradientStop now we have nsTAr…

…ray bindings r=heycam See servo/servo#11456, when they where added. MozReview-Commit-ID: FsHMk9FjOo1


        Bug 1276085: followup: Remove Gecko_SetGradientStop now we have nsTAr…

…ray bindings r=heycam See servo/servo#11456, when they where added. MozReview-Commit-ID: FsHMk9FjOo1


        Bug 1276085: followup: Remove Gecko_SetGradientStop now we have nsTAr…

…ray bindings r=heycam See servo/servo#11456, when they where added. MozReview-Commit-ID: FsHMk9FjOo1 UltraBlame original commit: 0560b158bb22366202b489a95d78cb66605d456e


        Bug 1276085: followup: Remove Gecko_SetGradientStop now we have nsTAr…

…ray bindings r=heycam See servo/servo#11456, when they where added. MozReview-Commit-ID: FsHMk9FjOo1 UltraBlame original commit: 0560b158bb22366202b489a95d78cb66605d456e


        Bug 1276085: followup: Remove Gecko_SetGradientStop now we have nsTAr…

…ray bindings r=heycam See servo/servo#11456, when they where added. MozReview-Commit-ID: FsHMk9FjOo1 UltraBlame original commit: 0560b158bb22366202b489a95d78cb66605d456e

highfive assigned mbrubeck May 26, 2016

mbrubeck reviewed May 27, 2016
View changes

emilio force-pushed the emilio:stylo-linear-gradient branch from 7306c76 to bd72743 May 27, 2016

mbrubeck added S-blocked-on-external and removed S-awaiting-review labels May 31, 2016

highfive added the S-awaiting-merge label Jun 1, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 1, 2016

highfive added S-awaiting-review and removed S-tests-failed labels Jun 1, 2016

emilio and others added 3 commits May 26, 2016

stylo: Support linear-gradients as background-image

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

abf8ced

stylo: Allow setting nsTArray members directly

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

3204bfd

Stop mapping overflow-wrap to mWordWrap.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

decf8c4

This was fixed in gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=955857

stylo: Refactor nsTArray sugar and remove unused import

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

Loading status checks…

33829b4

emilio force-pushed the emilio:stylo-linear-gradient branch from 25d4baa to 33829b4 Jun 1, 2016

stylo: Remove unneeded null-check, and isolate unsafe code

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

Loading status checks…

8220eaa

Gecko's allocs are infallible by default.

mbrubeck added S-awaiting-answer and removed S-blocked-on-external S-awaiting-review labels Jun 2, 2016

emilio mentioned this pull request Jun 2, 2016

Generate debug and release Gecko bindings. #11549

Merged

2 of 5 tasks complete

stylo: Use Deref<Target=[T]> for nsTArray instead of Index<u32>

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

Loading status checks…

c6340c4

Makes things a lot nicer :)

highfive added the S-awaiting-review label Jun 2, 2016

highfive added S-awaiting-merge and removed S-awaiting-review S-awaiting-answer labels Jun 2, 2016

bors-servo merged commit c6340c4 into servo:master Jun 2, 2016
2 checks passed

2 checks passed

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

homu Test successful
Details

highfive removed the S-awaiting-merge label Jun 2, 2016

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 9, 2016

Bug 1276085: followup: Remove Gecko_SetGradientStop now we have nsTAr…

7d1a406

…ray bindings r=heycam See servo/servo#11456, when they where added. MozReview-Commit-ID: FsHMk9FjOo1

jdashg pushed a commit to jdashg/gecko-cinn that referenced this pull request Jun 11, 2016

Bug 1276085: followup: Remove Gecko_SetGradientStop now we have nsTAr…

ea92655

…ray bindings r=heycam See servo/servo#11456, when they where added. MozReview-Commit-ID: FsHMk9FjOo1

servo / servo

stylo: Support linear-gradients as background-image #11456

stylo: Support linear-gradients as background-image #11456

emilio commented May 26, 2016 •

edited by larsbergstrom

highfive commented May 26, 2016

highfive commented May 26, 2016

emilio commented May 26, 2016

This comment has been minimized.

This comment has been minimized.

mbrubeck commented May 27, 2016

highfive commented May 27, 2016

emilio commented May 27, 2016

emilio commented Jun 1, 2016

bors-servo commented Jun 1, 2016

bors-servo commented Jun 1, 2016

bors-servo commented Jun 1, 2016

highfive commented Jun 1, 2016

emilio commented Jun 1, 2016

highfive commented Jun 1, 2016

highfive commented Jun 1, 2016

emilio commented Jun 1, 2016

highfive commented Jun 1, 2016

highfive commented Jun 1, 2016

mbrubeck commented Jun 2, 2016

highfive commented Jun 2, 2016

emilio commented Jun 2, 2016

bors-servo commented Jun 2, 2016

bors-servo commented Jun 2, 2016

bors-servo commented Jun 2, 2016

servo / servo

Join GitHub today

stylo: Support linear-gradients as background-image #11456

stylo: Support linear-gradients as background-image #11456

Conversation

emilio commented May 26, 2016 • edited by larsbergstrom

highfive commented May 26, 2016

highfive commented May 26, 2016

emilio commented May 26, 2016

This comment has been minimized.

mbrubeck May 27, 2016 • edited

This comment has been minimized.

mbrubeck May 27, 2016 • edited

mbrubeck commented May 27, 2016

highfive commented May 27, 2016

emilio commented May 27, 2016

emilio commented Jun 1, 2016

bors-servo commented Jun 1, 2016

bors-servo commented Jun 1, 2016

bors-servo commented Jun 1, 2016

highfive commented Jun 1, 2016

emilio commented Jun 1, 2016

highfive commented Jun 1, 2016

highfive commented Jun 1, 2016

emilio commented Jun 1, 2016

highfive commented Jun 1, 2016

highfive commented Jun 1, 2016

mbrubeck commented Jun 2, 2016

highfive commented Jun 2, 2016

emilio commented Jun 2, 2016

bors-servo commented Jun 2, 2016

bors-servo commented Jun 2, 2016

bors-servo commented Jun 2, 2016

emilio commented May 26, 2016 •

edited by larsbergstrom

mbrubeck May 27, 2016 •

edited

mbrubeck May 27, 2016 •

edited