Support OffscreenCanvas as CanvasImageSource

[jdm]

Code:

• servo/components/script/dom/webidls/CanvasRenderingContext2D.webidl

  Line 15 in 4fe8238

  /*OffscreenCanvas or*/

• servo/components/script/dom/canvasrenderingcontext2d.rs

  Lines 335 to 350 in 4fe8238

  	CanvasImageSource::HTMLCanvasElement(ref canvas) => {
  	self.draw_html_canvas_element(&canvas, htmlcanvas, sx, sy, sw, sh, dx, dy, dw, dh)
  	},
  	CanvasImageSource::HTMLImageElement(ref image) => {
  	// https://html.spec.whatwg.org/multipage/#img-error
  	// If the image argument is an HTMLImageElement object that is in the broken state,
  	// then throw an InvalidStateError exception
  	let url = image.get_url().ok_or(Error::InvalidState)?;
  	self.fetch_and_draw_image_data(htmlcanvas, url, sx, sy, sw, sh, dx, dy, dw, dh)
  	},
  	CanvasImageSource::CSSStyleValue(ref value) => {
  	let url = value
  	.get_url(self.base_url.clone())
  	.ok_or(Error::InvalidState)?;
  	self.fetch_and_draw_image_data(htmlcanvas, url, sx, sy, sw, sh, dx, dy, dw, dh)
  	},

• servo/components/script/dom/canvasrenderingcontext2d.rs

  Lines 359 to 419 in 4fe8238

  	fn draw_html_canvas_element(
  	&self,
  	canvas: &HTMLCanvasElement,
  	htmlcanvas: Option<&HTMLCanvasElement>,
  	sx: f64,
  	sy: f64,
  	sw: Option<f64>,
  	sh: Option<f64>,
  	dx: f64,
  	dy: f64,
  	dw: Option<f64>,
  	dh: Option<f64>,
  	) -> ErrorResult {
  	// 1. Check the usability of the image argument
  	if !canvas.is_valid() {
  	return Err(Error::InvalidState);
  	}
  	let canvas_size = canvas.get_size();
  	let dw = dw.unwrap_or(canvas_size.width as f64);
  	let dh = dh.unwrap_or(canvas_size.height as f64);
  	let sw = sw.unwrap_or(canvas_size.width as f64);
  	let sh = sh.unwrap_or(canvas_size.height as f64);
  	let image_size = Size2D::new(canvas_size.width as f64, canvas_size.height as f64);
  	// 2. Establish the source and destination rectangles
  	let (source_rect, dest_rect) =
  	self.adjust_source_dest_rects(image_size, sx, sy, sw, sh, dx, dy, dw, dh);
  	if !is_rect_valid(source_rect) || !is_rect_valid(dest_rect) {
  	return Ok(());
  	}
  	let smoothing_enabled = self.state.borrow().image_smoothing_enabled;
  	if let Some(context) = canvas.context() {
  	match *context {
  	CanvasContext::Context2d(ref context) => {
  	context.send_canvas_2d_msg(Canvas2dMsg::DrawImageInOther(
  	self.get_canvas_id(),
  	image_size,
  	dest_rect,
  	source_rect,
  	smoothing_enabled,
  	));
  	},
  	_ => return Err(Error::InvalidState),
  	}
  	} else {
  	self.send_canvas_2d_msg(Canvas2dMsg::DrawImage(
  	None,
  	image_size,
  	dest_rect,
  	source_rect,
  	smoothing_enabled,
  	));
  	}
  	self.mark_as_dirty(htmlcanvas);
  	Ok(())
  	}

Tests:

• ./mach test-wpt tests/wpt/web-platform-tests/offscreen-canvas/drawing-images-to-the-canvas should report newly passing tests

We'll want to generalize draw_html_canvas_element to support offscreen canvases as well, since the majority of the code is applicable to both. An enum argument is probably the best way to do that.

[bblanke]

On this

[bblanke]

Okay! I made some changes to the repo, and there are now 30 new tests passing! However, none of them are in drawing-images-to-the-canvas. I'm trying to figure out what's missing.

I added a match statement in canvas_state.rs to call the proper draw function when an offscreen canvas is passed in. I then created my own version of the draw_html_canvas_element_offscreen that basically does the same as the original function, but instead of matching a "normal" canvas's context, it matches for an OffscreenCanvasContext and sends it a message to draw the source's canvas into it. Do you see anything inherently wrong with this?

I'd appreciate any help that you could offer! Also, how do you effectively debug? Is there a way to view print statements from Rust code while running tests? Do I need to just bite the bullet and learn how to use GDB? What's your best advice on this?

Thanks so much for your time!

[jdm]

It's most effective to run a single test along with the --log-raw - argument. This will let you see the output of println! in your terminal. Alternatively, if you use macros like info!/debug!/warn!/etc. in a file like components/canvas/canvas_paint_thread.rs or components/script/dom/offscreencanvas.rs, then you can run tests with RUST_LOG=script::dom::offscreencanvas,canvas::canvas_paint_thread to see the output (that's an environment variable, in case it's unclear).

As for the code snippets, those seem reasonable to me (and nothing else in master...bblanke:OffscreenCanvasasCanvasImageSoure seems unreasonable). I recommend deleting the contents of https://github.com/bblanke/servo/blob/13ba9da91cd840fe721e1caa8141f8427b5c8058/tests/wpt/metadata/offscreen-canvas/drawing-images-to-the-canvas/2d.drawImage.canvas.html.ini and running that test to see the exact failing output that is reported. That might help narrow down where the failure is coming from.

[jdm]

As for gdb, that can definitely be an efficient way to figure out if code is actually running. You can go a long way with the following commands: run (start the program execution), break file.rs:45 (interrupt execution when line 45 of file.rs is reached), continue (continue executing from a breakpoint), next (advance to the next line), step (step into a function call), finish (continue execution until it has returned from the current function)

You can use ./mach test-wpt path/to/test.html --debugger to launch a gdb session with the test ready to start, set some breakpoints, then use run to start execution.

[bblanke]

GDB is working great
I had to use this instead of the shorthand:
breakpoint set --file foo.c --line 12

Good article for anyone else that finds themselves here: https://lldb.llvm.org/use/tutorial.html

[jdm]

Ah, when you said GDB I assumed you were actually using GDB, not LLDB :)

[bblanke]

Lol shoot yeah lldb! Through lldb, I found out that the functions that I expect to get called are indeed getting called (specifically, draw_html_canvas_offscreen). I also know that the test is failing because there's a location where the code expects a colored pixel, but the pixel is black. So, I'm wondering if there's something wrong with the way that I request the destination canvas to draw the source canvas onto it (through Canvas2dMsg::DrawImageInOther)? Does OffscreenCanvas need something added to it to make it support this operation?

[jdm]

I recommend adding some assertions to the test to verify that the canvas that's being used as the image source contains the expected pixels before the draw operation occurs. That should narrow down where the unexpected behaviour is stemming from.

[bblanke]

Figured it out! The Offscreen Canvas's width and height properties were switched. I fixed that and ran tests and now 830 new tests are passing...

Few questions:

1. The code builds and all of the tests unexpectedly pass (no fails)...is there still a possibility that I just screwed something up? or is this just a really lucky bug fix?
2. Is there an easier way to delete the metadata for the passing tests? I could look into writing a script (maybe extend mach?) if there isn't one yet.

[jdm]

Woo! I'm pretty sure this is just a lucky bug fix if a lot of tests happen to rely on using OffscreenCanvas as an image source. We already pass lots of onscreen canvas tests and the offscreen canvas uses the exact same backend, so this result makes sense to me. You can run the tests for a particular directory with --log-raw wptlog and then run ./mach update-wpt wptlog and the metadata will be updated for you.

[jdm]

With very convenient timing, we just merged 03e574f which incorporated web-platform-tests/wpt#20277. You will need to rebase your changes to master and regenerate the test results; I suspect they will be less overwhelmingly fail->pass as a result.

[bblanke]

Alright; I think I have it now. Submitted an incorrect commit earlier.