From ebdf4693abcd3f05a88cfb9d20664c0a489f4964 Mon Sep 17 00:00:00 2001 From: pylbrecht Date: Sat, 25 Jan 2020 19:36:02 +0100 Subject: [PATCH 1/2] Update fill and stroke style only when required So far fill and stroke style updates have been sent to the canvas paint thread by `SetFillStyle()` and `SetStrokeStyle()`. This resulted in fill/stroke style updates not being considered by the canvas paint thread between the latest call of `SetFillStyle()`/`SetStrokeStyle()` and the drawing operation (e.g. fill or stroke). This issue is solved by making `SetFillStyle()` and `SetStrokeStyle()` update the local canvas state and propagating the state to the canvas paint thread right before any drawing operation that requires it. --- components/script/canvas_state.rs | 41 +++++++++++-------- .../2d.gradient.object.update.html.ini | 5 --- .../2d.pattern.animated.gif.html.ini | 4 -- .../2d.gradient.object.update.html.ini | 4 -- .../2d.gradient.object.update.worker.js.ini | 4 -- 5 files changed, 23 insertions(+), 35 deletions(-) delete mode 100644 tests/wpt/metadata/2dcontext/fill-and-stroke-styles/2d.gradient.object.update.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/fill-and-stroke-styles/2d.pattern.animated.gif.html.ini delete mode 100644 tests/wpt/metadata/offscreen-canvas/fill-and-stroke-styles/2d.gradient.object.update.html.ini delete mode 100644 tests/wpt/metadata/offscreen-canvas/fill-and-stroke-styles/2d.gradient.object.update.worker.js.ini diff --git a/components/script/canvas_state.rs b/components/script/canvas_state.rs index 0416c96aff0..c59cb3c8823 100644 --- a/components/script/canvas_state.rs +++ b/components/script/canvas_state.rs @@ -654,6 +654,7 @@ impl CanvasState { // https://html.spec.whatwg.org/multipage/#dom-context-2d-fillrect pub fn fill_rect(&self, x: f64, y: f64, width: f64, height: f64) { if let Some(rect) = self.create_drawable_rect(x, y, width, height) { + self.update_fill_style(); self.send_canvas_2d_msg(Canvas2dMsg::FillRect(rect)); } } @@ -668,6 +669,7 @@ impl CanvasState { // https://html.spec.whatwg.org/multipage/#dom-context-2d-strokerect pub fn stroke_rect(&self, x: f64, y: f64, width: f64, height: f64) { if let Some(rect) = self.create_drawable_rect(x, y, width, height) { + self.update_stroke_style(); self.send_canvas_2d_msg(Canvas2dMsg::StrokeRect(rect)); } } @@ -756,24 +758,15 @@ impl CanvasState { StringOrCanvasGradientOrCanvasPattern::String(string) => { if let Ok(rgba) = self.parse_color(canvas, &string) { self.state.borrow_mut().stroke_style = CanvasFillOrStrokeStyle::Color(rgba); - self.send_canvas_2d_msg(Canvas2dMsg::SetStrokeStyle(FillOrStrokeStyle::Color( - rgba, - ))); } }, StringOrCanvasGradientOrCanvasPattern::CanvasGradient(gradient) => { self.state.borrow_mut().stroke_style = CanvasFillOrStrokeStyle::Gradient(Dom::from_ref(&*gradient)); - self.send_canvas_2d_msg(Canvas2dMsg::SetStrokeStyle( - gradient.to_fill_or_stroke_style(), - )); }, StringOrCanvasGradientOrCanvasPattern::CanvasPattern(pattern) => { self.state.borrow_mut().stroke_style = CanvasFillOrStrokeStyle::Pattern(Dom::from_ref(&*pattern)); - self.send_canvas_2d_msg(Canvas2dMsg::SetStrokeStyle( - pattern.to_fill_or_stroke_style(), - )); if !pattern.origin_is_clean() { self.set_origin_unclean(); } @@ -781,6 +774,15 @@ impl CanvasState { } } + fn update_stroke_style(&self) { + let style = match &self.state.borrow().stroke_style { + CanvasFillOrStrokeStyle::Color(rgba) => FillOrStrokeStyle::Color(*rgba), + CanvasFillOrStrokeStyle::Gradient(gradient) => gradient.to_fill_or_stroke_style(), + CanvasFillOrStrokeStyle::Pattern(pattern) => pattern.to_fill_or_stroke_style(), + }; + self.send_canvas_2d_msg(Canvas2dMsg::SetStrokeStyle(style)); + } + // https://html.spec.whatwg.org/multipage/#dom-context-2d-strokestyle pub fn fill_style(&self) -> StringOrCanvasGradientOrCanvasPattern { match self.state.borrow().fill_style { @@ -808,24 +810,15 @@ impl CanvasState { StringOrCanvasGradientOrCanvasPattern::String(string) => { if let Ok(rgba) = self.parse_color(canvas, &string) { self.state.borrow_mut().fill_style = CanvasFillOrStrokeStyle::Color(rgba); - self.send_canvas_2d_msg(Canvas2dMsg::SetFillStyle(FillOrStrokeStyle::Color( - rgba, - ))) } }, StringOrCanvasGradientOrCanvasPattern::CanvasGradient(gradient) => { self.state.borrow_mut().fill_style = CanvasFillOrStrokeStyle::Gradient(Dom::from_ref(&*gradient)); - self.send_canvas_2d_msg(Canvas2dMsg::SetFillStyle( - gradient.to_fill_or_stroke_style(), - )); }, StringOrCanvasGradientOrCanvasPattern::CanvasPattern(pattern) => { self.state.borrow_mut().fill_style = CanvasFillOrStrokeStyle::Pattern(Dom::from_ref(&*pattern)); - self.send_canvas_2d_msg(Canvas2dMsg::SetFillStyle( - pattern.to_fill_or_stroke_style(), - )); if !pattern.origin_is_clean() { self.set_origin_unclean(); } @@ -833,6 +826,15 @@ impl CanvasState { } } + fn update_fill_style(&self) { + let style = match &self.state.borrow().fill_style { + CanvasFillOrStrokeStyle::Color(rgba) => FillOrStrokeStyle::Color(*rgba), + CanvasFillOrStrokeStyle::Gradient(gradient) => gradient.to_fill_or_stroke_style(), + CanvasFillOrStrokeStyle::Pattern(pattern) => pattern.to_fill_or_stroke_style(), + }; + self.send_canvas_2d_msg(Canvas2dMsg::SetFillStyle(style)); + } + // https://html.spec.whatwg.org/multipage/#dom-context-2d-createlineargradient pub fn create_linear_gradient( &self, @@ -995,6 +997,7 @@ impl CanvasState { // https://html.spec.whatwg.org/multipage/#dom-context-2d-filltext pub fn fill_text(&self, text: DOMString, x: f64, y: f64, max_width: Option) { let parsed_text: String = text.into(); + self.update_fill_style(); self.send_canvas_2d_msg(Canvas2dMsg::FillText(parsed_text, x, y, max_width)); } @@ -1302,11 +1305,13 @@ impl CanvasState { // https://html.spec.whatwg.org/multipage/#dom-context-2d-fill pub fn fill(&self, _fill_rule: CanvasFillRule) { // TODO: Process fill rule + self.update_fill_style(); self.send_canvas_2d_msg(Canvas2dMsg::Fill); } // https://html.spec.whatwg.org/multipage/#dom-context-2d-stroke pub fn stroke(&self) { + self.update_stroke_style(); self.send_canvas_2d_msg(Canvas2dMsg::Stroke); } diff --git a/tests/wpt/metadata/2dcontext/fill-and-stroke-styles/2d.gradient.object.update.html.ini b/tests/wpt/metadata/2dcontext/fill-and-stroke-styles/2d.gradient.object.update.html.ini deleted file mode 100644 index 5ab8ded6255..00000000000 --- a/tests/wpt/metadata/2dcontext/fill-and-stroke-styles/2d.gradient.object.update.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.gradient.object.update.html] - type: testharness - [Canvas test: 2d.gradient.object.update] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/fill-and-stroke-styles/2d.pattern.animated.gif.html.ini b/tests/wpt/metadata/2dcontext/fill-and-stroke-styles/2d.pattern.animated.gif.html.ini deleted file mode 100644 index 8dc7eba742c..00000000000 --- a/tests/wpt/metadata/2dcontext/fill-and-stroke-styles/2d.pattern.animated.gif.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[2d.pattern.animated.gif.html] - [createPattern() of an animated GIF draws the first frame] - expected: FAIL - diff --git a/tests/wpt/metadata/offscreen-canvas/fill-and-stroke-styles/2d.gradient.object.update.html.ini b/tests/wpt/metadata/offscreen-canvas/fill-and-stroke-styles/2d.gradient.object.update.html.ini deleted file mode 100644 index fbbb458645d..00000000000 --- a/tests/wpt/metadata/offscreen-canvas/fill-and-stroke-styles/2d.gradient.object.update.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[2d.gradient.object.update.html] - [OffscreenCanvas test: 2d.gradient.object.update] - expected: FAIL - diff --git a/tests/wpt/metadata/offscreen-canvas/fill-and-stroke-styles/2d.gradient.object.update.worker.js.ini b/tests/wpt/metadata/offscreen-canvas/fill-and-stroke-styles/2d.gradient.object.update.worker.js.ini deleted file mode 100644 index 1676060bf3c..00000000000 --- a/tests/wpt/metadata/offscreen-canvas/fill-and-stroke-styles/2d.gradient.object.update.worker.js.ini +++ /dev/null @@ -1,4 +0,0 @@ -[2d.gradient.object.update.worker.html] - [2d] - expected: FAIL - From 5a773bf55a3dfd1b486887c3c36bafe0cbe58f2e Mon Sep 17 00:00:00 2001 From: pylbrecht Date: Sat, 25 Jan 2020 21:29:36 +0100 Subject: [PATCH 2/2] Send fill/stroke style along with drawing message --- components/canvas/canvas_paint_thread.rs | 27 +++++++++---- components/canvas_traits/canvas.rs | 12 +++--- components/script/canvas_state.rs | 48 ++++++++++-------------- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/components/canvas/canvas_paint_thread.rs b/components/canvas/canvas_paint_thread.rs index a1932a0e56f..80017dc194a 100644 --- a/components/canvas/canvas_paint_thread.rs +++ b/components/canvas/canvas_paint_thread.rs @@ -121,16 +121,29 @@ impl<'a> CanvasPaintThread<'a> { fn process_canvas_2d_message(&mut self, message: Canvas2dMsg, canvas_id: CanvasId) { match message { - Canvas2dMsg::FillText(text, x, y, max_width) => { - self.canvas(canvas_id).fill_text(text, x, y, max_width) + Canvas2dMsg::FillText(text, x, y, max_width, style) => { + self.canvas(canvas_id).set_fill_style(style); + self.canvas(canvas_id).fill_text(text, x, y, max_width); + }, + Canvas2dMsg::FillRect(rect, style) => { + self.canvas(canvas_id).set_fill_style(style); + self.canvas(canvas_id).fill_rect(&rect); + }, + Canvas2dMsg::StrokeRect(rect, style) => { + self.canvas(canvas_id).set_stroke_style(style); + self.canvas(canvas_id).stroke_rect(&rect); }, - Canvas2dMsg::FillRect(ref rect) => self.canvas(canvas_id).fill_rect(rect), - Canvas2dMsg::StrokeRect(ref rect) => self.canvas(canvas_id).stroke_rect(rect), Canvas2dMsg::ClearRect(ref rect) => self.canvas(canvas_id).clear_rect(rect), Canvas2dMsg::BeginPath => self.canvas(canvas_id).begin_path(), Canvas2dMsg::ClosePath => self.canvas(canvas_id).close_path(), - Canvas2dMsg::Fill => self.canvas(canvas_id).fill(), - Canvas2dMsg::Stroke => self.canvas(canvas_id).stroke(), + Canvas2dMsg::Fill(style) => { + self.canvas(canvas_id).set_fill_style(style); + self.canvas(canvas_id).fill(); + }, + Canvas2dMsg::Stroke(style) => { + self.canvas(canvas_id).set_stroke_style(style); + self.canvas(canvas_id).stroke(); + }, Canvas2dMsg::Clip => self.canvas(canvas_id).clip(), Canvas2dMsg::IsPointInPath(x, y, fill_rule, chan) => self .canvas(canvas_id) @@ -192,8 +205,6 @@ impl<'a> CanvasPaintThread<'a> { .ellipse(center, radius_x, radius_y, rotation, start, end, ccw), Canvas2dMsg::RestoreContext => self.canvas(canvas_id).restore_context_state(), Canvas2dMsg::SaveContext => self.canvas(canvas_id).save_context_state(), - Canvas2dMsg::SetFillStyle(style) => self.canvas(canvas_id).set_fill_style(style), - Canvas2dMsg::SetStrokeStyle(style) => self.canvas(canvas_id).set_stroke_style(style), Canvas2dMsg::SetLineWidth(width) => self.canvas(canvas_id).set_line_width(width), Canvas2dMsg::SetLineCap(cap) => self.canvas(canvas_id).set_line_cap(cap), Canvas2dMsg::SetLineJoin(join) => self.canvas(canvas_id).set_line_join(join), diff --git a/components/canvas_traits/canvas.rs b/components/canvas_traits/canvas.rs index ee659e16f3a..b203399a200 100644 --- a/components/canvas_traits/canvas.rs +++ b/components/canvas_traits/canvas.rs @@ -44,9 +44,9 @@ pub enum Canvas2dMsg { Clip, ClosePath, Ellipse(Point2D, f32, f32, f32, f32, f32, bool), - Fill, - FillText(String, f64, f64, Option), - FillRect(Rect), + Fill(FillOrStrokeStyle), + FillText(String, f64, f64, Option, FillOrStrokeStyle), + FillRect(Rect, FillOrStrokeStyle), GetImageData(Rect, Size2D, IpcBytesSender), IsPointInPath(f64, f64, FillRule, IpcSender), LineTo(Point2D), @@ -56,10 +56,8 @@ pub enum Canvas2dMsg { Rect(Rect), RestoreContext, SaveContext, - StrokeRect(Rect), - Stroke, - SetFillStyle(FillOrStrokeStyle), - SetStrokeStyle(FillOrStrokeStyle), + StrokeRect(Rect, FillOrStrokeStyle), + Stroke(FillOrStrokeStyle), SetLineWidth(f32), SetLineCap(LineCapStyle), SetLineJoin(LineJoinStyle), diff --git a/components/script/canvas_state.rs b/components/script/canvas_state.rs index c59cb3c8823..c9492ed5da5 100644 --- a/components/script/canvas_state.rs +++ b/components/script/canvas_state.rs @@ -64,6 +64,16 @@ pub(crate) enum CanvasFillOrStrokeStyle { Pattern(Dom), } +impl CanvasFillOrStrokeStyle { + fn to_fill_or_stroke_style(&self) -> FillOrStrokeStyle { + match self { + CanvasFillOrStrokeStyle::Color(rgba) => FillOrStrokeStyle::Color(*rgba), + CanvasFillOrStrokeStyle::Gradient(gradient) => gradient.to_fill_or_stroke_style(), + CanvasFillOrStrokeStyle::Pattern(pattern) => pattern.to_fill_or_stroke_style(), + } + } +} + #[unrooted_must_root_lint::must_root] #[derive(Clone, JSTraceable, MallocSizeOf)] pub(crate) struct CanvasContextState { @@ -654,8 +664,8 @@ impl CanvasState { // https://html.spec.whatwg.org/multipage/#dom-context-2d-fillrect pub fn fill_rect(&self, x: f64, y: f64, width: f64, height: f64) { if let Some(rect) = self.create_drawable_rect(x, y, width, height) { - self.update_fill_style(); - self.send_canvas_2d_msg(Canvas2dMsg::FillRect(rect)); + let style = self.state.borrow().fill_style.to_fill_or_stroke_style(); + self.send_canvas_2d_msg(Canvas2dMsg::FillRect(rect, style)); } } @@ -669,8 +679,8 @@ impl CanvasState { // https://html.spec.whatwg.org/multipage/#dom-context-2d-strokerect pub fn stroke_rect(&self, x: f64, y: f64, width: f64, height: f64) { if let Some(rect) = self.create_drawable_rect(x, y, width, height) { - self.update_stroke_style(); - self.send_canvas_2d_msg(Canvas2dMsg::StrokeRect(rect)); + let style = self.state.borrow().stroke_style.to_fill_or_stroke_style(); + self.send_canvas_2d_msg(Canvas2dMsg::StrokeRect(rect, style)); } } @@ -774,15 +784,6 @@ impl CanvasState { } } - fn update_stroke_style(&self) { - let style = match &self.state.borrow().stroke_style { - CanvasFillOrStrokeStyle::Color(rgba) => FillOrStrokeStyle::Color(*rgba), - CanvasFillOrStrokeStyle::Gradient(gradient) => gradient.to_fill_or_stroke_style(), - CanvasFillOrStrokeStyle::Pattern(pattern) => pattern.to_fill_or_stroke_style(), - }; - self.send_canvas_2d_msg(Canvas2dMsg::SetStrokeStyle(style)); - } - // https://html.spec.whatwg.org/multipage/#dom-context-2d-strokestyle pub fn fill_style(&self) -> StringOrCanvasGradientOrCanvasPattern { match self.state.borrow().fill_style { @@ -826,15 +827,6 @@ impl CanvasState { } } - fn update_fill_style(&self) { - let style = match &self.state.borrow().fill_style { - CanvasFillOrStrokeStyle::Color(rgba) => FillOrStrokeStyle::Color(*rgba), - CanvasFillOrStrokeStyle::Gradient(gradient) => gradient.to_fill_or_stroke_style(), - CanvasFillOrStrokeStyle::Pattern(pattern) => pattern.to_fill_or_stroke_style(), - }; - self.send_canvas_2d_msg(Canvas2dMsg::SetFillStyle(style)); - } - // https://html.spec.whatwg.org/multipage/#dom-context-2d-createlineargradient pub fn create_linear_gradient( &self, @@ -997,8 +989,8 @@ impl CanvasState { // https://html.spec.whatwg.org/multipage/#dom-context-2d-filltext pub fn fill_text(&self, text: DOMString, x: f64, y: f64, max_width: Option) { let parsed_text: String = text.into(); - self.update_fill_style(); - self.send_canvas_2d_msg(Canvas2dMsg::FillText(parsed_text, x, y, max_width)); + let style = self.state.borrow().fill_style.to_fill_or_stroke_style(); + self.send_canvas_2d_msg(Canvas2dMsg::FillText(parsed_text, x, y, max_width, style)); } // https://html.spec.whatwg.org/multipage/#textmetrics @@ -1305,14 +1297,14 @@ impl CanvasState { // https://html.spec.whatwg.org/multipage/#dom-context-2d-fill pub fn fill(&self, _fill_rule: CanvasFillRule) { // TODO: Process fill rule - self.update_fill_style(); - self.send_canvas_2d_msg(Canvas2dMsg::Fill); + let style = self.state.borrow().fill_style.to_fill_or_stroke_style(); + self.send_canvas_2d_msg(Canvas2dMsg::Fill(style)); } // https://html.spec.whatwg.org/multipage/#dom-context-2d-stroke pub fn stroke(&self) { - self.update_stroke_style(); - self.send_canvas_2d_msg(Canvas2dMsg::Stroke); + let style = self.state.borrow().stroke_style.to_fill_or_stroke_style(); + self.send_canvas_2d_msg(Canvas2dMsg::Stroke(style)); } // https://html.spec.whatwg.org/multipage/#dom-context-2d-clip