Paths

Table of Contentst

Differential D202884

Bug 1823463 - Render clip-path:shape().
ClosedPublic
Actions

Authored by boris on Feb 27 2024, 11:01 PM.

Details

Reviewers

emilio

Commits

rMOZILLACENTRAL27f7a56246fb: Bug 1823463 - Render clip-path:shape(). r=emilio

Bugzilla Bug ID

1823463

Summary

Update clip-path-shape-003.html and clip-path-shape-004.html because

Per SVG2 spec, we don't accept comma among commands, so I remove them.
Basically, these two tests want to test the result of shape() should be identical to the result of path(). However, I noticed the original tests which put a clip-path:path() with position:absolutely may have a fuzzy result if the path has some curves there. This may be caused by anti-alias together with absoultely positioned element (note: perhaps there are some floating point calculation in layout for this, so the final rendering coordinates may have some fractions). Therefore, I drop the absolutely positioned element, and just test that if the result of shape() is identical to the result of path().

Also, add two more tests for different reference-boxes together with
the usage of shape() (to make sure we resolve percentage values properly).

Diff Detail

Repository

rMOZILLACENTRAL mozilla-central

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Build Status

Buildable 640536
Build 739206: arc lint + arc unit

Event Timeline

wip

Herald added a project: secure-revision. · View Herald TranscriptFeb 27 2024, 11:01 PM

Harbormaster completed remote builds in B631970: Diff 828072.Feb 27 2024, 11:01 PM

phab-bot published this revision for review.Feb 27 2024, 11:01 PM

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".

phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".

phab-bot removed a project: secure-revision.

Herald added subscribers: zrhoffman, emilio. · View Herald TranscriptFeb 27 2024, 11:01 PM

Code analysis found 11 defects in diff 828072:

1 defect found by eslint (Mozlint)
2 defects found by clang-format
4 defects found by clang-format (Mozlint)
4 defects found by clang-tidy

WARNING: Found 10 defects (warning level) that can be dismissed.

IMPORTANT: Found 1 defect (error level) that must be fixed before landing.

You can run this analysis locally with:

./mach clang-format -p layout/svg/CSSClipPathInstance.cpp dom/svg/SVGPathData.cpp
./mach lint --warnings --outgoing
./mach static-analysis check --outgoing (C/C++)

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 828072.

wip

boris retitled this revision from Bug 1823463 - Render clip-path:shape() (wip). to Bug 1823463 - Render clip-path:shape()..Mar 7 2024, 10:38 PM

boris edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B636209: Diff 833113.Mar 7 2024, 10:38 PM

boris updated this revision to Diff 833575.Mar 8 2024, 8:49 PM

boris edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B636621: Diff 833575.Mar 8 2024, 8:49 PM

Code analysis found 12 defects in diff 833575:

4 build errors and 8 defects found by clang-tidy

WARNING: Found 8 defects (warning level) that can be dismissed.

IMPORTANT: Found 4 defects (error level) that must be fixed before landing.

You can run this analysis locally with:

./mach static-analysis check --outgoing (C/C++)

The analysis task source-test-clang-external failed, but we could not detect any defect.
Please check this task manually.

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 833575.

Fix clang-tidy

Harbormaster completed remote builds in B636695: Diff 833664.Mar 8 2024, 9:56 PM

12 defects closed compared to the previous diff 833575.

If you see a problem in this automated review, please report it here.

Fix test failures

Harbormaster completed remote builds in B636708: Diff 833681.Mar 8 2024, 11:22 PM

Update the name of variables

Harbormaster completed remote builds in B636719: Diff 833697.Mar 9 2024, 12:41 AM

boris added a reviewer: layout-reviewers.Mar 11 2024, 7:34 PM

emilio accepted this revision.Mar 15 2024, 6:59 PM

emilio added a project: testing-approved.

emilio added inline comments.

servo/ports/geckolib/cbindgen.toml
762–765	nit: As per the previous comment as well, should probably be called `ConvertToGfxPoint`, or just `ToGfxPoint`. Also maybe adding: inline gfx::Point ToGfxPoint(const CSSSize& aBasis) const { return ToGfxPoint(&aBasis); } Or so to make callers that provide a basis less awkward.