Implement paint worklets rendering context #17326

asajeffrey · 2017-06-14T20:14:26Z

Implement the rendering context for paint worklets. They really paint things now!

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes do not require tests because the existing reftest now passes

This change is

highfive · 2017-06-14T20:14:50Z

Heads up! This PR modifies the following files:

@KiChjang: components/script/dom/paintrenderingcontext2d.rs, components/script/dom/bindings/refcounted.rs, components/script/dom/webidls/PaintWorkletGlobalScope.webidl, components/script/dom/workletglobalscope.rs, components/script/dom/canvasrenderingcontext2d.rs and 14 more
@fitzgen: components/script/dom/paintrenderingcontext2d.rs, components/script/dom/bindings/refcounted.rs, components/script/dom/webidls/PaintWorkletGlobalScope.webidl, components/script/dom/workletglobalscope.rs, components/script/dom/canvasrenderingcontext2d.rs and 14 more
@emilio: components/layout/display_list_builder.rs

highfive · 2017-06-14T20:14:56Z

Warning

These commits modify unsafe code. Please review it carefully!

asajeffrey · 2017-06-14T20:16:15Z

The commit "Implemented paint worklets invoking worklet scripts." is #17239.

asajeffrey · 2017-06-14T20:17:19Z

cc @jdm @metajack @tschneidereit

jdm · 2017-06-14T21:20:43Z

components/script/dom/paintworkletglobalscope.rs

+                    return self.send_invalid_image(size, sender);
+                }
+                // Step 5.4
+                entry.insert(Heap::new(paint_instance.get()));


I'm just going to flag this now - we must not introduce any new uses of Heap::new into Servo due to servo/rust-mozjs#343 . Use Heap::default to create a Heap value and ensure it will never move after that point before calling Heap::set. This hashtable looks very scary to me for this reason :(

Oh good point.

My proposed fix: 660d8dd

jdm · 2017-06-15T15:37:13Z

components/script/dom/paintworkletglobalscope.rs

+        let context = PaintRenderingContext2D::new(self);
+        let definition = PaintDefinition {
+            class_constructor: Heap::new(ObjectValue(paint_obj.get())),
+            paint_function: Heap::new(paint_function),


These also need to be changed.

bors-servo · 2017-06-16T09:33:20Z

☔ The latest upstream changes (presumably #17352) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2017-06-22T16:10:24Z

☔ The latest upstream changes (presumably #17456) made this pull request unmergeable. Please resolve the merge conflicts.

asajeffrey · 2017-06-30T16:00:33Z

#17239 landed, so this PR is now unblocked. @jdm?

jdm · 2017-06-30T18:11:44Z

components/script/dom/canvasrenderingcontext2d.rs

@@ -347,7 +358,7 @@ impl CanvasRenderingContext2D {

        let smoothing_enabled = self.state.borrow().image_smoothing_enabled;

-        if &*self.canvas == canvas {
+        if self.canvas.as_ref().map(|c| &**c == canvas).unwrap_or(false) {


map_or instead of map.unwrap_or

jdm · 2017-06-30T18:15:58Z

components/script/dom/paintrenderingcontext2d.rs

+        self.context.Save()
+    }
+
+    #[allow(unrooted_must_root)]


Unnecessary.

Removed. Serves me right cutting-and-pasting from CanvasRenderingContext2D.

jdm · 2017-06-30T18:18:18Z

components/script/dom/paintworkletglobalscope.rs

    {
        // TODO: document paint definitions.
        self.invoke_a_paint_callback(name, size, sender);
    }

    /// https://drafts.css-houdini.org/css-paint-api/#invoke-a-paint-callback
+    #[allow(unrooted_must_root)]


What's this for?

Apparently nothing, I removed it.

asajeffrey · 2017-06-30T21:00:39Z

@bors-servo: r=jdm

bors-servo · 2017-06-30T21:00:40Z

📌 Commit e32817c has been approved by jdm

bors-servo · 2017-06-30T21:00:45Z

⌛ Testing commit e32817c with merge f129f38...

…text, r=jdm Implement paint worklets rendering context  Implement the rendering context for paint worklets. They really paint things now! ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because the existing reftest now passes    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17326)

bors-servo · 2017-06-30T21:34:53Z

💔 Test failed - linux-rel-wpt

asajeffrey · 2017-06-30T21:41:49Z

Updated test expectations. @bors-servo r=jdm

bors-servo · 2017-06-30T21:41:50Z

📌 Commit 328fb25 has been approved by jdm

bors-servo · 2017-06-30T22:40:28Z

⌛ Testing commit 328fb25 with merge a9fea06...

…text, r=jdm Implement paint worklets rendering context  Implement the rendering context for paint worklets. They really paint things now! ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because the existing reftest now passes    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17326)

bors-servo · 2017-06-30T23:53:31Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm
Pushing a9fea06 to master...

…=jdm Implement paint worklet properties  This is the final PR to get basic paint worklet support. It adds support for paint worklet properties (https://drafts.css-houdini.org/css-paint-api/#paint-definition-input-properties). When a paint worklet is registered, it specifies a list of CSS properties, and is provided with their computed values when it is invoked. This is a dependent PR: * "Implemented paint worklets invoking worklet scripts" is #17239. * "Implemented paint worklets rendering contexts" is #17326. There should be tests added for this, hopefully the existing wpt houdini tests. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #16839 - [x] There are tests for these changes    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17364)

Fixed scaling artefacts in paint worklets caused by zoom and hidpi  This PR renders paint worklet canvases at the device pixel resolution, rather than the CSS pixel resolution. It's a dependent PR, building on #17239, #17326 and #17364. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #17454 - [X] These changes do not require tests because we don't run reftests with zoom enabled    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17499)

highfive assigned emilio Jun 14, 2017

asajeffrey added A-content/css Interacting with CSS from web content (parsing, serializing, introspection) A-content/script Related to the script thread labels Jun 14, 2017

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 14, 2017

jdm reviewed Jun 14, 2017

View reviewed changes

jdm reviewed Jun 15, 2017

View reviewed changes

highfive added the S-needs-rebase There are merge conflict errors. label Jun 16, 2017

asajeffrey mentioned this pull request Jun 16, 2017

Implement paint worklet properties #17364

Merged

4 tasks

asajeffrey force-pushed the script-paint-worklets-rendering-context branch from 660d8dd to 137760e Compare June 20, 2017 16:21

asajeffrey mentioned this pull request Jun 22, 2017

Paint worklet size should agree with Chrome. #17456

Merged

4 tasks

highfive added the S-needs-rebase There are merge conflict errors. label Jun 22, 2017

asajeffrey force-pushed the script-paint-worklets-rendering-context branch from 137760e to ab8024d Compare June 22, 2017 21:56

asajeffrey pushed a commit to asajeffrey/servo that referenced this pull request Jun 23, 2017

PR servo#17326

7bc3219

asajeffrey mentioned this pull request Jun 23, 2017

Fixed scaling artefacts in paint worklets caused by zoom and hidpi #17499

Merged

4 tasks

asajeffrey pushed a commit to asajeffrey/servo that referenced this pull request Jun 28, 2017

PR servo#17326

868c3f7

asajeffrey force-pushed the script-paint-worklets-rendering-context branch 2 times, most recently from 74c3026 to 4fcf2d7 Compare June 30, 2017 15:56

asajeffrey removed the S-needs-rebase There are merge conflict errors. label Jun 30, 2017

jdm requested changes Jun 30, 2017

View reviewed changes

jdm approved these changes Jun 30, 2017

View reviewed changes

jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 30, 2017

asajeffrey pushed a commit to asajeffrey/servo that referenced this pull request Jun 30, 2017

PR servo#17326

18ca4dd

highfive assigned jdm and unassigned emilio Jun 30, 2017

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 30, 2017

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jun 30, 2017

Implemented paint worklet rendering context.

328fb25

asajeffrey force-pushed the script-paint-worklets-rendering-context branch from e32817c to 328fb25 Compare June 30, 2017 21:41

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jun 30, 2017

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 30, 2017

bors-servo merged commit 328fb25 into servo:master Jun 30, 2017

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 30, 2017

This was referenced Jun 30, 2017

Script paint worklets arguments #17546

Merged

Moving timers to content process #17394

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement paint worklets rendering context #17326

Implement paint worklets rendering context #17326

asajeffrey commented Jun 14, 2017 •

edited by larsbergstrom

highfive commented Jun 14, 2017

highfive commented Jun 14, 2017

asajeffrey commented Jun 14, 2017

asajeffrey commented Jun 14, 2017

jdm Jun 14, 2017

asajeffrey Jun 15, 2017

asajeffrey Jun 15, 2017

jdm Jun 15, 2017

bors-servo commented Jun 16, 2017

bors-servo commented Jun 22, 2017

asajeffrey commented Jun 30, 2017

jdm Jun 30, 2017

asajeffrey Jun 30, 2017

jdm Jun 30, 2017

asajeffrey Jun 30, 2017

jdm Jun 30, 2017

asajeffrey Jun 30, 2017

asajeffrey commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

asajeffrey commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

Implement paint worklets rendering context #17326

Implement paint worklets rendering context #17326

Conversation

asajeffrey commented Jun 14, 2017 • edited by larsbergstrom

highfive commented Jun 14, 2017

highfive commented Jun 14, 2017

asajeffrey commented Jun 14, 2017

asajeffrey commented Jun 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors-servo commented Jun 16, 2017

bors-servo commented Jun 22, 2017

asajeffrey commented Jun 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asajeffrey commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

asajeffrey commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

asajeffrey commented Jun 14, 2017 •

edited by larsbergstrom