Implement asymmetric border colors #689

sammykim · 2013-08-08T01:24:33Z

No description provided.

metajack · 2013-08-08T02:31:52Z

This is great. I have two suggestions:

Use SideOffsets2D instead. I realize the name isn't perfect, but we can change the name later.
Add a new constructor for SideOffsets2D called new_all_same() and refactor the calls to ::new(x, x, x, x) that exist now.

sanxiyn · 2013-08-08T04:25:03Z

src/components/gfx/color.rs

@@ -7,6 +7,13 @@ use AzColor = azure::azure_hl::Color;

 pub type Color = AzColor;

+pub struct Colors{


A space before the brace.

sanxiyn · 2013-08-08T04:25:23Z

src/components/gfx/display_list.rs

@@ -14,7 +14,7 @@
 /// They are therefore not exactly analogous to constructs like Skia pictures, which consist of
 /// low-level drawing primitives.

-use color::Color;
+use color::{Color,Colors};


A space after the comma.

sanxiyn · 2013-08-08T04:26:40Z

src/components/main/layout/box.rs

@@ -640,7 +641,12 @@ impl RenderBox {
                                extra: ExtraDisplayListData::new(*self),
                            },
                            border: debug_border,
-                            color: rgb(0, 0, 200).to_gfx_color(),


I think you want debug_color local variable just like debug_border.

I can't merge them to debug_color as same as debug_border.
Becuase their values are not same.

sanxiyn · 2013-08-08T04:27:46Z

src/components/main/layout/box.rs

@@ -907,6 +924,9 @@ impl RenderBox {

        // FIXME: all colors set to top color. this is obviously not right.


Delete this comment.

sanxiyn · 2013-08-08T04:27:54Z

src/components/main/layout/box.rs

@@ -907,6 +924,9 @@ impl RenderBox {

        // FIXME: all colors set to top color. this is obviously not right.
        let top_color = self.style().border_top_color();
+        let bottom_color = self.style().border_bottom_color();
+        let left_color = self.style().border_left_color();
+        let right_color = self.style().border_right_color();
        let color = top_color.to_gfx_color();


sammykim · 2013-08-08T06:01:38Z

@metajack I tried to use SideOffsets2D<Color>, but the compiler complains that Color does not implement Num.

sammykim · 2013-08-08T06:35:02Z

I sent servo/euclid#12.

metajack · 2013-08-08T14:04:03Z

servo/euclid#12 is now merged so you can finish the conversion.

sammykim · 2013-08-09T01:28:49Z

To implement new_all_same, the type needs to implement Clone, so that requires another submodule changes. :(

Please be patient, since I will be on a vacation in the next week.

sammykim · 2013-08-19T01:48:19Z

I sent servo/euclid#15 servo/rust-azure#48

metajack · 2013-08-19T15:35:20Z

Those are both merged. You can now update this PR.


        Implement asymmetric border colors


        Use new_all_same with same value instead of new


        Test file for asymmetric border color


        Update submodules

sammykim · 2013-08-20T01:08:46Z

r? @metajack

metajack · 2013-08-20T01:19:59Z

r+

bors-servo · 2013-08-20T01:43:06Z

saw approval from metajack
at sammykim@858cd07

bors-servo · 2013-08-20T01:43:09Z

merging sammykim/servo/Border-Color = 858cd07 into auto

bors-servo · 2013-08-20T01:43:11Z

sammykim/servo/Border-Color = 858cd07 merged ok, testing candidate = e86b8e1

bors-servo · 2013-08-20T02:04:02Z

all tests pass:
http://servo-buildbot.pub.build.mozilla.org/builders/linux/builds/253
http://servo-buildbot.pub.build.mozilla.org/builders/mac/builds/208

bors-servo · 2013-08-20T02:07:05Z

fast-forwarding master to auto = e86b8e1


        auto merge of #689 : sammykim/servo/Border-Color, r=metajack


        Merge pull request servo#689 from foolip/pause-remove-from-document

Test the steps for pausing a media element when removing from a document


        Auto merge of servo#689 - jrmuizel:cleanup-unstride, r=kvark

wrench: Cleanup unstride() This switches to using an iterator and factors out the saving.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/689)

sanxiyn reviewed Aug 8, 2013
View changes

src/components/gfx/color.rs Outdated

@@ -7,6 +7,13 @@ use AzColor = azure::azure_hl::Color;

pub type Color = AzColor;

pub struct Colors{

This comment has been minimized.

Sign in to view

Copy link

sanxiyn Aug 8, 2013
Contributor

A space before the brace.

sanxiyn reviewed Aug 8, 2013
View changes

src/components/main/layout/box.rs Outdated

@@ -907,6 +924,9 @@ impl RenderBox {

// FIXME: all colors set to top color. this is obviously not right.

This comment has been minimized.

Sign in to view

Copy link

sanxiyn Aug 8, 2013
Contributor

Delete this comment.

sanxiyn reviewed Aug 8, 2013
View changes

sammykim added 4 commits Aug 9, 2013

Implement asymmetric border colors

0e863b3

Use new_all_same with same value instead of new

3616c03

Test file for asymmetric border color

3e74538

Update submodules

Loading status checks…

858cd07

bors-servo pushed a commit that referenced this pull request Aug 20, 2013

auto merge of #689 : sammykim/servo/Border-Color, r=metajack

e86b8e1

bors-servo merged commit 858cd07 into servo:master Aug 20, 2013
1 check passed

1 check passed

default all tests passed

ChrisParis pushed a commit to ChrisParis/servo that referenced this pull request Sep 7, 2014

Merge pull request servo#689 from foolip/pause-remove-from-document

8e762ce

Test the steps for pausing a media element when removing from a document

servo / servo

Implement asymmetric border colors #689

Implement asymmetric border colors #689

sammykim commented Aug 8, 2013

metajack commented Aug 8, 2013

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.

sammykim commented Aug 8, 2013

sammykim commented Aug 8, 2013

metajack commented Aug 8, 2013

sammykim commented Aug 9, 2013

sammykim commented Aug 19, 2013

metajack commented Aug 19, 2013

sammykim commented Aug 20, 2013

This comment has been minimized.

metajack commented on `858cd07` Aug 20, 2013

This comment has been minimized.

bors-servo commented on `858cd07` Aug 20, 2013

This comment has been minimized.

bors-servo replied Aug 20, 2013

This comment has been minimized.

bors-servo replied Aug 20, 2013

This comment has been minimized.

bors-servo replied Aug 20, 2013

This comment has been minimized.

bors-servo replied Aug 20, 2013

		@@ -7,6 +7,13 @@ use AzColor = azure::azure_hl::Color;

		pub type Color = AzColor;

		pub struct Colors{

		@@ -907,6 +924,9 @@ impl RenderBox {

		// FIXME: all colors set to top color. this is obviously not right.

servo / servo

Join GitHub today

Implement asymmetric border colors #689

Implement asymmetric border colors #689

Conversation

sammykim commented Aug 8, 2013

metajack commented Aug 8, 2013

This comment has been minimized.

sanxiyn Aug 8, 2013

This comment has been minimized.

sanxiyn Aug 8, 2013

This comment has been minimized.

sanxiyn Aug 8, 2013

This comment has been minimized.

sammykim Aug 8, 2013

This comment has been minimized.

sanxiyn Aug 8, 2013

This comment has been minimized.

sanxiyn Aug 8, 2013

sammykim commented Aug 8, 2013

sammykim commented Aug 8, 2013

metajack commented Aug 8, 2013

sammykim commented Aug 9, 2013

sammykim commented Aug 19, 2013

metajack commented Aug 19, 2013

sammykim commented Aug 20, 2013

This comment has been minimized.

metajack commented on 858cd07 Aug 20, 2013

This comment has been minimized.

bors-servo commented on 858cd07 Aug 20, 2013

This comment has been minimized.

bors-servo replied Aug 20, 2013

This comment has been minimized.

bors-servo replied Aug 20, 2013

This comment has been minimized.

bors-servo replied Aug 20, 2013

This comment has been minimized.

bors-servo replied Aug 20, 2013

metajack commented on `858cd07` Aug 20, 2013

bors-servo commented on `858cd07` Aug 20, 2013