Implement raqote backend for canvas 2D rendering #24048

pylbrecht · 2019-08-25T13:51:43Z

This is a follow-up of #23936.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #23431

This change is


        Implement Path::contains_point()

components/canvas/raqote_backend.rs


        Implement PathBuilder::get_current_point()


        Refactor Path::contains_point() using any()

jrmuizel

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @asajeffrey, @CYBAI, and @pylbrecht)

components/canvas/raqote_backend.rs, line 190 at r3 (raw file):

        self.as_raqote().ops.iter().any(|op| match op {
            PathOp::MoveTo(point) | PathOp::LineTo(point) => {
                point.x as f64 == x && point.y as f64 == y

This contains_point() implementation is wrong. contains_point() is supposed to return whether the point is inside the path and not whether any of the path ops use that particular point.

jrmuizel

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @asajeffrey, @CYBAI, and @pylbrecht)

components/canvas/raqote_backend.rs, line 190 at r3 (raw file):

Previously, jrmuizel (Jeff Muizelaar) wrote…

This contains_point() implementation is wrong. contains_point() is supposed to return whether the point is inside the path and not whether any of the path ops use that particular point.

I'll add an implementation of contains_point() to raqote.

components/canvas/raqote_backend.rs

jrmuizel · 2019-08-27T01:44:16Z

Raqote now has a (possibly buggy) implementation of contains_point() jrmuizel/raqote@b1437ce

pylbrecht · 2019-08-27T10:37:46Z

This contains_point() implementation is wrong. contains_point() is supposed to return whether the point is inside the path and not whether any of the path ops use that particular point.

Thanks for pointing that out, @jrmuizel! I have zero experience in implementing 2D graphics whatsoever. I hope this doesn't get in the way too much, while working on this issue. I really don't want to create more work than neccessary.
May I hit you up, if I have any questions regarding 2D graphics?

jrmuizel · 2019-08-27T12:23:07Z

May I hit you up, if I have any questions regarding 2D graphics?

Sure. I'd be happy to answer any questions.


        Implement create_gradient_stops()


        Use raqote's implementation of Path::contains_point()


        Don't use catch all in match

jrmuizel

Looks reasonable to me.

jrmuizel · 2019-09-01T17:38:27Z

components/canvas/raqote_backend.rs

-        unimplemented!()
+    pub fn contains_point(&self, x: f64, y: f64, _path_transform: &Transform2D<f32>) -> bool {
+        let path = self.as_raqote();
+        path.contains_point(0.1, path.winding, x as f32, y as f32)


The newest version of raqote drops the winding parameter from contains_point() (though you can just deal with that later)

jdm · 2019-09-02T04:41:34Z

@bors-servo r=jrmuizel

bors-servo · 2019-09-02T04:41:35Z

📌 Commit d245682 has been approved by jrmuizel

bors-servo · 2019-09-02T06:48:41Z

⌛ Testing commit d245682 with merge b871578...


        Auto merge of #24048 - pylbrecht:raqote, r=jrmuizel

Implement raqote backend for canvas 2D rendering  This is a follow-up of #23936. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #23431     --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24048)

bors-servo · 2019-09-02T09:49:44Z

💔 Test failed - status-taskcluster

jdm · 2019-09-02T13:48:38Z

@bors-servo retry

bors-servo · 2019-09-02T13:48:39Z

💣 Failed to start rebuilding: Unknown error

bors-servo · 2019-09-02T13:48:45Z

⌛ Testing commit d245682 with merge fdc206e...


        Auto merge of #24048 - pylbrecht:raqote, r=jrmuizel

Implement raqote backend for canvas 2D rendering  This is a follow-up of #23936. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #23431     --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24048)

bors-servo · 2019-09-02T14:10:34Z

💔 Test failed - status-taskcluster

CYBAI · 2019-09-02T14:15:16Z

@bors-servo retry

bors-servo · 2019-09-02T16:32:20Z

💣 Failed to start rebuilding: Unknown error

bors-servo · 2019-09-02T16:32:25Z

⌛ Testing commit d245682 with merge 18f59fc...


        Auto merge of #24048 - pylbrecht:raqote, r=jrmuizel

Implement raqote backend for canvas 2D rendering  This is a follow-up of #23936. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #23431     --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24048)

bors-servo · 2019-09-02T17:23:34Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jrmuizel
Pushing 18f59fc to master...

Implement Path::contains_point()

Loading status checks…

2a0be45

highfive assigned asajeffrey Aug 25, 2019

highfive added the S-awaiting-review label Aug 25, 2019

CYBAI reviewed Aug 25, 2019

View changes

components/canvas/raqote_backend.rs Outdated Show resolved Hide resolved

pylbrecht added 2 commits Aug 25, 2019

Implement PathBuilder::get_current_point()

Loading status checks…

ee7f3db

Refactor Path::contains_point() using any()

Loading status checks…

89b8bd5

jrmuizel requested changes Aug 26, 2019

View changes

jdm requested changes Aug 26, 2019

View changes

components/canvas/raqote_backend.rs Outdated Show resolved Hide resolved

jdm assigned jdm and unassigned asajeffrey Aug 27, 2019

jdm added S-needs-code-changes and removed S-awaiting-review labels Aug 27, 2019

Implement create_gradient_stops()

Loading status checks…

7c81d20

highfive added S-awaiting-review and removed S-needs-code-changes labels Aug 29, 2019

jrmuizel added S-needs-code-changes and removed S-awaiting-review labels Aug 29, 2019

Use raqote's implementation of Path::contains_point()

Loading status checks…

47a6129

highfive added S-awaiting-review and removed S-needs-code-changes labels Sep 1, 2019

Don't use catch all in match

Loading status checks…

d245682

jrmuizel approved these changes Sep 1, 2019

View changes

highfive assigned jrmuizel and unassigned jdm Sep 2, 2019

highfive added S-awaiting-merge and removed S-awaiting-review labels Sep 2, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Sep 2, 2019

highfive added S-awaiting-merge and removed S-tests-failed labels Sep 2, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Sep 2, 2019

highfive added S-awaiting-merge and removed S-tests-failed labels Sep 2, 2019

bors-servo merged commit d245682 into servo:master Sep 2, 2019
2 of 3 checks passed

2 of 3 checks passed

Taskcluster (pull_request) TaskGroup: failure
Details

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Sep 2, 2019

servo / servo

Implement raqote backend for canvas 2D rendering #24048

Implement raqote backend for canvas 2D rendering #24048

pylbrecht commented Aug 25, 2019 •

edited by SimonSapin

jrmuizel left a comment

jrmuizel left a comment

jrmuizel commented Aug 27, 2019 •

edited

pylbrecht commented Aug 27, 2019

jrmuizel commented Aug 27, 2019

jrmuizel left a comment

This comment has been minimized.

jdm commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

jdm commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

CYBAI commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

servo / servo

Join GitHub today

Implement raqote backend for canvas 2D rendering #24048

Implement raqote backend for canvas 2D rendering #24048

Conversation

pylbrecht commented Aug 25, 2019 • edited by SimonSapin

jrmuizel left a comment

jrmuizel left a comment

jrmuizel commented Aug 27, 2019 • edited

pylbrecht commented Aug 27, 2019

jrmuizel commented Aug 27, 2019

jrmuizel left a comment

This comment has been minimized.

jrmuizel Sep 1, 2019

jdm commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

jdm commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

CYBAI commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

bors-servo commented Sep 2, 2019

pylbrecht commented Aug 25, 2019 •

edited by SimonSapin

jrmuizel commented Aug 27, 2019 •

edited