Add a simple implementation of CanvasRenderingContext2d.fillText #25782

kaiakz · 2020-02-17T09:46:57Z

I added a simple implementation of CanvasRenderingContext2d.fillText.
Some code are merged from @mikrut, and I fixed a bug about text scaling.
Also, the bug of text rotation should be fixed after raqote merged my other PR.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix (part of) #11681 (GitHub issue number if applicable)

There are tests for these changes

highfive · 2020-02-17T09:47:09Z

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

jdm · 2020-02-21T08:55:29Z

@bors-servo try=wpt

bors-servo · 2020-02-21T08:55:35Z

⌛ Trying commit 573b6a4 with merge a2bb672...


        Auto merge of #25782 - kaiakz:master, r=<try>

@mikrut

Add a simple implementation of CanvasRenderingContext2d.fillText  I added a simple implementation of CanvasRenderingContext2d.fillText. Some code are merged from @mikrut, and I fixed a bug about text scaling. Also, the bug of text rotation should be fixed after `raqote` merged my other PR. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #11681 (GitHub issue number if applicable)  - [X] There are tests for these changes

bors-servo · 2020-02-21T09:39:36Z

💔 Test failed - status-taskcluster

jdm · 2020-02-21T09:54:20Z

  ▶ FAIL [expected PASS] /2dcontext/text-styles/canvas_text_font_001.htm
  │   → /2dcontext/text-styles/canvas_text_font_001.htm 54ed3fe6baf2d8bc9bddcdb0e82725a5c568a6e6
  └   → /2dcontext/text-styles/canvas_text_font_001-ref.htm 652917f3c91833309587adc905f58a56e6998530

This means that you will need to run:

./mach test-wpt /2dcontext/text-styles/canvas_text_font_001.htm --log-raw wpt
./mach update-wpt
git add tests/wpt/metadata/2dcontext
And commit the changes.

kaiakz · 2020-02-21T11:30:00Z

@jdm
./mach test-wpt /2dcontext/text-styles/canvas_text_font_001.htm --log-raw wpt says:

 0:03.57 INFO Using 8 client processes
 0:03.66 INFO Starting http server on 127.0.0.1:8000
 0:03.66 INFO Starting http server on 127.0.0.1:8001
 0:03.68 INFO Starting https server on 127.0.0.1:8443
 0:04.15 SUITE_START: web-platform-test - running 1 tests
 0:04.15 INFO Running reftest tests
 0:04.16 INFO Starting runner
 0:04.17 INFO No more tests
 0:04.17 INFO No more tests
 0:04.17 INFO No more tests
 0:04.17 INFO No more tests
 0:04.18 INFO No more tests
 0:04.18 TEST_START: /2dcontext/text-styles/canvas_text_font_001.htm
 0:04.18 INFO No more tests
 0:04.18 WARNING Got command init_succeeded in state running
 0:04.18 INFO No more tests
 0:04.56 pid:8768 Full command: /run/media/kaiakz/Data/moz/folk/servo/target/debug/servo --hard-fail -u Servo/wptrunner -z http://web-platform.test:8000/2dcontext/text-styles/canvas_text_font_001.htm -Z disable-text-aa,load-webfonts-synchronously,replace-surrogates --user-stylesheet /run/media/kaiakz/Data/moz/folk/servo/resources/ahem.css --pref dom.canvas-text.enabled=false --certificate-path /run/media/kaiakzi/Data/moz/folk/servo/tests/wpt/web-platform-tests/tools/certs/cacert.pem --exit --output=/tmp/tmpsNxVdR/39f64eea-4444-4292-b235-2488ae0592ff --resolution 800x600
pid:8768 
 0:04.56 pid:8768 
 0:04.57 pid:8768 
 0:08.77 pid:8768 [2020-02-21T11:20:24Z ERROR script::dom::bindings::error] Error at http://web-platform.test:8000/2dcontext/text-styles/canvas_text_font_001.htm:20:21 ctx.fillText is not a function
 0:09.62 pid:8768 Shutting down the Constellation after generating an output file or exit flag specified
 0:09.90 pid:8864 Full command: /run/media/kaiakz/Data/moz/folk/servo/target/debug/servo --hard-fail -u Servo/wptrunner -z http://web-platform.test:8000/2dcontext/text-styles/canvas_text_font_001-ref.htm -Z disable-text-aa,load-webfonts-synchronously,replace-surrogates --user-stylesheet /run/media/kaiakz/Data/moz/folk/servo/resources/ahem.css --certificate-path /run/media/kaiakz/Data/moz/folk/servo/tests/wpt/web-platform-tests/tools/certs/cacert.pem --exit --output=/tmp/tmpsNxVdR/a15d030f-1666-4ac9-b93a-a63583a11768 --resolution 800x600
pid:8864 
 0:09.90 pid:8864 
 0:09.90 pid:8864 
 0:14.39 pid:8864 Shutting down the Constellation after generating an output file or exit flag specified
 0:15.21 INFO Image hashes didn't match, checking pixel differences
 0:15.32 INFO Found 900 pixels different, maximum difference per channel 255
 0:15.32 TEST_END: FAIL, expected PASS - /2dcontext/text-styles/canvas_text_font_001.htm 1fa1aef7b66b7af2d04621d380782299ec9c0e10
/2dcontext/text-styles/canvas_text_font_001-ref.htm d8c11dca7773f5977e977513c7310bd60d853355
 0:15.32 INFO No more tests
 0:15.33 INFO Closing logging queue
 0:15.33 INFO queue closed
 0:15.33 INFO Running wdspec tests
 0:15.33 INFO No wdspec tests to run
 0:15.33 INFO Running testharness tests
 0:15.33 INFO No testharness tests to run
 0:15.33 INFO Running crashtest tests
 0:15.33 INFO No crashtest tests to run
 0:15.33 INFO Got 1 unexpected results
 0:15.33 SUITE_END

web-platform-test
~~~~~~~~~~~~~~~~~
Ran 1 checks (1 tests)
Expected results: 0
Unexpected results: 1
  test: 1 (1 fail)

Unexpected Results
------------------
FAIL /2dcontext/text-styles/canvas_text_font_001.htm - /2dcontext/text-styles/canvas_text_font_001.htm 1fa1aef7b66b7af2d04621d380782299ec9c0e10
/2dcontext/text-styles/canvas_text_font_001-ref.htm d8c11dca7773f5977e977513c7310bd60d853355
 0:15.37 INFO Closing logging queue
 0:15.37 INFO queue closed

How can I fix it? Does it mean I have to implement the canvas_text_font?

jdm · 2020-02-21T14:18:39Z

@kaiakz I gave you the steps to fix it - the current implementation test fails because the reference rendering does not match the test rendering, because the font support isn't implemented. The steps I suggested update the expected test result to indicate that the test will not pass, so the test failure won't be reported as a problem.

jdm · 2020-02-21T15:18:57Z

r? @pcwalton
Could you check if this looks like a reasonable first cut at a canvas text implementation, disregarding the lack of font selection?

pcwalton · 2020-02-21T20:48:13Z

components/canvas/raqote_backend.rs

+        pattern: canvas_data::Pattern,
+        draw_options: &DrawOptions,
+    ) {
+        let font = SystemSource::new()


Needs a TODO because this should eventually use the same infrastructure as layout (i.e. layout should be updated to use font-kit as well).

Also, this is in the chrome process, right? It needs to not be in the content process because the content process shouldn't have access to the SystemSource. (It's too dangerous; on macOS for example it needs a connection to the font server, which has had a lot of security vulnerabilities.)

Yes, the canvas code runs in the chrome process.

So, I need to do:

add a TODO to raqote_backend.rs

move font_kit code from raqote_backend.rs to canvas_data.rs and pass the SystemSource things through` the parameter?(Well, my code made some compromised, since I am not familiar with all components.)

Also, this is in the chrome process, right? It needs to not be in the content process because the content process shouldn't have access to the SystemSource.

I noticed there is a FontCacheThread, maybe I can use it?

Let's keep the font_kit code in raqote_backend.rs for now until it's more clear how the proper font integration will work. The FontCacheThread is integrated with many other parts of the system, so converting that code to use font_kit will be part of resolving that TODO.

@jdm
If I understood well, I moved all font_kit code to raqote_backend.rs, including some functions:
replace_whitespace & get_text_width( They are parts of text preparation algorithm, in fact, I just moved the algorithm to raqote_backend.rs).

I would like to do the converting if have any spare time. 😄

bors-servo · 2020-03-19T12:32:20Z

⌛ Testing commit 777b9d7 with merge 8d9a099...


        Auto merge of #25782 - kaiakz:master, r=jdm

@mikrut

Add a simple implementation of CanvasRenderingContext2d.fillText  I added a simple implementation of CanvasRenderingContext2d.fillText. Some code are merged from @mikrut, and I fixed a bug about text scaling. Also, the bug of text rotation should be fixed after `raqote` merged my other PR. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix (part of) #11681 (GitHub issue number if applicable)  - [X] There are tests for these changes

bors-servo · 2020-03-19T13:11:16Z

💔 Test failed - status-taskcluster

pylbrecht · 2020-03-19T14:04:39Z

The layout-2020 equivalents must be updated as well (see #25940 (comment) for an explanation why). The layout-2020 tests' metadata files are located in tests/wpt/metadata-layout-2020/.

1 unexpected results that are NOT known-intermittents:
  ▶ Unexpected subtest result in /2dcontext/line-styles/2d.line.cap.closed.html:
  └ PASS [expected FAIL] Line caps are not drawn at the corners of an unclosed rectangle

2 unexpected results that are NOT known-intermittents:
  ▶ Unexpected subtest result in /2dcontext/fill-and-stroke-styles/2d.gradient.interpolate.zerosize.fillText.html:
  │ FAIL [expected PASS] Canvas test: 2d.gradient.interpolate.zerosize.fillText
  │   → assert_equals: d[12824] expected 0 but got 28
  │ 
  │ testColor@http://web-platform.test:8000/2dcontext/resources/canvas-tests.js:78:22
  │ _assertGreen@http://web-platform.test:8000/2dcontext/resources/canvas-tests.js:84:22
  │ @http://web-platform.test:8000/2dcontext/fill-and-stroke-styles/2d.gradient.interpolate.zerosize.fillText.html:31:13
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1958:25
  │ _addTest/</<@http://web-platform.test:8000/2dcontext/resources/canvas-tests.js:66:15
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1958:25
  └ _addTest/<@http://web-platform.test:8000/2dcontext/resources/canvas-tests.js:63:11
  ▶ Unexpected subtest result in /2dcontext/fill-and-stroke-styles/2d.gradient.radial.inside3.html:
  └ PASS [expected FAIL] Canvas test: 2d.gradient.radial.inside3

kaiakz · 2020-03-19T14:23:23Z

@pylbrecht
How did you update the layout-2020 tests' metadata files? ./mach test-wpt & ./mach update-wpt do not work for those.

jdm · 2020-03-19T14:31:58Z

Sorry, let's just wait for #25960 to merge.

pylbrecht · 2020-03-19T14:32:09Z

I did it manually: copying the corresponding .ini file from the regular tests/wpt/metadata/ directory to the tests/wpt/metadata-layout-2020/ directory in case of a FAIL, or deleting the corresponding .ini file in case of a PASS.
But I think you also can wait until #25960 is merged.

jdm · 2020-03-19T15:02:58Z

@bors-servo retry

bors-servo · 2020-03-19T15:39:14Z

⌛ Testing commit 777b9d7 with merge 3735f7a...


        Auto merge of #25782 - kaiakz:master, r=jdm

@mikrut

Add a simple implementation of CanvasRenderingContext2d.fillText  I added a simple implementation of CanvasRenderingContext2d.fillText. Some code are merged from @mikrut, and I fixed a bug about text scaling. Also, the bug of text rotation should be fixed after `raqote` merged my other PR. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix (part of) #11681 (GitHub issue number if applicable)  - [X] There are tests for these changes

bors-servo · 2020-03-19T16:37:39Z

💔 Test failed - status-taskcluster

jdm · 2020-03-19T16:43:27Z

@bors-servo retry

#23290

bors-servo · 2020-03-19T17:24:10Z

⌛ Testing commit 777b9d7 with merge ffdb83b...

bors-servo · 2020-03-19T17:44:44Z

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing ffdb83b to master...

paulrouget · 2020-03-23T11:35:59Z

This makes servo crash on Windows: #26015

highfive assigned asajeffrey Feb 17, 2020

highfive added the S-awaiting-review label Feb 17, 2020

kaiakz requested a review from jdm Feb 17, 2020

jdm assigned jdm and unassigned asajeffrey Feb 21, 2020

jdm approved these changes Feb 21, 2020

View changes

highfive added the S-tests-failed label Feb 21, 2020

jdm added S-needs-code-changes and removed S-awaiting-review labels Feb 21, 2020

highfive added S-awaiting-review and removed S-tests-failed S-needs-code-changes labels Feb 21, 2020

highfive assigned pcwalton and unassigned jdm Feb 21, 2020

pcwalton reviewed Feb 21, 2020

View changes

jdm assigned jdm and unassigned pcwalton Feb 24, 2020

jdm added S-needs-code-changes and removed S-awaiting-review labels Feb 24, 2020

highfive added S-awaiting-review and removed S-needs-code-changes labels Feb 25, 2020

highfive added S-awaiting-merge and removed S-awaiting-review S-tests-failed labels Mar 19, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Mar 19, 2020

highfive added S-awaiting-merge and removed S-tests-failed labels Mar 19, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Mar 19, 2020

highfive added S-awaiting-merge and removed S-tests-failed labels Mar 19, 2020

bors-servo merged commit ffdb83b into servo:master Mar 19, 2020
2 checks passed

2 checks passed

Community-TC (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Mar 19, 2020

utsavoza mentioned this pull request May 28, 2020

Implement CanvasRenderingContext2d.fillText #11681

Closed

servo / servo

Add a simple implementation of CanvasRenderingContext2d.fillText #25782

Add a simple implementation of CanvasRenderingContext2d.fillText #25782

kaiakz commented Feb 17, 2020 •

edited by jdm

highfive commented Feb 17, 2020

jdm commented Feb 21, 2020

bors-servo commented Feb 21, 2020

bors-servo commented Feb 21, 2020

jdm commented Feb 21, 2020

kaiakz commented Feb 21, 2020 •

edited

jdm commented Feb 21, 2020

jdm commented Feb 21, 2020

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.

bors-servo commented Mar 19, 2020

bors-servo commented Mar 19, 2020

pylbrecht commented Mar 19, 2020

kaiakz commented Mar 19, 2020 •

edited

jdm commented Mar 19, 2020

pylbrecht commented Mar 19, 2020

jdm commented Mar 19, 2020

bors-servo commented Mar 19, 2020

bors-servo commented Mar 19, 2020

jdm commented Mar 19, 2020

bors-servo commented Mar 19, 2020

bors-servo commented Mar 19, 2020

paulrouget commented Mar 23, 2020

servo / servo

Join GitHub today

Add a simple implementation of CanvasRenderingContext2d.fillText #25782

Add a simple implementation of CanvasRenderingContext2d.fillText #25782

Conversation

kaiakz commented Feb 17, 2020 • edited by jdm

highfive commented Feb 17, 2020

jdm commented Feb 21, 2020

bors-servo commented Feb 21, 2020

bors-servo commented Feb 21, 2020

jdm commented Feb 21, 2020

kaiakz commented Feb 21, 2020 • edited

jdm commented Feb 21, 2020

jdm commented Feb 21, 2020

This comment has been minimized.

pcwalton Feb 21, 2020

This comment has been minimized.

pcwalton Feb 21, 2020

This comment has been minimized.

jdm Feb 21, 2020

This comment has been minimized.

kaiakz Feb 22, 2020 • edited

This comment has been minimized.

jdm Feb 24, 2020

This comment has been minimized.

kaiakz Feb 25, 2020 • edited

bors-servo commented Mar 19, 2020

bors-servo commented Mar 19, 2020

pylbrecht commented Mar 19, 2020

kaiakz commented Mar 19, 2020 • edited

jdm commented Mar 19, 2020

pylbrecht commented Mar 19, 2020

jdm commented Mar 19, 2020

bors-servo commented Mar 19, 2020

bors-servo commented Mar 19, 2020

jdm commented Mar 19, 2020

bors-servo commented Mar 19, 2020

bors-servo commented Mar 19, 2020

paulrouget commented Mar 23, 2020

kaiakz commented Feb 17, 2020 •

edited by jdm

kaiakz commented Feb 21, 2020 •

edited

kaiakz Feb 22, 2020 •

edited

kaiakz Feb 25, 2020 •

edited

kaiakz commented Mar 19, 2020 •

edited