Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render border #1276

Closed
wants to merge 2 commits into from
Closed

Render border #1276

wants to merge 2 commits into from

Conversation

@hyunjunekim
Copy link
Contributor

hyunjunekim commented Nov 18, 2013

Implemented a border which was the solid type.
Because of not support css pseudo element(:before, :after), content in css property and base64, So we couldn't test Acid2(http://www.webstandards.org/files/acid2/test.html).

@highfive
Copy link

highfive commented Nov 18, 2013

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

let mut stroke_opts = StrokeOptions(0 as AzFloat, 10 as AzFloat);
let mut dash: [AzFloat, ..2] = [0 as AzFloat, 0 as AzFloat];
//let mut start = Point2D(0.0 as f32,0.0 as f32);
//let mut end = Point2D(0.0 as f32,0.0 as f32);

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

Remove these commented out lines.

dash[0] = border_width * 3 as AzFloat;
dash[1] = border_width * 3 as AzFloat;
stroke_opts.mDashPattern = vec::raw::to_ptr(dash);
stroke_opts.mDashLength = dash.len() as size_t;

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

Can you change this to:

let border_width = match direction {
    Top => border.top,
    Left => border.left,
    ...
};
stroke_opts.line_width = border_width;
dash[0] = border_width * 3 as AzFloat;
dash[1] = border_width * 3 as AzFloat;
stroke_opts.mDashPattern = vec::raw::to_ptr(dash);
stroke_opts.mDashLength = dash.len() as size_t;

Just to avoid duplicating the stroke option setting code.

let left_top = Point2D(rect.origin.x, rect.origin.y);
let right_top = Point2D(rect.origin.x + rect.size.width, rect.origin.y);
let left_bottom = Point2D(rect.origin.x, rect.origin.y + rect.size.height);
let right_bottom = Point2D(rect.origin.x + rect.size.width, rect.origin.y + rect.size.height);

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: fix indentation here.


fn draw_solid_border_segment(&self, direction: Direction, bounds: &Rect<Au>, border: SideOffsets2D<f32>, color: Color) {
let rect = bounds.to_azure_rect();
let draw_opts = DrawOptions(1 as AzFloat,0 as uint16_t);

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: fix indentation.

Also I'd write this as:

let draw_opts = DrawOptions(1.0, 0);
//let mut start = Point2D(0.0 as f32,0.0 as f32);
//let mut end = Point2D(0.0 as f32,0.0 as f32);

stroke_opts.set_cap_style(AZ_CAP_BUTT as u8);

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: fix indentation.

let rect = bounds.to_azure_rect();
let draw_opts = DrawOptions(1 as AzFloat, 0 as uint16_t);
let mut stroke_opts = StrokeOptions(0 as AzFloat, 10 as AzFloat);
let mut dash: [AzFloat, ..2] = [0 as AzFloat, 0 as AzFloat];

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: fix indentation.

self.draw_border_segment(Top, bounds, border, color, style);
self.draw_border_segment(Right, bounds, border, color, style);
self.draw_border_segment(Bottom, bounds, border, color, style);
self.draw_border_segment(Left, bounds, border, color, style);

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: fix indentation.

fn apply_border_style(style: border_style::T, border_width: AzFloat, dash: &mut [AzFloat], stroke_opts: &mut StrokeOptions){
match style{
fn draw_border_segment(&self, direction: Direction, bounds: &Rect<Au>, border: SideOffsets2D<f32>, color: SideOffsets2D<Color>, style: SideOffsets2D<border_style::T>) {
//let mut color_select = color.top;

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: remove commented out line.

Left => {(style.left, color.left)}
Right => {(style.right, color.right)}
Bottom => {(style.bottom, color.bottom)}
};

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: fix indentation.

Also this can be written as:

let (style_select, color_select) = match direction {
    Top => (style.top, color.top),
    Left => (style.left, color.left),
    ...
};

(i.e. without the {})

path_builder.line_to(right_bottom + Point2D(-border.right, -border.bottom));
path_builder.line_to(right_bottom);
}
}

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: indent

}
};

self.draw_target.stroke_line(start,

This comment has been minimized.

@pcwalton

pcwalton Nov 18, 2013

Contributor

nit: fix indentation.

@hyunjunekim
Copy link
Contributor Author

hyunjunekim commented Nov 20, 2013

Fixed build bot fails. I rebased and now it works well. about #1260 r? @pcwalton

bors-servo pushed a commit that referenced this pull request Nov 20, 2013
Implemented a border which was the solid type.
Because of not support css pseudo element(:before, :after), content in css property and base64, So we couldn't test Acid2(http://www.webstandards.org/files/acid2/test.html).
@jdm
Copy link
Member

jdm commented Nov 24, 2013

Looks like a rebase is required before we can retry merging.

@hyunjunekim
Copy link
Contributor Author

hyunjunekim commented Nov 24, 2013

Ok, I will be a rebase, Thank you, jdm!

@brson
Copy link
Contributor

brson commented Nov 24, 2013

Nice work, Hyunjune! °¨»çÇÕ´Ï´Ù
On Nov 18, 2013 1:04 AM, "hyunjunekim" notifications@github.com wrote:

Implemented a border which was the solid type.
Because of not support css pseudo element(:before, :after), content in css
property and base64, So we couldn't test Acid2(

http://www.webstandards.org/files/acid2/test.html).

You can merge this Pull Request by running

git pull https://github.com/hyunjunekim/servo RenderBorder

Or view, comment on, or merge it at:

#1276
Commit Summary

  • Render border solid-style properly
  • Update Submodule
  • Add diamond test code

File Changes

  • M src/components/gfx/render_context.rshttps://github.com//pull/1276/files#diff-0(211)
  • M src/support/azure/rust-azurehttps://github.com//pull/1276/files#diff-1(2)
  • M src/test/html/test_border.htmlhttps://github.com//pull/1276/files#diff-2(19)

Patch Links:

@kmcallister
Copy link
Contributor

kmcallister commented Nov 25, 2013

@brson nice mojibake :)

$ echo '°¨»çÇÕ´Ï´Ù' | iconv -t iso8859-1 | iconv -f euc-kr
감사합니다
@sammykim
Copy link
Contributor

sammykim commented Nov 28, 2013

We rebased them again.
r? @pcwalton . Thanks!

@jdm jdm mentioned this pull request Nov 30, 2013
@jdm
Copy link
Member

jdm commented Nov 30, 2013

Rebased in #1322.

@jdm jdm closed this Nov 30, 2013
bors-servo pushed a commit that referenced this pull request Nov 30, 2013
Implemented a border which was the solid type.
Because of not support css pseudo element(:before, :after), content in css property and base64, So we couldn't test Acid2(http://www.webstandards.org/files/acid2/test.html).

Rebase of #1276.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.